Note: I'll add tests for this on the master branch, but not here.

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

The only semantic change is the fix of the spelling of the option
--ipolicy-disk-templates.

Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This fixes Issue 257.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Helga Velroyen <helgav@google.com>

Commit 1ce03fb1 (“Add ht-based result checks to opcodes”) introduced
infrastructure for checking opcode results, and subsequent commits
improved the list of opcodes which do declare a result, however this
was not tested for dry-run mode operation.

Furthermore, there's no authoritative list of which opcode/LUs support
dry_run mode at all; currently, this is based/restricted on the list
of CLI options, so… for now we disable the result verification if the
opcode has been executed in dry_run mode.

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

This is in preparation for a 2.6.2 release.

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

When all OS parameters should be unset (“gnt-os modify -H -xen-pvm
foo”), a TypeError was raised. This fixes issue 311.

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

In cmdlib we must only use the hypervisor class, and never instantiate
it. As such we have to call GetHypervisorClass instead, to avoid
getting an instance of it.

This fixes Issue 316, because __init__ is not called from masterd
anymore, and thus can't fail on EnsureDirs.

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

The 'command' attribute of the OpOobCommand command is defined with a
default value of None, but its validation requires a member of
constants.OOB_COMMANDS, which doesn't accept None. This result in the
following error when submitting an opcode without the command:

  error type: wrong_input, error details:
  Parameter 'OP_OOB_COMMAND.command' fails validation

I suspect this was simply a mistake, since the commit that introduced
it (65e183af, “opcodes: Add opcode parameter definitions”) did lots of
bulk updates.

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

s/exists/exist/

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Michele Tartara <mtartara@google.com>

The text of the manpage explains that an index can be prepended to
"remove" but the short help doesn't mention it. Adding it helps making
the syntax clear.

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

Commit 6a1434d7 (“Make migration RPC non-blocking”) changed the API
for reporting migration status, but has a small cosmetic bug: if the
migration status if failure, but the RPC itself to get the status
didn't fail, it shows the following error message:

  Could not migrate instance instance2: None

since it always uses result.fail_msg, irrespective of which part of
the if condition failed.

This patch simply updates the msg if not already set, leading to:

  Could not migrate instance instance2: hypervisor returned failure

Proper error display can be done once the migration status objects can
return failure information as well, beside status.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Helga Velroyen <helgav@google.com>

Commit 6a1434d7 (“Make migration RPC non-blocking”) changed from
raising HypervisorErrors to returning MigrationStatus
objects. However, these objects don't have an "info" attribute, so
they can't pass a reason back (which is in itself a bug); but the KVM
hypervisor code attempts to do so, and fails at runtime with:

  Failed to get migration status: 'MigrationStatus' object has no attribute 'info'

instead of the intended:

  Migration failed, aborting: too many broken 'info migrate' answers

For now (on stable-2.6), let's just remove the "info" reason, and
later we can add it back properly once we have a way to correctly
represent migration status failures in the LU.

This fixes issue 297.

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

Currently, the code uses createFile, which has the effect of always
truncating the file. This is bad, as the content of the PID file is
wiped even when we wouldn't be able to lock it!

We switch to openFd (createFile is just a wrapper over that), and we
use an explicit set of flags; defaultFileFlags is already safe
(trunc=False), but I prefer to set it explicitly with our desired
flags.

Note that this bug doesn't manifest in normal usage, as daemon-util
won't try to start the daemon if already running. But if anyone or
anything does call ganeti-confd explicitly, the pid file will be
emptied and the daemon will keep trying to be restarted forever…

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

pylint complained, I fixed it, and unfortunately pushed too early.

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

Once again this will be used by forthcoming RAPI test.

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

A newly added test for RAPI will also verify the returned headers. A
test in ganeti.rapi.client_unittest.py is split into smaller stand-alone
tests.

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

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>

This small patch fixes compatibility with a few newer Haskell libraries:

- base 4.6, included with ghc 7.6, removed the deprecated 'catch'
  function from Prelude, so our "import Prelude hiding (catch)" is now
  an error; we workaround by using fully-qualified
  Control.Exception.catch name

- containers 0.5 changed the signature of 'deleteFindMax'; we
  workaround by using separate 'findMax' and 'deleteMax'

- QuickCheck 2.5 removed the 'maxDiscards' test parameter, replacing
  it with a much better 'maxDiscardsRatio'; however, until we can
  depend on that, we workaround by just removing it (we don't control
  anymore the maxDiscards, instead leaving it default; for our default
  test size, this is no change, as the default value is already 500,
  which is our default as well) and not printing it anymore

Tested on Squeeze (+extra libs), Wheezy and experimental, which covers
all supported GHC versions.

Also, merging this in master will be a pain, but unless we want to
stop supporting 2.6…

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@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>

Signed-off-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>

This is the bit of documentation missing for issue 170. Doing
development on a machine which already has Ganeti installed kind of
works, but only when the installed and the developed version are very
similar, and even then it can be problematic.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@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>

Sorry, this should have gone in the previous commit, I forgot about
it.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Bernardo Dal Seno <bdalseno@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>

Setting a high prefix discourages the bridge from adopting the tap's
MAC. Xen is not affected by this since the MAC is forced to
"fe:ff:ff:ff:ff:ff".

This addresses issue #217.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
(cherry picked from commit be3f52f3)
(this is critical enough that we want it on stable-2.6)

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Guido Trotter <ultrotter@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>

Better formatting of text, past tense used when appropriate.

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