This addresses Issue 237.

Mirroring no disk is a no-op. As such we can treat them like mirrored
instances, since the data they need (none) will be present on all nodes.

This is definitely enough to failover or migrate instances with a manual
target node. Further work might be needed for support in hail.

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

This fixes issue 396.

- Fix a wrong comment that mentions drbd8 when actually the code acts
  only on externally mirrored instances.
- Fix a wrong assert that requires failover/migrate to acquire the NAL
  on externally mirrored instances: this is the case only when a
  iallocator is used, not if a node is manually specified.

Extra qa should be added, but this can only be done if an externally
mirrored instance or a diskless instance is tested. It is easier to add
it on the master branch, where we have diskless instances support.

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

When instance policy is violated, creation fails.

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

Violations on policy changes are checked.

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

qa_cluster.TestClusterSetISpecs() is exported as it will be used in future
tests.

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

Tests for:
  objects.InstancePolicy.CheckParameterSyntax()
  objects.InstancePolicy.CheckDiskTemplates()
  objects.InstancePolicy.CheckISpecSyntax()

Instance policies with an empty disk-template list now are reported.

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

IPOLICY_DEFAULTS is now a legal policy (the disk-templates entry was a set
instead of a list, before).

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

Unknown elements were silently removed on startup. This means that a
software upgrade could result in lost configuration information if
cfgupgrade wasn't run promptly.

Added unit test for Cluster.UpgradeConfig() to cover this case.

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

Policy violations of back-end parameters that used the cluster default
value were not reported in cluster-verify.

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

"default" was not accepted as a valid input value for instance specs in
group policies, due to a bug introduced in 2cc673a3 (and released with
2.6.0). Added QA for this and another similar case.

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

Not 100% coverage, though.

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

Instance disk template is checked against the policy, and diskless
instances aren't checked for the number of disks.

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

A fragment in LUInstanceCreate.CheckPrereq() removed in commit ba147ff8 was
reintroduced in merge 8e09e801 due to a change in df28c49b.

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

There was a typo which prevented the correct option from being passed to
RAPI

Signed-off-by: Daniel Krambrock <danielk_lists@z9d.de>
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Currently, hbal exits with status 1 if early exit is requested, even
when all jobs are successful. This is counter-intuitive behaviour, so
let's fix it (Issue 386).

Note that the man page had conflicting information already, so it's a
good thing to clean this up.

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

When an OpCode's parameter has a tuple as default value, this code
will break:

  buf.write("defaults to ``%s``" % default)

The patch fixes this and other potential cases by always passing a
tuple to '%'.

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

Currently the man page build rule is run without ‘-e’, so the actual
checks in there are just warnings (check-man-warnings,
check-man-dashes).

The patch fixes this and also fixes the man pages which made me see the
problem.

Additionally, check-man-dashes is now verbose, otherwise it's hard to
find out actually where in the page the error is.

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

Currently we only symlink this into two hard-coded directories, which
breaks the source code viewing for all the others.

The patch replaces this with automatic building; as playing with
relative symlinks is non-trivial, I just generate it many times (the
file is small enough (<250) that it shouldn't matter, size-wise, and
it takes 2ms to do it).

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

This should help a lot of users, since pylint 0.21.1 was quite an old
version…

The tree is "clean" w.r.t. the new pylint version.

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

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

Currently the Python source constants are used as-is, which is not
nice on the eyes (especially for unqualified names, it was hard to
separate them visually).

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

This patch does multiple enhancements to the way we build the Haddock
docs, motivated by the fact that if we don't run Haddock over test
files as well, bad formatting can be submitted and will accumulate
over time (without any checks).

The patch does:

- replace manual built ALL_APIDOC_HS_DIRS with automatically built one
  (from HS_DIRS)
- change Haddock so that it runs from the top directory (instead of
  from src/)
- change HsColour target file to be built via bash parameter
  substitution, rather than sed (I don't know how to do it in one go,
  so I use 2 intermediate variables)
- change 'hs-apidoc' target so that it depends on the real target
  file; in case no source file has been modified, running 'make
  hs-apidoc' twice will not result in two runs
- run HsColour/Haddock under en_US.UTF-8 locale, otherwise they can't
  parse correctly the Unicode chars in the test files

Additionally, wrong formatting (oops) in a test file has been
corrected.

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

Two test helper files didn't have export lists (lost during the split
of tests). This patch finally reintroduces them, to hopefully make the
export list saner and with fewer changes for purely-internal changes.

Additionally, a few missing docstrings are added as well.

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

When file storage is disabled at ./configure time, we shouldn't pass
opcodes containing DTFile/DTSharedFile/StorageFile to Python for
validation, as they will fail.

This patch implements this by simply tweaking the Arbitrary instances
for DiskTemplate and StorageType (which IMHO is a nice and clean
way!), and also fixing the generation of arbitrary IPolicies to use
the correct 'allDiskTemplates' list (otherwise we'd loop forever
trying to generate a list of all templates).

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

Currently QA tries to use “gnt-node list-storage” with all storage
types, but when file storage is disabled this will fail.

The patch changes it so that if file storage is disabled, we test that
indeed fails, and otherwise we include it in the regular tests.

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

It turns out that, in Python, booleans are also integers. So they fall
under the “isinstance(value, int)” case, resulting in all enable*
constants being integers in Haskell, which is not nice, even though
we're not using them directly today.

Patch simply adds a special casing for booleans, before integers.

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

The vcluster changes broke the disabling of file storage; we can
workaround by (manually) skipping the virtualisation of file storage
paths if they are not enabled.

Note that tests/QA are still broken with disabled file storage; this
patch only fixes production code.

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

In a few cases, we tested the verbosity level for (== 0), instead of
higher/lower than a certain value. If the user passes multiple
"--quiet" options, this can result in negative verbosity levels, which
behave like "extra verbosity".

Sigh for plain data types as opposed to proper verbosity levels…

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

Change the docstring of chompPrefix to prevent the error
"doc comment parse failed" that was raised by some version of
haddock while generating the documentation for this function.

Signed-off-by: Michele Tartara <mtartara@google.com>
Reviewed-by: Helga Velroyen <helgav@google.com>
(cherry picked from commit 94042ae4)

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

Currently, the iallocator interface requires LVM storage, due to the
way it computes the node storage information.

By changing the code to understand that GetVGName() can return None,
and by setting the disk_total/disk_free node parameters to the value
zero, we can workaround this issue until proper multi-storage
reporting is implemented.

The hail iallocator plugin works without changes once this is
implemented, as it already handles non-LVM storage as "foreign"
storage (with unlimited capacity).

The patch also changes a bit how the parameters are retrieved and
used; hopefully this is a bit safer and more readable.

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

'MakeLegacyNodeInfo' is not the best place for this, but we'd have to
duplicate it if we wanted a LVM-less version, so the easiest is to add
an optional parameter that allows it to accept/skip LVM-less results.

It still requires at most one VG result, so its behaviour isn't
changed in this respect.

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

This is similar to commit 8775e62a; the addition of node_whitelist
broke this LU as well.

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

If cluster is initialized with --no-lvm-storage then volume_group_name
does not exist in config.data. Thus we must define it as optional in
confd.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
[iustin@google.com: fixed Haskell RPC definition]
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Passing --net 0:add,ip=5.5.5.5 failed due to a reference
of a non initialized variable (new_net_obj). Reorder the checks
and add some comments to be readable.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Thanks Michael for pointing out the error!

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

The document doesn't mention which certificates need to be passed as
ca-file, so let's try to clarify that.

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

Remove any early returns in LUNetworkDisconnect/LUNetworkConnect
and replace them with if-else statements.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

In this method self.tags might be None and cannot be used
in join(). Use GetTags instead.

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Guido Trotter <ultrotter@google.com>

networks config objects have UUIDs and thus should be included
in _AllUUIDObjects().

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Locks for group instances are aquired only if conflicts are checked.
To this end we must _CheckNodeGroupInstances() only then otherwise
this check will always fail (owned_instances will be []).

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
Reviewed-by: Guido Trotter <ultrotter@google.com>