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>

This fixes an old TODO for moving these; now they can be reused in the
Haskell Query2 implementation.

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

I'd rather not introduce this typo in the Haskell code too, and also
not having them identical is not good.

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

Since we have now access to all cluster parameters, we can "fill" the
parameters, and can finally enable it in CLI/RAPI.

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

This patch adds the tags field to the objects which were missing it
(all except Cluster), implements handling the LuxiCall QueryTags, and
then enables the use of the query socket in cli.ListTags, used by all
commands, and in the RAPI client.

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

Currently, RAPI uses queries (as in QueryNode with field tags) for
getting the tags, whereas the CLI uses the QueryTags Luxi
command. Since this discrepancy is not good, let's switch RAPI to use
the same method as the CLI.

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

This implements the same query=True|False functionality as in
GetClient for cli.py, however since the RAPI code is much more
unit-tested (and the unit-test clients are mocked, for the most part,
without support for addresses) we have to do many adaptations in the
tests.

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

And expand cli.GetClient() to allow opening the query socket, instead
of the main master socket.

Finally, enable the query socket use in gnt-cluster version, since
that is already implemented fully in Queryd.hs/hconfd.

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

This is not used yet, but is helpful when developing the Luxi endpoint
for hconfd.

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

As timeout constants will now be used also on Haskell side, they have
been moved from rpc_defs and rpc to constants. The same is true for
connection timeout for connecting to nodes.

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

Debian Wheezy will ship with this version, and it has many improved checks compared to 0.6, so let's:

- bump version in the docs
- silence some new checks that are wrong due to our indent=2 instead of 4
- fix lots of errors in the code where the indentation was wrong by 1
  or 2 spaces
- fix a few cases of == True, False, None and replace with 'is'
- re-indent some cases where the code is OK, but pep8 complains

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

Since this is an inheritable parameter, having it as a MABYE_STRING
creates only problems (per our derivation rules). We change it to
STRING, with the default "", meaning no program. Note that most of the
code already accepts this as valid for "no program", and some comments
even say that this is the expected value.

We have some other parameters like this, I'll have to investigate
whether they need to be changed too. But right now I need this for the
hconfd changes (it's a prerequisite for them, I forgot to send it in
that patch series).

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: René Nussbaumer <rn@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>

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>

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>

This has been a long-standing cleanup item, which we've always
refrained from doing due to the high estimated effort needed.

In reality, it turned out that after some infrastructure improvements
(the previous patches), the actual job queue-related changes are quite
small.

We will need to update the NEWS file later, but so far the RAPI
documentation doesn't mention that the job ID is a string (it only
says it is "a number"), so it doesn't look like it needs update.

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

Currently, all the CLI helpers in qlang.py and cli.py assume that all
namefields are string, which leads to various breakage in case the're
actually not.

To improve the flexibility of the helpers, we add a bit of
infrastructure for accepting so called "numeric" namefields; this is a
bit of a hack, as a proper fix would actually add QFT_* support to the
helpers, and case for example the regex/globbing on QFT_TEXT, etc. But
that's left for (eventual) later improvement.

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

It wasn't clear for which tests we were ignoring the errors.

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

A while back, we did cleanup the code and ensured (manually) that use
of OpPrereqError includes an errors.ECODE_* field as second
argument. Since we cannot automate the check for this, it turns out
that more and more such usage has crept over the years, including in
the master code (the use on the CLI side is not as important).

Note that this also uncovered a few errors in ovf.py where the errors
messages were wrongly constructed.

Still looking for a way to automate this check…

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

The recreate-disks command no longer requires an explicit list of nodes.

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

This affects only the backend.

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

Ideally this should have used relocation mode, but to due lacking of
support in current allocators, allocation mode has been used instead.

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

_CheckInstanceNodeGroups behaves in the old way with the default value
of the new parameter.

Signed-off-by: Adam Ingrassia <api@google.com>
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Iustin Pop <iustin@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>

This removes "pass" from a few definitions, making all errors
consistent.

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

This adds supports for the (non-validated) cpu parameter to KVM.

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

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>

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>

This is not implemented currently.

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

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>

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>