GitLab

Since in objects we don't have access to utils.py, we add a warning that
the result value from objects.Disk.StaticDevPath might not be a valid
path, and change its only caller to validate the path.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This switches hv_kvm to PathJoin. There are still a few cases of direct
path construction, but those _should_ be safe.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This changes to hv_fake to PathJoin.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This patch abstract the computation of an instance's root directory into
a separate function (that uses PathJoin instead of "%s/%s").
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Currently we don't enforce name validation for the LVM backend, on the
idea that LVM itself will reject invalid names and we catch those
errors.

However, recent LVM documents the accepted VG/LV name space, so it's
easy to add this in the LVM backend code.

In addition, the patch changes some hardcoded /dev/ constructions with
utils.PathJoin().
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Rationale: the file-based storage backend can add/remove files under a
certain directory. However, the master node is also controlling the
setting of the file-based root directory, so basically it means we can't
prevent arbitrary modifications by the master of the node's filesystem.

In order to mitigate this for setups where the file-based storage is not
used, we introduce a new setting at ./configure time, that controls the
enable/disable of file-based storage. Since this is not modifiable by
the master (over RPC), it is now possible in this case to prevent
unintended modifications of the node's filesystem from the master.

The new setting is used in bdev.py to not expose the file-based storage
at all, and in cmdlib.py to prevent attempts at creation of such
instances.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This is a no-op change, but at least we concentrate the calls to path
joins into a single function.

A use in utils.FindFile is left as-is (don't want to raise exceptions
there, at least for now).
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

The various OS operations create log files in a specific directory
(constants.LOG_OS_DIR). The construction of the log names is however
spread and duplicated across multiple functions.

This patch abstracts this into a separate function that also validates
the log name, which should be safer in the long run. We also rename the
export logs from having a prefix of “exp” to “export”, since it was the
only operation that had an abbreviated prefix.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

For non-existing hooks (the majority of cases probably), logging a
warning every time is not helpful. So we first check if we have a valid
directory.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This passes a full burnin with lots of instances, and should be safe as
we mostly to join a known root (various constants) to a run-time
variable.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This will replace os.path.join since it is not safe for directory
traversal issues.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

In order to protect from accidental use of _CleanDirectory on a random
directory, we add a list of allowed clean directories, somewhat similar
to _ALLOWED_UPLOAD_FILES (but statically computed).
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Using absolute path for a privileged command is a bad idea as this path may vary.
For example /usr/sbin/brctl in Debian and /sbin/brctl in ALTLinux. Using $PATH is
a better idea.
Signed-off-by: Vitaly Kuznetsov <vitty@altlinux.ru>
Reviewed-by: Iustin Pop <iustin@google.com>
Signed-off-by: Iustin Pop <iustin@google.com>

This patch adds ability to customize ssh config directory with --with-ssh-config-dir
(instead of hardcoded /etc/ssh value). This is useful in Linux distributions with
custom ssh config directories (/etc/openssh in ALTLinux, for example).
Signed-off-by: Vitaly Kuznetsov <vitty@altlinux.ru>
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This avoids the need for them to be injected in the nbma repository.
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

The first can be problematic if poll(2) returns POLLHUP|POLLERR on a
socket. Before it would be only be respected for SOCKOP_RECV, but since
they can also occur on other socket operations, esp. in combination with
OpenSSL, letting the socket functions handle POLLHUP|POLLERR seems to be
the right thing.

The second is a typo leading to an endless loop if the first line of an
HTTP connection is empty (simply "\r\n"). Instead of removing the empty
line, it would remove anything after it.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

This is going to be used from the nbma repository, to ensure that the
nld daemon is running.
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Signed-off-by: Balazs Lecz <leczb@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

The first can be problematic if poll(2) returns POLLHUP|POLLERR on a
socket. Before it would be only be respected for SOCKOP_RECV, but since
they can also occur on other socket operations, esp. in combination with
OpenSSL, letting the socket functions handle POLLHUP|POLLERR seems to be
the right thing.

The second is a typo leading to an endless loop if the first line of an
HTTP connection is empty (simply "\r\n"). Instead of removing the empty
line, it would remove anything after it.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

LUQueryConfigValues supports multiple output fields. If the client asked
for the watcher pause status, it would not get a list, but simply the
value.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

These hooks are run on all nodes, after the "base" daemons are started.
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

The first argument to _ErrorIf should always be True in this case.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

This function is a generic pythonic version of runparts. We currently
use it in the backend HooksRunner, but we'll use it for running
different directories as well.
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

And save lots of lines of code, in the process
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This allows to run a command with only the passed in environment, rather
than just updating the default one with it.

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

Signed-off-by: René Nussbaumer <rn@google.com>
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Jobs submitted via the standard command line utilities didn't give any
indication that anything is happening while they were waiting in the job
queue (e.g. due to other jobs using all worker threads) or acquiring
locks. This could be very confusing for people not familiar with Ganeti's
architecture. Now they'll show a message after the first WaitForJobChanges
timeout.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

If too many clients try to connect to the master at the same time, some of
them might fail if the master doesn't accept the connections fast enough.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

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

We add this as a new opcode since we don't want to alter the behaviour
of current opcodes/lus.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

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

This is a new mode that request a solution for the evacuation of
multiple nodes. The external script will be fed a list of names, and is
expected to return a list of [instance, new_node(s)] lists, detailing
the evacuation path of each instance.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This patch switches the default result key from 'nodes' to 'result'. The
old name is still accepted for backwards-compatiblity, and should be
removed in later versions.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Currently the 'name' parameter in the constructor is required (as a
non-keyword argument). Since the (to follow) node evac IAllocator mode
doesn't have 'name' as a valid argument, we're moving this one into the
per-request key, leaving the constructor required arguments more
abstract.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This moves the setting of the request member on the in_data, of the
request type, and of the branching basef on request type outside of
individual functions and directly into the constructor.

Since the values we're using externally are identical to the
constants.py values, we're also using those directly.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

Until now this was only done for the master node, though
the problem originally fixed in 8f215968

 also occurs for
other node daemons.
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 function could be useful in other places and this
way we can easily unittest it.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

On fork, the tempfile module's pseudo random generator is
not reset. If several processes (e.g. two children or parent
and child) try to create a temporary file, they'll conflict.
This function can be used to reset the name generator which
contains the pseudo random generator.

A unittest is included. It is in a separate script because
it changes a variable in the tempfile module to speed up the
test.
Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

In case we add a node with “--no-ssh-key-check”, this should override
any default yes/ask values in the system-wide (or user) ssh key check.

Currently this only works in batch mode, whereas in non-batch we only
override a 'no'. The patch fixes SshRunner such that in non-batch mode
we enforce the value of StrictHostKeyChecking in all cases.

Bug found and initial investigation by Theo Van Dinter.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>