Since the --master-netdev option is now used both at cluster init and at
cluster modify time, it cannot have a default value of DEFAULT_BRIDGE
(xen-br0) anymore. As such at cluster init we make the initialization
explicit if another value hasn't been passed. This fixes gnt-cluster
modify changing the master_netdev to xen-br0 if called without that
argument.

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

Right now, the QA code is not covered by pylint, and this shows at
least one low-impact bug.

This patch does the necessary changes to make QA pylint-clean, and the
changes the makefile to run pylint for it.

Notable changes:

- qa_utils.GenericQueryTest: randfields was not used at all, and my
  belief is that it was indented to be used in order not to modify the
  input list; so I replaced randfields with fields, so we only shuffle
  the our local copy
- qa_node.TestOutOfBand was using it's own copy of AcquireNode(), so I
  replaced it with the existing version
- qa_os: was using 'dir' in a couple of places, replaced with dirname

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

The recent additions to QA (many more tests) make QA slow if the
machine on which the QA runs is not very close to the tested nodes —
or in general, when the SSH handhaske is costly.

We discussed before about using a persistent connection, and here is
the patch that implements it. On a very small QA (very very small), it
cuts down a lot of time (almost half), so it should be useful even for
a full QA.

I've also thought about changing from external ssh to paramiko, but I
estimated that it would be more work to correctly interleave the IO
from the remote process than just running a background SSH.

Also note that yes, the global dict is ugly, but I don't know of
another simple way to implement this.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Patch f55312bd added the OOB tests to TestClusterVerify, which is not
actually a test for cluster verify, but a runner for cluster verify
that is called multiple times, for each instance type, etc. This led
to running the OOB commands multiple times, which is painful
especially as this is a slow test.

The patch moves this to a separate test, that is run only once.

Furthermore, the way that data files are copied around is very
inefficient: touch + mv + chmod + mv + rm for each node (5 times
number of nodes), whereas it could be simply: touch on master, chmod
on master, cluster copyfile, chmod on master, cluster copyfile,
cluster command rm, i.e. only 5 fixed ssh calls to the master. The
code is changed as such, for increased speed.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Passing tap devices to KVM as file descriptors requires that the respective
file decriptors remain open during utils.RunCmd execution. To this direction,
we add a “noclose_fds” keyword argument to utils.RunCmd, accepting a list of
file descriptors to keep open. The actual fd handling is implemented in
_RunCmdPipe and _RunCmdFile using subprocess.Popen's “preexec_fn”[1],
since subprocess.Popen provides no other way to selectively handle fds.

A small modification is also made to test/ganeti.utils_unittest.py to comply
with _RunCmdPipe's new API and a new unit test is added to test the selective
fd retention functionality.

[1] “If preexec_fn is set to a callable object, this object will be called in
     the child process just before the child is executed. (Unix only)”
    Subprocess documentation

Signed-off-by: Apollon Oikonomopoulos <apollon@noc.grnet.gr>
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

For an instance with a single disk the message should be “… disks 0 to
0”, not “… 0 to 1”.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

Also convert the per-line pylint instructions to one per-module
instruction.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

It's equivalent to a simple RemoveFile now.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

It's not exactly a wrapper, but this seemed like the best place.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

“import signal” was missing, but of course it didn't show during my
testing.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Use a function instead of a variable written by another module.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

This avoids having to monkey-patch the utils module for unittests.
Monkey patching is evil and caused a bug while moving code around.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Sorry I thought I did run commit-check but must not have paid attention
to its output. There was a typo in the docstring. This patch fixes this.

Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

The hooks is due to me not checking the recent patch from David, and
the cli.py change is due to the utils.py split.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Signed-off-by: David Knowles <dknowles@google.com>
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Docstring indicated "cmd" should be a list, when it's expected that
it'll be a string.

Also, converted docstring to current epydoc-style list of function
arguments.

Signed-off-by: Adeodato Simo <dato@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Signed-off-by: Adeodato Simo <dato@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Signed-off-by: Adeodato Simo <dato@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Emit a warning for each instance with a mirrored disk template that has
its primary and secondary nodes living in different node groups.

Signed-off-by: Adeodato Simo <dato@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Signed-off-by: Adeodato Simo <dato@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Signed-off-by: Adeodato Simo <dato@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This operation allows to assign nodes to a group, verifying whether
the change would leave any mirrored instance split between different
node groups.

Signed-off-by: Adeodato Simo <dato@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

To stay consistent, RAPI resources should only use a restricted set of
characters. No uppercase, no “_”, etc. This is enforced by this patch.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

This makes it a public function, which is necessary for
moving code into separate files.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>