This is basically another case of commit d488adb6.

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

The function is supposed to return a list, not a string. Without this
patch the error message ends up as “C, a, n, ', t,  , c, h, a, n, g, e,
…”.

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

Thanks to Agata Murawska, a random seed that generates bad test cases
was found ("seed was 1768143307 2118231514, test size 74"). This is
due to the fact that some nodes were generated with tMem=0, fMem=0,
which means that pMem was NaN and thus the node was not comparing
equal to itself.

Worked around by limiting the test case generation to nodes with
positive values; a better fix would be to compare nodes differently
(subset of properties or ignore NaN values).

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

Commit 8a69b3a8 changed the DRBD barriers disabled by default if
“--disable-drbd-barriers” is used to “bfd”, where “d” stands for
disk-drain. DRBD's upstream updated their documentation[1] to
say disk drains should always be used.

[1] http://git.drbd.org/?p=drbd-8.3.git;a=commit;h=f952dcfed57acf



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

While reviewing the RAPI docs (for commit 003306f9), I realised that
the punctuation used in RAPI is inconsistent, especially with regards
to definition lists.

This patch changes all such lists to not have punctuation, except for
one case (the ecode descriptions, which have multiple
paragraphs). Furthermore, it fixes the few opcodes which had docs
ending with punctuation to not do so anymore.

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

Sadly we don't have a way to test that, for example, all
(G|N|I)Q_CONFIG fields are exported in RAPI, so for now we don't test
this; but we should, in the future.

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

The hypervisors are a list and need to be comma-separated.

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

This prevents from setting for example drbd options on the plain disk
template.

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

This verifies the user (especially in nested dicts) does not
provide a key which is not seen in the defaults dict for that dict.

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

For example, the “initrd_path” parameter is only available with xen-pvm,
whereas “boot_order” is only available with xen-hvm.

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

Some parts of QA should only be run with certain hypervisors (e.g.
testing instance HV parameters). To make that happen we always define
the cluster's hypervisors and provide a function to get the default
hypervisor.

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

Sorry, I broke the cfgupgrade unittests via 904910c4, since that
commit added the requirement for the "instances" dict in the
configuration.

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

Since Ganeti 2.6 does warn about disks with wrong iv_names, it would
be better to fix them when upgrading to 2.6.

Not sure if this is too brittle and whether we want to add more
safeguards on the contents of the config/instance/disks before
touching them.

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

These can be used, for example, to get jobs submitted after a certain
timestamp.

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

As reported on the devel mailing list by Christos Stavrakakis,
creation of plain instances is broken when the --no-wait-for-sync flag
is passed, because in that case WaitForSync is not called, hence
SetDiskID is not called at all, resulting in a None physical_id being
passed to backend.

We fix that by explicitly calling SetDiskID, which will cover the
pause/resume and os_add RPC calls.

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

This patch only touches comments and man pages.

Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Commit 4b42c3d6 broke non-root user mode since, while trying to do a
cleanup a move all local variable definitions to the start of the
function; however, the plain_name var is only defined later, so this
actually doesn't work.

Note that enabling set -u doesn't _directly_ work, since we rely on
some variables which are usually undefined (EXTRA_*_ARGS, etc.). It
can be fixed, but in a later patch.

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

I've added the changes under beta1, not beta2, since it's easier to
keep it separate.

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

For epydoc raise statement, it already expects an exception which will
be hyperlinked; if passing an L{…} construct, it will instead break
the formatting (the @raise will not be recognised, but merged into the
previous paragraph).

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

* stable-2.5:
  Fix race condition in test for *FileID functions
  Move KVM keymap from GenerateRuntime to ExecuteRuntime
  Add the keymap directory to the list of runtime KVM dirs

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

In this test the “file ID” of a temporary file is compared against the
file ID gathered via an open file descriptor to the same file. For
reasons unknown to me utime(2) is called in-between to update the
inode's a- and mtime. Depending on the file system's timestamp
resolution this can lead to a different file ID.

Found by chance during QA and reproduced by adding a delay before the
call to utime(2).

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
(cherry picked from commit fbd55434)

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

Per issue 243, "side-effects" are GenerateRuntime are bad as they
execute only on the initial node of the instance. By moving the
write-out of the keymap file to ExecuteRuntime, it will be done both
at start and at migrate time.

Furthermore, we update the docstring of GenerateKVMRuntime to explain
this, and add a fixme related to the spice per-interface binding.

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

Commit 4f580fef added the keymap support, but missed that this
directory needs to be ensured/created at hypervisor init time.

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

Due to the way $(MAKE) behaves (it forces execution of the commands
that contain it), use of $(MAKE) in a command line together with
side-effects commands will break stuff. It's better to keep it either
on a separate line, or if not possible, move said commands to a
separate make target.

After this patch, make -n distcheck, make -n regen-vcs-version and
possibly other commands now work correctly.

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

Currently, the lint target does a sequential: pep8, pylint, pylint on
the QA sources. hlint is run only when explicitly requested.

This patch reorganises the lint target by:

- splitting the current lint target into separate pylint (slow),
  pylint-qa (fast), pep8 (fast) targets
- making lint itself depend on the above plus hlint
- both pep8 and hlint are optional, based on whether the said binaries
  were detected at configure time

make -j is slightly faster after this patch (1m18s → 1m7s).

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

This has been reported internally 3-4 times already, and the current
version (from 8b437a6e) is still not good enough, it seems.

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

Commit f346a7d9 broke the TestInstanceImport test in QA, since it
dropped the "always generate" parameter for backup import. We add an
override mac parameter so that, no matter what is defined in the
config file, imported instances (can) have a different MAC.

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

During the preparations for 2.6, we cleaned up the codebase to use
newer exceptions (e.g. commits 30d25dd8, 79ac58fa), so we can now drop
this override.

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

This patch fixes a (brown-bag) bug in the ssconf implementation where
the looked-for filenames were not prefixed with the ssconf file
prefix.

Additionally, a test for this is added (which fails without the fix).

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

This way, it can be reused in the Haskell code too.

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

In some QA environments static MAC addresses must be used.

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

In some QA environments external scripts may add tags. With this patch a
regular expression can be used to ignore them during QA runs.

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

* stable-2.5:
  Fix parallel build failures
  QA: Add default setting for tests

Conflicts:
        Makefile.am (expected, from the parallel build fixes)
        qa/qa_config.py (due to cherry-pick, ignored, simply checkout out with --ours)

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

This is the 2.5 version of the "fix build failures":

- man/%.gen could be left over even in case of failure, due to
  automake bug
- make man/%.gen runs RUN_IN_TEMPDIR, so let's depend on it, since
  that target has the proper dependencies (create needed dirs)
- man/%.gen depends on a number of built sources, but the dependency
  was not declared

Furthermore, wraps a long comment.

Tested with -j4/-j16, after `make maintainer-clean'.

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

Commit 1010ec70 enabled all tests by default. In some scenarios where
the configuration is static one may not want newly added tests to be
run.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
(cherry picked from commit 59a8fe48)

Currently, the "common" instance tests (shutdown, rapi stopped
console, stopped modify, startup, rapi shutdown, rapi startup, list,
info, modify, rapi modify , console, rapi console, reinstall, rename,
rapi rename, rename and back, rapi rename and back, grow disk, reboot,
tags, cluster verify, rapi test instance, node list, job list) are run
4 times:

- one time for plain instance created via RAPI, using client
- one time for plain instance created via RAPI, without client
- one time for plain instance created via gnt-instance
- one time for DRBD instance created via gnt-instance

This makes the QA long, and is over-doing it (for non-full QAs): an
instance created via RAPI (either client) and via gnt-instance should
have the same parameters, but our duplicate tests do not ensure that,
just that instances behave OK.

This patch adds a toggle so that we can skip the common tests for
RAPI-created instances; the creation/deletion is still performed, but
all the other operations are not. This reduces the time of a "quick"
QA by ~20% (1h:15m → 1h).

The common tests will still be run (unconditionally) for
gnt-instance-created instances.

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

Currently, this is how the QA log looks like:

Command: ssh -oEscapeChar=none -oBatchMode=yes -l root -t -oStrictHostKeyChecking=yes -oClearAllForwardings=yes -oForwardAgent=yes -oControlPath=/tmp/ganeti-qa-multiplexer.DMzkuH -oControlMaster=no node14 exit
Command: ssh -oEscapeChar=none -oBatchMode=yes -l root -t -oStrictHostKeyChecking=yes -oClearAllForwardings=yes -oForwardAgent=yes node8 exit
Command: ssh -oEscapeChar=none -oBatchMode=yes -l root -t -oStrictHostKeyChecking=yes -oClearAllForwardings=yes -oForwardAgent=yes node18 exit

And these arguments are repeated over and over. This patch proposes to
log once the arguments, at the beginning of the QA (so that the SSH
commands can be reproduced, if needed) and then drop them from the
log. The new output looks like:

SSH command for primary node: ssh -oEscapeChar=none -oBatchMode=yes -lroot -oStrictHostKeyChecking=yes -oClearAllForwardings=yes -oForwardAgent=yes -oControlPath=/tmp/ganeti-qa-multiplexer.24lgrK -oControlMaster=no node14
SSH command for other nodes: ssh -oEscapeChar=none -oBatchMode=yes -lroot -oStrictHostKeyChecking=yes -oClearAllForwardings=yes -oForwardAgent=yes NODE
(the above are the informational messages about parameters, then)
Command: ssh node14 exit
Command: ssh node8 exit
Command: ssh node18 exit

This makes the QA log much more readable, by dropping unneeded clutter
(look how long the original lines were), and hopefully also a bit
smaller (current QA log is > 100MB of text output).

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Bernardo Dal Seno <bdalseno@google.com>

This makes ssh generate much less noise when QA is run itself without
a tty, as otherwise we get thousands of lines of:

  Pseudo-terminal will not be allocated because stdin is not a terminal.

in the QA logs (which are already big enough).

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

Commit 2e04d454 introduced the new offline state for the instance
state, but being a big monolithic patch it sneaked in something that
doesn't make sense.

The checks for extra instances (either wrongly up or just unknown) are
done purely on a name-basis, not on objects, so the types there are
wrong. Furthermore, they have no relation to the admin state of the
instance, so we just drop the entire if block. We keep the increment
of the offline instance count, but move it to a different loop over
instances.

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

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