A newly added piece of code will also have to parse headers, so having
this wrapper saves us from copying this part of code.

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

Commit f0d22861 changed the logic of
gnt_instance._ConvertNicDiskModifications to also allow a parameter
named “modify”. Unfortunately the corresponding unittest was not
updated. An “if”/“else” condition is also merged.

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

- Rename xm-console-wrapper to xen-console-wrapper
- Pass the xen command to use as a parameter

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

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

Until now the only way to make live migration work in conjunction with
"xl" was to add ssh known_hosts keys for every node's secondary ip on
every other node.

With this command we remove the target key verification: this is not
worse than what we were doing before with "xm", and allows the migration
to happen under either toolstack, without extra manual work. Of course
the full security of ssh is not used by live migration, then.

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

If the toolstack is set to "xl" we shouldn't ping xend for liveness
before attempting a live migration.

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

When a job is still waiting for locks and the queue is shutting down,
they should be returned and not actually start processing. Until now
jobs which transitioned from “queued” to “waiting” were already
considered to be running as far as the shutdown code was concerned.

This fixes issue 296.

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>

Currently, we have instance rename doing extra checks on the host
name, to prevent accidental wrong renames; however, instance create
doesn't do these checks (issue 291), which (if DNS is misconfigured)
can lead to hard to diagnose errors.

This patch abstracts the name checking from LUInstanceRename into a
separate function, which is then reused in both instance rename and
instance create.

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

This addresses issue 290: when receiving new jobs, logging is
incomplete, and we don't have the job ID(s) and/or summaries
logged. Only later, when the job is queried for or being processed, we
know more.

This is not good when troubleshooting, so let's improve the initial
logging.

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

There are two issues with lock exceptions right now:

- first, we don't log the original error; this is fine for now
  (locking.py always returns the same error here), but in general is
  brittle: if locking.py would start returning more information, we'd
  completely miss that

- second, an actual honest lock conflict is not an internal error;
  it's simply an optimistic lock failing, and as such we should not
  return internal error, but rather resource_not_unique

This addresses issue 287.

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

Commit 2c0af7da which added the runtime memory changes functionality
had a small typo (wrong name); I've rewritten this to only compute the
delta once, for simplicity.

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

This variable can be empty, when we want to disable LVM, so we can't
use TMaybeString.

Fixes issue 285.

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

This patch makes _RemoveDisks symmetric to _CreateDisks with respect
to file-based storage: _CreateDisks uses "in constants.DTS_FILEBASED",
whereas _RemoveDisks was not update and only uses "==
constants.DT_FILE". This results in stale directories left on the
filesystem.

Fixes issue 262.

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

Currently, the warning/notice about non-redundant instances in cluster
verify is based non empty secondaries list (how old is this?); the
proper way to check this nowadays is via DTS_MIRRORED.

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

This fixes issue 282.

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

Currently the message does not say explicitly that instance-initiated
reboots are useless to trigger the use of new parameters, per the
thread on the user mailing list. Let's improve it a bit.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

In Ganeti 2.6, disk adoption is broken due to the ipolicy checks being
done before we read volume size from remote nodes. We fix this by
simply moving these checks to after the disk adoption code which
updates the disk size; it's not that nice that we fail a (almost)
config-level check after we've reserved the LVs, etc., but we need to
do so in order to validate the ipolicy correctly.

Tested:

- normal instance creation
- creation via adoption with good size (pass)
- creation via adoption with wrong LV size (fail as expected)
- QA in progress

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

Example output:
$ gnt-cluster verify-disks
Submitted jobs 4327
Waiting for job 4327 ...
No disks need to be activated.

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

The man page and the bultin-in help for gnt-instance replace-disks were
inconsistent. Also fixed some typos in man pages.

Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

First: This enables the use of “gnt-job watch $id” for archived jobs.

Now, the reason for actually making this work is that during
sufficiently large group or node evacuations jobs are archived before
the client gets to poll for their output. This led to situations where
the jobs would finish successfully, but the client reported an error
because it couldn't see the job anymore.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Bernardo Dal Seno <bdalseno@google.com>
(cherry picked from commit 04569469)

People unfamiliar with Ganeti's internals might be confused with the
different hostnames showing up later in the process.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Bernardo Dal Seno <bdalseno@google.com>

If one or more jobs failed the exit status should be set accordingly.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Bernardo Dal Seno <bdalseno@google.com>

This fixes issue 190. The problem was that the check for DRBD was
enabled if LVM storage is used and didn't depend at all on whether DRBD
is enabled.

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

If an administrator sets always_failover, it means that there is no need
for another explicit approval to failover instead of migrating.

Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>
(cherry picked from commit b5f0b5cc)

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

Commit 5fce6a89 changed RpcRunner._InstDict to add the disk parameters
on all encoded instances. It didn't remove a special case in
“_InstDictOspDp”. Update and fix unittests as well.

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

We call _GatherAndLinkBlockDevs during the process, which in turn
calls _RecursiveFindBD. This needs disk parameters to work.

See also commit b8291e00.

This was reported by Ansgar and Damien.

Signed-off-by: Constantinos Venetsanopoulos <cven@grnet.gr>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

When locks are copied “locking.ALL_SET” must be handled separately
(ALL_SET has the value None). Reported by Constantinos Venetsanopoulos
who saw failover for RDB-based instances not working.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

There was an issue with the recent ipolicy introduction which lead to a
bug in gnt-debug iallocator. It was not providing the spindle_use field
and therefore it wont let you create a valid iallocator request.

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

To help developing Ganeti on newer distributions, let's try to fix
pylint warnings/errors. I'm using pylint from current Debian wheezy:
pylint 0.25.1, astng 0.23.1, common 0.58.0, and we have 3 things that
needs fixing.

First, a really wide "except", with the silencing in the wrong
place. I'm not sure why this doesn't have "except Exception", so let's
add it. However, pylint still complains about "Catching too general
exception", even though we do want to catch both system and our
exception, so let's add a silence for W0703. It's true that we
shouldn't catch KeyboardInterrupt and friends, but that should be
cleaned up on the master branch.

Second, pylint complains about "redefining name builtin tuple",
because we do some pattern matching in the except blocks in
netutils. This seems to be a false positive, but let's clean the code
around this.

And finally, type inference again goes bad, so let's silence E1103
with its "boolean doesn't have 'get' method".

After this, I can run "make lint", and by extension "make
commit-check" on Debian Wheezy, yay! We might be able to bump our
required pylint versions to something not ancient…

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

Pylint version:

  pylint 0.25.1,
  astng 0.23.1, common 0.58.0

crashes when passing the fully-qualified decorator name with:

  File "/usr/lib/pymodules/python2.7/pylint/checkers/base.py", line 161, in visit_function
    if not redefined_by_decorator(node):
  File "/usr/lib/pymodules/python2.7/pylint/checkers/base.py", line 116, in redefined_by_decorator
    decorator.expr.name == node.name):
AttributeError: 'Getattr' object has no attribute 'name'

I found out that simply using a shortened name will 'fix' this issue,
so let's do this to allow running newer pylint versions.

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

Currently, hail fails with FailDisk when trying to add an instance
of type: 'file', 'sharedfile' and 'rbd'.

This is due to a "0" or None value in the corresponding dict inside
_ComputeDiskSize, which results in a "O" or non Int value of the
exported 'disk_space_total' parameter. This in turn makes hail fail,
when trying to process the value:

 - with "Unable to read Int" if value is None (file)
 - with FailDisk if value is 0 (sharedfile, rbd)

The latter happens because the 0 value doesn't match the instance's
IPolicy, since it is lower than the minimum disk size.

The second problem still exists when using adoption with 'plain'
and 'blockdev' template and will be addressed in another commit.

Signed-off-by: Constantinos Venetsanopoulos <cven@grnet.gr>
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Due to an oversight, the pause/resume sync RPC calls in _WipeDisks
lack the verification of the overall RPC status, and directly iterate
over the payload. The code actually doing the wipe does verify
correctly the results. This can result in jobs failing with a hard to
diagnose:

OpExecError ['NoneType' object is not iterable]

instead of proper "RPC failed" message.

This patch adds a hard check on the pause call, but for the resume
call it just logs a warning if the RPC failed; the rationale being
that if we can't contact the node for pausing the sync, it's likely
wiping will fail too, but after the wipe has been done, we can
continue.

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

This breaks the command, as optparse considers that an error.

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

It seems that 'explicitely' is wrong, and that the right form is
'explicitly'. This is just fixing the typo plus adjusting affected
paragraphs.

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

In case LUClusterVerifyGroup is run on a group which doesn't contain
the master node, the following could happen:

- master node is selected due to the explicit check
- if the order of nodes in the 'absent_nodes' list is such that the
  master node is the first in it, then we'll select (again) the master
  node
- passing duplicate nodes to RPC calls will break due to RPC
  internals; this should be fixed separately, but in the meantime we
  just refrain from passing such duplicates

This patch should not change the semantics of the code, since it
wasn't guaranteed even before that we find a vm_capable node.

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

Commit 904b3bfe tried to fix the deletion of custom ndparams from
group, but instead broke both modification and deletion: because we
run ForceDictType on self.op.ndparams instead of the updated
new_ndparams, we can neither delete nor set properly spindle_count
(since it won't be coerced to int).

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

This generalises commit 4304964a to cdroms too, since they have
somewhat the same logic. We just abstract the needs_boot_flag into a
separate variable, and then reuse it in the cdrom section.

Note that the logic of what 'if=' type to pass to KVM was very
convoluted, and (I think) incorrect; I went and cleaned it to be more
consistent.

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

This addresses issue 230: passing two methods of booting to KVM can,
depending on the KVM version, confuse it.

Note that commit 4304964a introduced a partial fix for this (but only
for disks, and keyed on KVM versions). However, it didn't fix cdrom
booting, which still fails with the same error, so let's fix it more
generically; we still leave the per-disk check since that is about
-boot c versus -drive …,boot=on rather than two boot methods.

Patch is based on the one submitted by Vladimir Mencl, many thanks!

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

On node groups we don't have the std field. However, the InstancePolicy
object always verifies that the std value is within a given range. As we
fill it up with defaults if not set (as it happens to be on node groups)
and the min value is higher than the default std value (taken from
constants.py) we fail.

We overcome this situation by simply let the function know if we want to
verify the std value at all. If we don't want to verify std, we just set
it to a compliant value (min_v) and continue.

We also slightly adapt the error message provided, as we don't have std
values on groups.

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