"make regen-vcs-version" is run at every ./devel/upload time, in order
to have reasonably up-to-date information in the uploaded
scripts/binaries. However, this means it currently triggers
recompilation of (at least) Ganeti/Version.hs, and sometimes more than
that.

Since we only care about the contents of the file and not the
timestamp, let's change the make rule so that it only updates the file
if there are actually changes in the content, as checked by
"cmp". This results in a much faster ./devel/upload, especially for
small changes in the (non-Haskell) code, or when the code has already
been compiled.

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

Sorry, I broke lint again :), by introducing two sub-standard changes.

Additionally, this silences an older existing warning that only
triggers with some versions of hlint (e.g. 1.8.28 which is present in
Wheezy).

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

The “_postfork_fn” parameter was only used for tests until now. To
implement a good locking scheme, remote commands must also make use of
this callback to release a lock when the command was successfully
started (but did not yet finish).

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

* devel-2.6:
  Improve error message when migration status fail
  Fix type error in kvm/GetMigrationStatus
  Fix PID file writing in Haskell daemons

Conflicts (both trivial):
        htools/Ganeti/Daemon.hs (_writePidFile rename)
        lib/hypervisor/hv_kvm.py (master fixed an indentation issue)

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

* stable-2.6:
  Improve error message when migration status fail
  Fix type error in kvm/GetMigrationStatus
  Fix PID file writing in Haskell daemons

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

We need to change a few things, most importantly CLI options defaults,
but otherwise we already used the path to files from functions which
were already in the I/O monad, so we don't have to change much of the
code flow.

Additionally, Path.hs now has an explicit export list, to stop
accidental leakage of symbols.

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

Currently, keyToFilename uses itself the default config path. In the
light of making that an function in the IO monad, let's remove the
"default" path functionality from this function and make it always
require the config path; its caller, readSSConfFile, can then do the
maybe/default path change.

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

Some options have defaults that depend on the environment, and we
could handle these in two ways:

- use a place-holder value (e.g. data X a = Default | Custom a) that
  is later read from the environment
- move the options list to IO monad, where it can read the
  environment, etc.

The second option allows also displaying the actual defaults in the
`--help' output, even though it's not as nice, so I went with it.

This patch only changes the option types, without actually changing
any options yet.

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

The "starting" message is changed to match the Python one, and in case
the preparation fails, we also log the error (beside printing it on
stderr or writing it to the error reporting pipe), as at this time
logging is usually set up.

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

The luxi binding is now annotated for better readability.

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

Currently, the writing of the PID file uses a "standard" error
reporting: catch exception, transform it into a 'Result' type, leave
handling of that to the caller. However, for daemon startup, we
actually want exceptions to be propagated up until the handler which
will write the details to the pipe.

This patch removes the writePidFile wrapper (and stops exporting it),
and changes the code to simply annotate better the I/O error.

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

It turns out that annotateIOError already exists in the standard
library, with a different purpose (this made me waste 10 minutes
trying to understand why the types were wrong…). Let's move this out
of BasicTypes, since it's used only in Daemon.hs, and also rename it.

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

This changes the daemon reporting error to the same mechanism as in
Python: use a pipe which is written to from the forked children.

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

This does the address parsing earlier, before we fork, for
better/faster error reporting.

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

This will help with the general daemon split of prepare/run, and flag
errors earlier in the startup.

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

Currently, we keep information about the "target" of a tag operation
in a data type similar to (TagKind, Maybe String). This is unsafe, as
nothing (at the type level) prevents us from accidentally having
(TagCluster, Just "instance1.example.com"), or (TagInstance, Nothing).

To fix this problem, we rename the current TagObject type to TagType
(an internal utility type), and create TagObject as a better/safer
data type (see the definition), which doesn't allow such possibilities
in the future.

The downside is that, since at encoding level (both opcode and luxi)
this is done in an ugly way (type elements spread at the same level as
level as other value), we have to add custom encoders/decoders. The
encoder is shared between the OpCode and Luxi usage, the decoder is
different however as Luxi uses custom decoding.

This also fixes the recent breakage in confd w.r.t. QueryTags.

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

This is needed so that we have more flexibility in generating Luxi
serialisation code (deserialisation is still custom). Also, only
exceptions are now using the 'simple' field types, so we might be able
later to convert and remove that TH code as well.

Since we will use custom serialisation fields in the future, we change
the order of serialisation for custom-save fields; Luxi uses
positional as opposed to name-based ordering, so we need to keep this
stable.

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

Doh, this is exactly the opposite of what we wanted… good thing no
test failed so far :)

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

This can be improved, by taking all hardcoded names as parameters, to
serve as a more-general "build save clause for a multi-constructor
data type". I'm not renaming the function as well, since I don't know
exactly how much we can abstract later.

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

This way constants like “rapi.RAPI_ACCESS_WRITE” can be used in
documentation.

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

Commit 208a6cff just included empty docstrings.

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

Instead of being given the priority for acquiring locks by means of a
parameter, mcpu will now call back. This is in preparation for
implementing a command to change a job's priority on the fly and allows
to change it while locks are being acquired (taking effect on the next
lock acquire).

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

* devel-2.6:
  http/__init__.py: Remove extraneous argument
  rapi.testutils: Add utility to format HTTP headers
  rapi.testutils: Return headers from mock utility
  http: Add wrapper for mimetools.Message
  Add missing tests for commit f0d22861

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

* stable-2.6:
  http/__init__.py: Remove extraneous argument
  rapi.testutils: Add utility to format HTTP headers
  rapi.testutils: Return headers from mock utility
  http: Add wrapper for mimetools.Message
  Add missing tests for commit f0d22861

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

This should be the last module rename, promise!

We rename this to conform to the other hierarchies (e.g. Query), and
to not have both Confd.hs and Confd/*.hs.

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

Also, adjust comment to $(genOpCode) block to avoid repetition of
"only".

Signed-off-by: Dato Simó <dato@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

In addition to ReqQueryTags in Luxi.hs, the TagObject ADT is also
required for the "kind" attribute of OpTagsSet and OpTagsDel, which
are coming to OpCodes.hs next. Hence, we move TagObject there, and
adjust imports accordingly.

Signed-off-by: Dato Simó <dato@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 mirrors the code in the Python code base, and is required for
clean error reporting during startup.

This patch implements the basic infrastructure; the confd daemon is
not yet modified to take advantage of this, just the types are
adjusted.

Also, the pipe-based error reporting will go in a future patch, once
we can actually use that for reporting.

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

This is only in master, so needed to be fixed separately.

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

Instead of having to explicitely increment the value (“… += 1”), a call
to next() is enough. These numbers should in no case be re-used (they
are used for ordering tasks). Using “itertools.count” is useful here as
it guarantees that a returned number won't be returned another time.
Manual code for this could, over the course of time, gain unintended
bugs.

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

* devel-2.6:
  Fix compatibility with newer Haskell libraries
  Fix gnt-instance console with xl

Conflicts:
        Makefile.am (reordering, fixed)
        htools/Ganeti/Confd/Server.hs (hlint fixes on master)
        htools/Ganeti/Daemon.hs (hlint)
        htools/Ganeti/HTools/Backend/Rapi.hs (hlint)
        htools/Ganeti/HTools/ExtLoader.hs (hlint)
        htools/Ganeti/HTools/QC.hs (file renamed/split in master, fixed)
        htools/test.hs (we don't use maxDiscards, ignored)
        lib/constants.py (move to pathutils)
        lib/hypervisor/hv_xen.py (move to pathutils)

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

* stable-2.6:
  Fix compatibility with newer Haskell libraries
  Fix gnt-instance console with xl

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