Group.hs: add 'allTags'; adjust loaders and test data for it

This commit adds a Group.allTags field to store the tags of node groups,
and teaches each loader backend in HTools to populate it (additionally, the
IAllocator class in lib/cmdlib.py now includes tags for groups too). Test
data is updated to include an empty set of tags for node groups in all
affected test cases.

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

Instance.hs: rename 'tags' to 'exclTags', provide 'allTags'

The mergeData function in Loader.hs included a step to filter an instance's
tags to include only the exclusion tags (as specified via the commandline,
or cluster-level tags). Later on, code in Node.hs assumed Instance.tags to
contain only tags to be used for exclusion.

Because in the future we will need to access the full list of an instance's
tags (and not only exclusion tags), this commits deprecates the 'tags'
field, and introduces Instance.exclTags and Instance.allTags.

Instance.allTags is now populated from the different backends (Text, Luxi,
Rapi, etc.), and Instance.exclTags is only populated from Loader.mergeData,
as was done previously. This means that loading tags from e.g. Text or Simu
and assuming that they'll be used as exclusion tags without going through
Loader.hs will no longer work; but this was already the case with other
fields, and 'mergeData' or 'loadExternalData' continue to be the only entry
points to get a consistent view of the cluster. (Additionally, there were
no tests that made this assumption that I could find.)

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

htools-excl.test: add test case for exclusion tags in hbal

In preparation for future modifications in the exclusion tags field, add a
test that verifies that exclusion tags are being honored: in a test cluster
with two instances of the same exclusion group in each node, hbal should
shuffle instances around to improve the score.

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

ensure-dirs: Fix permissions on master socket

A socket shouldn't have its executable bit set.

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

Update security document for version 2.6

Quite some things were out of date. Some formatting was also updated.

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

Merge branch 'stable-2.6' into devel-2.6

* stable-2.6:
  Update NEWS and bump version to 2.6.1

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

Update NEWS and bump version to 2.6.1

This is a small bug-fix only release.

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

Text.hs: update field lists in parseData comments

The comments in parseData had become out of date with the implementations
of load{Group,Node,Inst}. This commit updates the field list in comments to
match the implementations.

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

Merge branch 'stable-2.6' into devel-2.6

* stable-2.6:
  verify-disks: Explicitely state nothing has to be done
  Add list of design documents implemented in version 2.6
  Better list of replace-disks arguments + typos fixed
  jqueue: Look at archived jobs when watching
  Show old primary/secondary node on disk replacement
  gnt-instance reinstall: Don't always exit with success
  LUClusterVerify: Ignore /proc/drbd if DRBD is disabled
  Fixed typos in devnotes.rst
  Always_failover doesn't require --allow-failover anymore
  bash_completion: Enable extglob while parsing file
  rpc: Remove duplicated logic, fix unittests
  Annotate disk params on instance_start
  cmdlib: Handle locking.ALL_SET correctly when copying locks

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

verify-disks: Explicitely state nothing has to be done

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>

Add list of design documents implemented in version 2.6

Each version should have its dedicated list.

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

Better list of replace-disks arguments + typos fixed

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>

jqueue: Look at archived jobs when watching

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 045694691691af1b10d1d055bdac55cf205e47de)

Show old primary/secondary node on disk replacement

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>

gnt-instance reinstall: Don't always exit with success

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>

LUClusterVerify: Ignore /proc/drbd if DRBD is disabled

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 3d8ae32740475f35ac2e7a4937f5469700202e5b)

Fixed typos in devnotes.rst

Signed-off-by: Gintautas Miliauskas <gintas@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Always_failover doesn't require --allow-failover anymore

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 b5f0b5cc16dd3221df06e2fc1353b048eef17d06)

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

bash_completion: Enable extglob while parsing file

In older versions of GNU Bash extended patterns, such as “@(…)”, are only
available with the “extglob” shell option. As pointed out in [1] and [2],
“extglob” must be enabled while parsing the code. Therefore the flag must be
enabled at the beginning of the script and be reset to its original value at
the end as to not interfere with other code on shell initialization.

[1] http://unix.stackexchange.com/questions/45957
[2] http://mywiki.wooledge.org/glob

Reported by Sascha Lucas.

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

rpc: Remove duplicated logic, fix unittests

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>

Annotate disk params on instance_start

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

See also commit b8291e0.

This was reported by Ansgar and Damien.

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

cmdlib: Handle locking.ALL_SET correctly when copying locks

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>

Merge branch 'stable-2.6' into devel-2.6

* stable-2.6:
  Fix bug in non-mirrored instance allocation
  Fix gnt-debug iallocator

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Fix bug in non-mirrored instance allocation

The function `allocateOnSingle' has a bug in the calculation of the
cluster score used for deciding which of the many target nodes to use
in placing the instance: it uses the original node list for the score
calculation.

Due to this, since the original node list is the same for all target
nodes, it means that basically `allocateOnSingle' returns the same
score, no matter the target node, and hence the choosing of the node
is arbitrary, instead of being done on the basis of the algorithm.

This has gone uncaught until reported because the unittests only test
1 allocation at a time on an empty cluster, and do not check the
consistency of the score. I'll send separate patches on the master
branch for adding more checks to prevent this in the future.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Fix gnt-debug iallocator

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>

Merge branch 'stable-2.6' into devel-2.6

* stable-2.6:
  Fix warnings/errors with newer pylint
  Fix decorator uses which crash newer pylint

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

Fix warnings/errors with newer pylint

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>

Fix decorator uses which crash newer pylint

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>

Instance autorepair design

This design describes a tool that will perform automatic repairs on
instances when they are detected to be unhealthy (living on offline or
drained nodes, at the moment). These repairs can be scheduled
automatically or requested as a one-off by a tool or person.

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

Merge branch 'stable-2.6' into devel-2.6

* stable-2.6:
  Make stable-2.6 compatible with newer pep8
  Fix computation of disk sizes in _ComputeDiskSize
  Add verification of RPC results in _WipeDisks
  Add test for checking that all gnt-* subcommands run OK
  Fix double use of PRIORITY_OPT in gnt-node migrate
  Add new Makefile target to rebuild the whole dist

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

Make stable-2.6 compatible with newer pep8

This is done so that all current branches can run with newer pep8;
note that instead of fixing the problems (like I did on master), I've
just silenced more. These should *not* be merged onto master!

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

Fix computation of disk sizes in _ComputeDiskSize

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>

Add verification of RPC results in _WipeDisks

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>

Add test for checking that all gnt-* subcommands run OK

This is a bit of a shell munging trickery, but works for now. Making
it more generic can be done later.

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

Fix double use of PRIORITY_OPT in gnt-node migrate

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>

Add new Makefile target to rebuild the whole dist

Due to the fact how the automake system works it doesn't rebuild already
prebuild files in distcheck. This lead to a bug, where a rebuild of the
documentation was failing because we missed the fact that the files were
missing from the archive.

By adding distrebuildcheck we workaround that issue by running a
maintainer-clean which also removes prebuild files.

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

rapi client: accept arbitrary shutdown arguments

The "ignore_offline_nodes" parameter is unsupported. Rather than
explicitely adding it, just pass all keyword arguments in the body of
the query, and rapi on the other side will do the right thing.

Supports for old arguments that were passed via the query is unchanged.

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

Handle offline nodes for "instance down" checks

When offlining an instance because its primary node is down, we must be
able to cope with the situation.

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

Add missing rst files to Makefile.am

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

Release version 2.6.0 (final)

Phew, it wasn't easy, but…

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

Fix 'explicitely' common typo

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>

Fix issue in LUClusterVerifyGroup with multi-group clusters

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>

Add QA test for node group modification of ndparams

Additionally, we run hbal if the htools tests are enabled (new
key). Note that hbal will directly exit, since there are no instances,
but at least it will load and parse the group details.

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

Fix node group modification of node parameters

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>

Merge commit 'v2.5.2' into stable-2.6

* commit 'v2.5.2':
  Fix RST formatting in NEWS file
  Update NEWS and bump version for release 2.5.2
  Fix boot=on flag for CDROMs
  KVM: only pass boot flag once

Conflicts:
        NEWS         (trivial, merged the entries)
        configure.ac (trivial, kept ours)

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

Fix RST formatting in NEWS file

*FileID is interpreted as a start-of-bold, without corresponding end
marker. Sorry for not catching it in the first place.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Balazs Lecz <leczb@google.com>

Update NEWS and bump version for release 2.5.2

This is a trivial release, with just bug fixes, so a small change log.

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

Fix boot=on flag for CDROMs

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>

KVM: only pass boot flag once

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>

Ensure a stable content of the bash completion file

Currently, the order of commands in the bash completion file is
random, because the sub-commands are not sorted. This makes it harder
to investigate the differences in packaged Ganeti or in installed
Ganeti, since chunks in this file will have a random order.

To fix this, we sort the subcommands based on the first subcommand in
a given group ('first' also in sorted order); this results in a stable
contents of file, as tested by building it many times and checking for
differences.

The patch also does a few other minor changes to the file
(e.g. updating copyright years, etc.).

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

Release Ganeti 2.6.0 rc4

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

Prepare NEWS for Ganeti 2.6.0rc4

Another release candidate is coming.

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

Add some rudimentary node group ipolicy checks

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

Fix setting ipolicy on node groups

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>

Fix --no-headers for the new list-drbd command

Sorry, I forgot that with GenerateTable this needs to be handled
manually. Fixed now and tested in both ways.

(But to be honest, this should be abstracted in GenerateTable, instead
of the 'if' test in all its callers.)

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

Add a simple QA test for gnt-node list-drbd

This will be run only if 'haskell-confd' QA configuration has been
enabled.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Add a new gnt-node command list-drbd

This uses confd to query the DRBD minors, which is very special; no
other command currently does so.

Since the backend is only implemented in the Haskell version of confd,
we have checks that 1) confd is enable, and 2) hs confd is also
enabled. If by mistake people do manage to query Python confd, the
error message will be clean:

  Query gave non-ok status '2': not implemented

So nothing breaks in an "ugly" way.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Implement a node to drbd minors query function

This can be queried remotely since it's a pure configuration query.

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

Add a new unused confd query

This is not implemented currently.

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

Add support for computing instance all/secondary nodes

This fixes an old FIXME. Since we now how actual DRBD configuration
data, we can finally compute the instance's secondary nodes.

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

Add disk logical ID support in Objects.hs

This is a complex field, so we have to do a lot of manual work for now.

The complexity arises from the fact that the contents of the field,
and the way to parse it, depends on the disk type field, so we don't
have a single, static way of parsing it. Hence we needed the
extensions to the Template Haskell code.

Since we now can both load and save the disk type, we can remove the
in-memory (duplicate) disk type from the disk objects, relying only on
the logical ID to hold the type information.

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

Fix a docstring in bdev's DRBD8 class

It seems this was not updated since the move to static minors…

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Extend the Template Haskell loadFn model

Currently, we only allow field-by-field de-serialisation. Since we
have cases where information about how to un-serialise a field is
split across two JSON fields (e.g. disk type and disk logical_id,
hypervisor and hvparams, etc.), we need to pass the entire object to
custom read functions.

Furthermore, since we will have to generate two actual fields from the
single in-memory field, we need to extend the custom save function so
that they can generate additional fields beyond the "main" field value
they currently generate.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Change how customFields are built

Instead of passing an expression (which cannot come from the current
module), we pass a name, which is allowed to reference functions from
the module we're in. Since we currently don't have custom fields, we
don't need to modify any callers.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Implement lookup of partial names in Config.hs

This uses the recently-moved functions to implement partial lookup of
names on getNode and getInstance, similar to the Python codebase.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Reorganise the lookup functions

Currently, the LookupResult, MatchPriority and related functions are
locate in Loader.hs, since (so far) only hbal needs them in the
selection of instances. However, with the new functionality on confd
side, we need these functions there too, but we don't want to import
Loader.hs (which pulls in lots of balancing-related code). So we move
all these function to BasicTypes.hs, since that module is a leaf one,
with no other dependencies.

Unittests are slightly adjusted (but they are still tested under the
'Loader' group).

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Remove an unused function

This is not used, as we need a more complex serialisation, which is
done in the saveObjectField function.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Ensure that disk.params is always defined (and a dict)

Commit cce46164 fixed upgrading from other 2.6 versions, but
accidentally broke upgrading from 2.5 (disk.params was left as None,
which breaks FillDict). The simplest way to handle params is to always
set them to an empty dict (disregarding what they currently contain).

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

Another small consistency fix with if branches

While looking at the testability of this piece of code, I found another
consistency problem. We have two if branches instead of one, with
elif's.

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

Fix inconsistency in the LUXI protocol w.r.t. args

This inconsistency was found during rebalancing. Hbal failed because,
Ganeti couldn't load the opcode. After digging through the cause, an
inconsistency with the "args" field in the LUXI protocol was triggered
by the TemplateHaskell side where it's done uniformed.

For SubmitJob and SubmitManyJobs we treat args as one argument,
containing the job definition. In every other LUXI call args is actually
a list of arguments. This patch fixes this consistency.

This change is NOT backwards compatible.

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

Fix UpgradeConfig of Disk object regards disk params

This bug was found during disk parameter debugging. While looking at the
config some values seem present on the disk parameters, but that's not
expected behaviour. This patch fixes this, and also fixes the "broken"
configs automatically upon masterd restart.

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

Make Ganeti 2.6.0 rc3 release

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

Allow reinstall even when secondaries are offline

Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Prepare NEWS file for Ganeti 2.6.0 rc3

Update NEWS regarding rc3 fixes and document changes in behaviour.

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

QA updated to test instance removing when sec. is offline

Added test checking that it is possible to remove an instance with drbd
disk when non-primary node is offline.

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

Ignore offline node errors when removing disks

When we delete DRBD disks from some instance, we do not want to get
errors due to nodes other than that instance's primary being offline.

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

Allow instance disc activation with offline secondaries

Currently, this is not allowed, so one can't run a replace-disks; this
breaks any non-invasive method of recovering the redundancy of the
instance if its disks are already stopped (but it still works if the
disks on the primary are active). So let's fix this inconsistency.

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

RAPI regression beparams/memory fix

For compatibility with the old Ganeti version, we want to keep the
beparams/memory field around for another release. This patch fixes this
regression.

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

Fix redistribution of files w.r.t. offline nodes

Currently, _RedistributeAncillaryFiles computes two lists: the list of
online nodes (for all files redistribution), and the list of
vm_capable nodes, for hypervisor-specific files. However, the
vm_capable list includes offline nodes too, leading to warning
messages:

  WARNING: Copy of file /etc/xen/xend-config.sxp to node node13.example.com failed: Node is marked offline

We fix this by trivially intersecting the vm_capable list with the
online one.

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

Fix cluster verify error on master-ip-setup script

This error does not show up until we exceed the pool of master
candidates and have nodes which are not master candidates.

The background is that we check for master-ip-setup script on master
candidates and expect them not to be on the other nodes. However, we
distribute a default master-ip-script which break this assumption.
Furthermore, there's no reason why the file should just exists on the
master candidates.

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

Add a Makefile rule to test unclean git status

This is trivial, but can be used easily from automated builds to check
that the git tree is clean: files are not modified and untracked files
are not present.

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

Add test helpers to .gitignore

Commit 3096524e enabled building of test helpers, but forgot to add
them to .gitignore, resulting in "unclean" git status output.

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

Fix a typo in NEWS

Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: Bernardo Dal Seno <bdalseno@google.com>

Release Ganeti 2.6.0rc2

Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Prepare NEWS file for 2.6.0 rc2

We are about to release another version of Ganeti 2.6, so we prepare the
NEWS file with the changes from rc1 to rc2.

Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Update time on Haskell binaries after build

This patch works around an issue in our build system. Since we don't
use cabal or 'ghc -M', we don't track actual dependencies in our
Makefile; this in turn means that editing a file that only is used in
the main 'htools' binary will keep triggering 'ghc --make' for the
hconfd binary; and this actually takes a bit of time to look at all
the dependencies.

So to work around this, we add a touch after each ghc --make, such
that we trigger the unneeded build at most once.

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

hcheck: add two simple type aliases for readability

The same types are reused a couple of times, so let's add a couple of
type aliases for easier change later and readability.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

hcheck: rework output mode

Looking at the output of hcheck, in human readable mode, it looks like
it always starts with a blank line. This is not nice, so I wanted to
redo this to start cleanly.

However, looking at the code, I realised that we need some internal
cleanup; 'perGroupChecks' was doing both pure computation (computing
the score/stats) and I/O work (printing the stats), which is ugly, so
I split that in two; this allows perGroupChecks to be simpler, and we
can therefore first compute the group stats, and start the output with
the mode in which we run.

So right now, hcheck will first output whether we need to run a
rebalance, or if no-simulation is selected, etc., and only then
display the actual stats, making the output a bit more consistent.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

hcheck: reword and fix typo in instance/offline msg

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Add hlint warning for wrong use of 'length'

While removing split instances check from hcheck, I saw a wrong use of
'length': this should only be used when one needs the actual length of
the list, and not when one wants to check whether the list is empty or
not (in that case, 'null' should be used).

To prevent this in the future, let's add a hlint hint for it, and fix
the warning it generates in hbal.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Remove can_rebalance functionality from hcheck

Since hbal can now run rebalances even in the presence of split
instances, we can remove this check and always run the rebalance.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Fix building of hcheck man page

The code used a space, not a tab, in Makefile.am, which means it
actually broke the variable declaring the man pages.

Furthermore, we add some blank lines in the man page, otherwise the
paragraphs will be run-in together.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Fix tiny typo in htools.rst

Backtracking through git blame, it turns out than commit 49148d15
changed "\fB20,102400,16384,4\fR" into "**B20,102400,16384,4**". But
of course that's a mistake, since the markup was \fB, not just \f.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Allow param `modify' during gnt-instance modify

With the new gnt-instance modify syntax for addition and removal of
disks/NICs on arbitrary indexes, we hit an assertion if the user
passes `modify' as one of the disk's parameters. E.g::

 gnt-instance modify --disk 2:modify,size=3G instance1
 gnt-instance modify --disk 3:add,size=1G,modify instance2

This patch fixes the bug, by allowing `modify' to be passed as a
parameter (as happens with `add' and `remove'), as long as it is
not done alongside `add' or `remove'. If so, it is treated in the
same way as if none of modify/add/remove is passed --> modify.

Signed-off-by: Constantinos Venetsanopoulos <cven@grnet.gr>
Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Correct a wrong --help option for --simulate

This was not update when the alloc policy was added to the parameter.

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

Add a shell test for hbal and split instances

This is not perfect, as we only test that hbal completes successfully
and that it show a score improvement, but it's better than nothing.

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

hbal: relax restrictions on split instances

The recent patch series on hcheck made me realise how much pain we
have due to hbal not being able to work (at all) with split instances.

This is sub-optimal; ideally, hbal would degrade its behaviour, but
not completely refuse to work. This patch does this, by marking any
split instances as not movable (since such instances most likely mean
that some manual intervention is done on then), but otherwise
proceeding with the rebalance of the selected node group.

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

Add newline at the end of shelltest files

Signed-off-by: Agata Murawska <agatamurawska@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

Add forgotten unittest changes for instance_os_add

The previous patch which fixed disk parameters didn't adapt the
unittests so it lead to failing QA.

Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: Agata Murawska <agatamurawska@google.com>

Annotate disk params on instance_os_add

We call _OpenRealBD during the process and this needs disk parameters to
work. This was reported by Constantinos.

The fix is very ugly though.

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