When the queue was empty, the calculation for unchecked jobs while
archiving would return -1. ``last_touched`` is set to 0, the job ID list
(``all_job_ids``) is empty. Calculating ``len(all_job_ids) -
last_touched - 1`` resulted in -1.

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

With this change, there won't be unnecessary space characters
at the end of lines.

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

Before it would log something like “starting task
(<ganeti.http.client._HttpClientPendingRequest object at 0x2aaaad176790>,)”,
which isn't really useful for debugging. Now it'll log “[…]
<ganeti.http.client._HttpClientPendingRequest
req=<ganeti.http.client.HttpClientRequest 172.24.x.y:1811 PUT /node_info at
0x2aaaaab7ed10> at 0x2aaaaab823d0>”.

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>

This way it shows up in debug logs.

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

Having a proper name instead of just a number makes debugging
easier.

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

This patch fixes a race condition when acquiring all locks in
a LockSet instance. The list of lock names needs to be sorted
to guarantee a consistent locking order, but the names were not
sorted when acquiring all locks in the set.

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

Reading and comparing sorted lists is easier when debugging locking problems.

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: Guido Trotter <ultrotter@google.com>

In case an exception was thrown while acquiring the lock, not necessarily all
owned locks are also really acquired. Before this change, an exception could be
masked by another exception thrown here. There is no good clean-up strategy
when acquiring a lock fails with an exception in either case.

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

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

This change will normalize the MAC to all lower after validation.

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

This changes from submitting jobs to get the tags (in cli scripts) to
queries, which (since the tags query is a cheap one) should be much
faster.

The tags queries are already done without locks (in the generic query
paths for instances/nodes/cluster), so this shouldn't break tags query
via gnt-* list-tags.

On a small cluster, the runtime of gnt-cluster/gnt-instance list tags
more than halves; on a big cluster (with many MCs) I expect it to be
more than 5 times faster. The speed of the tags get is not the main
gain, it is eliminating a job when a simple query is enough.

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

Since the cluster name might be used for various purposes on nodes, we
should let all nodes "know" about a cluster rename by running the post
hook on all nodes. This will make cluster rename slightly
slower/costlier, but it is not/shouldn't be an operation that is run
very often.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

This allows automated builders to override the format, for example.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

In some cases pylint doesn't parse the import correctly, so we add
silences; but there are also many cases of unused imports, which we
simply remove.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

A piece of old code in bdev.py uses a for loop over a single variable
because we can 'break' out of the loop or exit on the 'else' path. This
is not a nice usage of the for loop, it should be converted to a
standard if...elif...else structure.

In the meantime we silence a warning from pylint (it is actually
invalid, IMHO) and add a TODO.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

Two variables are used in a strange way in ExecConfd. Until that is
clarified, add a TODO and a silence for the unused variable warning.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

There seems to be a bug in pylint relating to W0201 (“Attribute '%r'
defined outside __init__”) being re-enabled by simple comments and not
being able to be disabled again.

Until that is fixed, we disable this warning.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

R0922, “Abstract class is only referenced 1 times”, cannot be disabled
in the source code, and thus
lib/http/auth.py:HttpServerRequestAuthentication gives this warning
(it's actually also referenced from the tests, but we do not include
them).

Therefore we need to disable it at pylintrc level.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

Many of our functions have to follow a given API, and thus we have to
keep a given signature, but pylint doesn't understand this. Therefore,
we silence this warning.

The patch does a few other cleanups.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

This is a very slow checker, estimated to be O(n²) by its author; so
using it all the time is not good. It can be re-enabled on the command
line via “--disable-checker=”.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

The node_list argument to _DiagnoseByOS is not used, and is obsoleted by
the fact that the rlist argument already has the valid nodes as keys
(assuming RPC behaviour didn't change). Thus, we remove it and silence
the warning.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

Of all daemons, only rapi did abort when given argument. None of our
daemons use any arguments, but they accepted them blindly. This is a
very bad experience for the user.

This patch adds checking and exiting in all daemons, in a uniform way.
One other option would have been to add a flag to GenericMain
(noargs=True).

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

The disk template is not needed, all that's used is the disk data. As
such, remove this parameter from the function.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

Currently only the rpc call, but not its description (which also shows
the argument) is logged. We change this to log failmsg too, and this
also silences a warning.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

So we just silence the warning.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

The 'name' argument is not used anymore, probably since before 2.0.
Since this is an internal function, we can just remove it (from its
caller too).

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

Plus a silence for a wrong "uninitialized var".

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

Many methods are simple pure functions, and not depending on the object
state. We convert these to staticmethods.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

This removes unused variables in the rest of the code (outside lib/).

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

This patch should have only:

- pylint disables
- docstring changes
- whitespace changes

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

Similar to commit c881c5, we move the decorators out of classes, such
that they become simple functions instead of methods. This more clean,
since only the wrapped functions need to be methods/have access to
‘self’.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

'file' is a builtin keyword/type. Like many others, it should not be
used as a variable/argument name.

No code is actually passing in this argument so renaming it is simple.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

This runs pylint over all the python files. Yes, it takes a long while,
but it's the only way to properly analyze the source codes as only in
this way pylint can see all uses of the various modules/classes/etc.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

The chroot and fake hypervisors were missing:

- the powercycle node functionality
- proper handling of migration requests

The powercycle was just used as in the other hypervisors (use the
standard linux powercycle). The migration for chroot was disabled
explicitly, whereas for the fake one it was implemented to simulate
correctly. This required some work on the fake hypervisor, but now the
implementation of start/stop/etc. is much more clean.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

This adds targeted pylint disables, but since _autoconf.py is
autogenerated we need to do this in Makefile.am (hence the separate
patch).

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

We don't get any useful out of this - a git grep is as effective, and
these only pollute the pylint output.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>

This patch adds explicit errors (instead of notimplemented) in
FileStorage (and the associated TODOs).

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Olivier Tharan <olive@google.com>