GitLab

This removes some constraints:
  - only two disks supported, this is no longer true as the underlying
    functions can now compute size for a variable number of disks
  - error when the hypervisor was not being passed
  - typo error

Reviewed-by: imsnah

This patch converts the DRBD minors reservation protocol from explicit
release to automatic release on the success paths. On the errors paths,
it's still needed to manual release.

The patch doesn't bring much by itself, but is needed for a future patch
which enhances the automatic verification of configuration consistency.

Reviewed-by: ultrotter

Due to historic reasons, the “should run or not” attribute of an
instance was denoted by its “status” attribute having a string value of
either ‘up’ or ‘down’. Checking this is in code was done via hardcoding
of the strings.

This was long done for a redo, and this patch changes this attribute to
“admin_up” having a boolean value. The patch is in fact shorter than I
expected, and passes burnin.

The patch also fixes an error in BuildInstanceHookEnvByObject where the
instance.os was passed as the status value.

Reviewed-by: ultrotter

Currently the hypervisor is expected to do all the migration from the
source side. With this patch we also add the option of passing some
information to the target side, and starting some operation there.

As a bonus, a function to cleanup any started operation is included.

Reviewed-by: iustinp

This allows the rename failures to show the ouput of OS scripts.

Reviewed-by: ultrotter

The ConfigWriter.AllocateDRBDMinor requires the instance name, not the
instance object. The LUSetInstanceParms is passing wrongly the instance
object, which can cause breakage.

The patch also adds asserts to check for this mismatch in ConfigWriter.

Reviewed-by: ultrotter

This patch adds support for verification of drbd minors space in cluster
verify: minors which belong to running instances and should be online
but are not, and minors which do not belong to any instace but are in
use.

The patch requires exposing some methods from bdev.DRBD8 and
config.ConfigWriter which were until now private methods.

Reviewed-by: ultrotter

Reviewed-by: ultrotter

This allows the install and reinstall instance to return (hopefully)
relevant log files from the OS create scripts.

Reviewed-by: ultrotter

This will record the failure cause in starting up the instance in the
job log (and thus to the user).

Reviewed-by: ultrotter

Commit 2302 only modified _CreateBlockDevOnPrimary to the new style
result, but _CreateBlockDevOnSecondary was forgotten. After the merger
of the two functions, _CreateBlockDevOnSecondary was taken as template
so we checked against old-style values, thus completely breaking error
handling.

Reviewed-by: imsnah

This patch replaces a few obvious uses of [instance.primary_node] +
list(instance.secondary_nodes) (or similar usage) with the new
instance.all_nodes.

Reviewed-by: ultrotter

Some callers of _CreateBlockDev need recursive behaviour, but not all.
The replace secondary first creates (manually) new LVs to ensure storage
is there, and then it creates the new DRBD. At this point, we need a
non-recursive call so that the LVs are not needlessly re-created.

This patch splits the single device creation into a separate function,
so that LUReplaceDisks can use it.

Reviewed-by: ultrotter

Since only two boolean parameters differ between these two functions, we
combine them as to have less code duplication. This will be needed in
the future as we will need to split off the recursive part off.

Reviewed-by: ultrotter

This allows errors to be visible at the user level instead of just node
daemon logs.

Reviewed-by: ultrotter

For future propagation of error messages from backend to cmdlib and to
the job log, just having True/False return from the disk creation
function is not enough.

This patch converts these functions (_CreateDisks, _CreateBlockDevOnXXX)
to raise exception on errors, and otherwise the return value is None.

Reviewed-by: ultrotter

Currently we use a different UUID for the _data and _meta volumes of a
DRBD disk. This is confusing as it's hard to associate the two in the
output of “lvs” or “gnt-node volumes”.

The patch changes so that they use the same prefix.

Reviewed-by: ultrotter

Due to deficiencies in our block device implementation, it is a must to
call SetDiskID on disks before passing them to remote nodes. Since in
export/import, we don't touch the disks themselves, this was not needed
before in this function.

However, since having instance symlinks, the correct ID is needed here
too, and with static minors it's a "must need". This reflects into
failed instance starts after migration and/or failover.

Reviewed-by: ultrotter

We need to pass the fully-qualified node to _CheckNodeOnline, not the short
one.

Reviewed-by: imsnah

This is forward port via copy (and not individual patches cherry-pick)
of the latest code on the 1.2 branch related to the migration.

The changes compared to 1.2 are the fact that we don't need the
IdentifyDisks step anymore (the drbd rpc calls are independent now), and
the rpc module improvements.

Reviewed-by: ultrotter

In replace disks to new secondary, since Attach (and therefore
call_blockdev_find) is not modifying the devices anymore, we need to
switch this LU to the new call_drbd_disconnect_net and
call_drbd_attach_net functions.

Due to the authentication needed in 2.0, we need to be more careful with
the activation order. In 1.2, we have the case that the new node was
directly activated with networking information, and could connect to the
primary while it was still connected or WFConnect to the old secondary.

In the new scheme, we:
  - create the new drbd in StandAlone mode
  - shutdown old secondary (primary becomes WFConnection)
  - disconnect primary (and thus it goes into StandAlone)
  - connect both primary and new secondary to network using the
    call_drbd_attach_net rpc

This should be safer, and is cleaner. This passes burnin.

Reviewed-by: ultrotter

... as found by the QA script - bug was introduced by me in commit 2117.

Reviwed-by: imsnah

The RpcResult introduction still left some bugs (after multiple patches):
  - we don't correctly check the result type
  - rename a variable to prevent a conflict

Reviewed-by: imsnah

The checking for invalid instance names in LUQueryInstanceData is broken
since commit 1642.

Reviewed-by: imsnah

This LU can be used to force a push of the config in case it's needed,
for example after an upgrade to update the ssconf_release_version file.

Reviewed-by: imsnah

We shouldn't query offline nodes in gnt-os. This patch adds an utility
function to ConfigWriter that returns the names of online nodes and uses
it in LUDiagnoseOS to query only the good nodes.

Reviewed-by: imsnah

In 1.2, due to the md+drbd7 legacy, we had a complex choice of replace
modes, and the new drbd8 modes where forced into this syntax, with some
complicated rules of transition from one mode to another (if REPLACE_ALL
but not new node passed, switch to REPLACE_SEC, etc.).

This patch cleans this situation by making a clear separation between
the two main modes:
  - replace on current nodes (with the two sub-cases on primary and on
    secondary)
  - change to a new node (either via manually specified node or via
    iallocator)

Reviewed-by: imsnah

For offline nodes, we shouldn't add them to the NV_NODELIST and
NV_NODENETTEST tests since they most likely won't succeed.

The patch makes gnt-cluster verify happy again in such cases.

Reviewed-by: imsnah

Commit 2117 changed _AssembleInstanceDisks to correctly parse the
failure status of the new RpcResult structure, but it didn't fix the
storing of only the result payload. Since RpcResult is not JSON
serializable, LUActivateInstanceDisks is failing.

Reviewed-by: ultrotter

Reviewed-by: amishchenko

The primary node is part of the instance, not of the opcode.

Reviewed-by: iustinp

Two problems: the iallocator.hypervisor wasn't initialized to None in
the constructor, so pylint doesn't realize it's initialized later with
setattr.

Second, 'hypervisor' is a module, so we shouldn't use it as a variable.

Reviewed-by: amishchenko

And a small whitespace fix.

Reviewed-by: amishchenko

hypervisor is a module, so we shouldn't use it as an argument.

Reviewed-by: amishchenko

A few cases of wrong variable name.

Reviewed-by: amishchenko

This is a leftover from the abstraction of AdjustCandidatePool, and it
also requires the config lock, so it's better to remove it.

Reviewed-by: amishchenko

Reviewed-by: amishchenko

The patch also changes the result to error for non-reachable secondary nodes
(as for primary nodes).

Reviewed-by: ultrotter

Currently the ConfigWriter.MaintainCandidatePool returns node names, and
_AdjustCandidatePool uses them as such, but then it passes these to
context.ReaddNode which in turn passes them to jqueue.JobQueue.AddNode which
uses them as objects.Node instances.

Since this is currently the only usage, we change return type from
ConfigWriter.MaintainCandidatePool to be objects and adjust the logging of
their names, so that the auto-adjusement works.

Reviewed-by: ultrotter

This patch changes gnt-node modify and the associated opcode/lu to allow
modification of the node offline attribute.

Setting a node into offline mode automatically demotes it from the
master role.

Reviewed-by: ultrotter