GitLab

This CL updates QueryInstanceData to return NICs in the new format (mac,
ip, mode, link) and fixes gnt-instance info to properly display them.
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Compatibility with the old parameters is maintained, by allowing to
query for "bridge" "nic.bridges" and "nic.bridge/N", but None is
returned in that case for routed nics.
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

We're going to use this helper function for more than just hooks, so
we'll give it a more generic name.
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Currently we support querying for "mac" "ip" or "bridge", meaning "the
one of the first nic. We are not checking that there is a first nic,
though, and thus could incur in errors. This patch fixes it by returning
"None" should there be no such nic, as it's done when explicitely asking
for a nic via nic.<field>/<N>
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Some indentation was wrong, and pylint rightfully complained.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Now that all results are the same, we can even more simplify the
handling in cmdlib more. Almost all if result.RemoteFailMsg()…
constructs are similar, and we resurect the RpcResult.Raise() function
to take a message argument, which it will process and raise an
appropriate exception.

This means a significant reduce in boilerplate code. Only the cases
which handle the error specially (e.g. by warning only) need to touch
directly the failure message, which was renamed on the RpcResult object
for more clarity.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Currently the OSes have a special, customized error handling: the OS
object can represent either a valid OS, or an invalid OS. The associated
function, instead of raising other exception or failing, create custom
OS objects representing failed OSes.

While this was good when no other RPC had failure handling, it's
extremely different from how other function in backend.py expect
failures to be signalled.

This patch reworks this completely:
  - the OS object always represents valid OSes (the next patch will
    remove the valid/invalid field and associated constants)
  - the call_os_diagnose returns instead of a list of OS objects, a list
    of (name, path, status, diagnose_msg); the status is then used in
    cmdlib to determine validity and the status and diagnose_msg values
    are used in gnt-os for display
  - call_os_get returns either a valid OS or a RPC remote failure (with
    the error message)
  - the other functions in backend.py now just call backend.OSFromDisk()
    which will return either a valid OS object or raise an exception
  - the bulk of the OSFromDisk was moved to _TryOSFromDisk which returns
    status, value for the functions which don't want an exception raised

The gnt-os list and diagnose commands still work after this patch.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This patch converts all three file-storage rpc (create, delete, rename)
to new style result. This is done in a single patch as they all use a
helper function which itself needs to/can be converted.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This patch converts this rpc into the new style. Since the function
already had some error handling, we remove this custom error reporting
and replace it with our (new-style) result type. This allows significant
cleaning up of this code.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This also converts (and fixes) unittests and mock objects to deal with
this change, and the custom hook verifier in cmdlib.LUClusterVerify.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This also removes custom post-processing from rpc.py; since this call
has only one user, it was simple to move it back to the caller.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This also cleans up its single use in cmdlib.py.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This patch converts this rpc call to the new style result, and also
changes in the process the meaning of the QuitGanetiException's
arguments and the node daemon rpc call exception handler.

The problem with the exception handler is that we used a two-stage one,
and the inner used to catch all exception (including this one), so in
the logs we always had an exception logged, instead of the normal
'leaving cluster message'. The patch also adds logging of the
exception's arguments, so that we have a trail in the logs about the
shutdown mode.

The exception's arguments were reversed from the normal RPC results
style. While it makes somewhat more sense for this exception, we change
them such that they match the rpc result format.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This is used in multiple places outside cmdlib.py, so it's a more
interesting patch.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This patch also does some cleanup and enforces valid results (with
proper type, i.e. int for memory/disk values) from remote node,
otherwise we handle the result as failure.

We do this so that we can remove custom processing in rpc.py which is
very old and doesn't play with the new style result (and has bit me by
modifying the result received from the remote node).

After this change, the node_info rpc becomes a normal one.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This should actually have a function in backend, but it's fine for now.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Since backend.GetInstanceList() is used both as RPC endpoint and as
internal function, it can't return (status, value). Instead it returns
only valid instance info, and failures are denoted by exceptions; and
the ganeti-noded function adds the (True,) status.

The patch also fixes a typo.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This changes from a list of booleans to «status, error messages». This
means that instead knowing which disk has failed (position based), we
get a list of all failures (with details how they failed).
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This was a very simple (boolean) RPC, so converting it to actually have
more value with the new style results was more difficult.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This doesn't have known failure modes but converting will help later.

We also now call directly utils.ListVolumeGroups() instead of the
backend.ListVolumeGroups() so that we don't have to undo the (status,
value) result type.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This is a big change, because we need to cleanup its users too.

The call and thus LUVerifyDisks LU used to differentiate between failure
at node level and failure at LV level, by returning different types in
the RPC result. This is way too complicated for our needs.

The patch changes to new style result (easy change), and then:
  - changes LUVerifyDisks.Exec() to return a tuple of 3-elements
    instead of 4-elements; we collapse the «nodes not reachable» and
    «nodes with LVM errors» in a single dict
  - changes gnt-cluster to parse 3-element results and simplifies the
    different by-error handling code

Note that the status is added in ganeti-noded, and not in the function
itself, as the function is used in other places too.

This was tested with down nodes and broken VGs.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This converts the export_remove rpc to new style result and also fixes
and old TODO by adding exception handling (and conversion to failure).
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This is used in multiple places, so it has a little more changes than
the previous ones.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This also removes some code from ganeti-noded and rpc.py, which should
not do such processing of data (and be simply glue code). (Or
alternatively they could, if we had better infrastructure).
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This rpc doesn't have explicit failure modes, but once all RPCs are
converted and we can catch and convert expceptions to failure in noded,
we will have meaningfull error messages.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This also slightly changes the code in cmdlib that uses it for more
clarity.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

In order to do that we change the way nics are espected to be passed to
the _BuildInstanceHookEnv function, from (ip, bridge, mac) to (ip, mac,
mode, link).

To translate new nics, which have the ip, mac and nicparams fields to
this new format we add an auxiliary _PreBuildNICHooksList function,
which is called both in _BuildInstanceHookEnvByObject and in
LUCreateInstance, where we don't have an Instance object created yet, at
hooks execution time.

LUSetInstanceParams hooks generation is also changed to provide the
right override to the nics field, should any nics be modified.
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Since default_bridge is deprecated, avoid displaying it when querying
for cluster information.
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Rather than checking all bridges, we check all links for bridged nics,
if any. Since this function is now slightly more complex than before,
it's been split into an auxiliary function, and more parameters are
used, so that it can be called from more places, rather than
reimplementing the same code there.
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Hooks are dealt with in a later patch in the series.
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

There is some duplicated code between hv and be parameters to create the
updated parameters dict. This will need to be done also for nic
parameters, so we abstract this functionality in a function.
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>