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

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

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

Commit f396ad8c returns the TCP port used by DRBD disk back to the
TCP/UDP port pool using AddTcpUdpPort().

However, AddTcpUdpPort() writes the config on every invocation,
using _WriteConfig(). This causes two problems:

 * it causes critical errors logged by VerifyConfig(), after the DRBD
   disk removal, and until the actual instance removal.
 * if the code following AddTcpUdpPort() fails, the port is already
   returned back the pool, which causes the port to have duplicates
   (inconsistent config).

AddTcpUdpPort() is invoked in three cases:

 * during InstanceRemove() through _RemoveDisks().
 * during InstanceSetParams() in case of disk removal.
 * during InstanceSetParams() through _ConvertDrbdToPlain().

This commit fixes the problem by removing the _WriteConfig() call from
AddTcpUdpPort(), delegate it to Update() via the
TemporaryReservationManager and ensure AddTcpUdpPort() precedes
Update().

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
[iustin@google.com: small comments adjustements]
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
(cherry picked from commit 3b3b1bca)

Commit f396ad8c returns the TCP port used by DRBD disk back to the
TCP/UDP port pool using AddTcpUdpPort().

However, AddTcpUdpPort() writes the config on every invocation,
using _WriteConfig(). This causes two problems:

 * it causes critical errors logged by VerifyConfig(), after the DRBD
   disk removal, and until the actual instance removal.
 * if the code following AddTcpUdpPort() fails, the port is already
   returned back the pool, which causes the port to have duplicates
   (inconsistent config).

AddTcpUdpPort() is invoked in three cases:

 * during InstanceRemove() through _RemoveDisks().
 * during InstanceSetParams() in case of disk removal.
 * during InstanceSetParams() through _ConvertDrbdToPlain().

This commit fixes the problem by removing the _WriteConfig() call from
AddTcpUdpPort(), delegate it to Update() via the
TemporaryReservationManager and ensure AddTcpUdpPort() precedes
Update().

Signed-off-by: Dimitris Aragiorgis <dimara@grnet.gr>
[iustin@google.com: small comments adjustements]
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

This requires acquiring the node group locks in shared mode.

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

In at least two cases "%s" is replaced with str(), too.

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

This check verifies “iv_name” of all instance disks. If one is wrong
(which shouldn't happen in the first place), cluster verification will
warn:

“ERROR: cluster: Instance 'inst.example.com' has wrongly named disks:
name of disk 1 should be 'disk/1', but is 'disk/4'”

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

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

Stop using the disk index encoded in “iv_name” when renaming an instance.

This patch also changes the code to operate on a copy of the instance
until the major changes have been applied. In the case of a failure we
won't loose the instance object anymore.

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

This adds the new parameter and changes the validation routines to
handle IPOLICY_PARAMETER value as floats. Very very ugly code :(,
should be redone much cleaner.

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

As opposed to the initial situation, the PARAMETERS will be more than
just the min/std/max ispecs. Let's rename it, before re-adding the
PARAMETERS constant with a different meaning.

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

This is a bit more complex patch, as it requires changing the
assumption that all keys in the policy dict points to values that are
themselves dicts. Right now we introduce an assumption that any
non-dicts are lists, we'll see in the future if this holds or whether
we need more complex type checking (manual, yay Python).

The patch also does some trivial style changes.

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

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

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

The parameter is called “mods”, not “modes”.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Andrea Spadaccini <spadaccio@google.com>
(cherry picked from commit 1730d4a1)

The parameter is called “mods”, not “modes”.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Andrea Spadaccini <spadaccio@google.com>

Note: This bug only manifests itself in Ganeti 2.5, but since the
problematic code also exists in 2.4, I decided to fix it there.

If a node was assigned to a new group using “gnt-group assign-nodes” the
node object's group would be changed, but not the duplicate member list
in the group object. The latter is an optimization to require fewer
locks for other operations. The per-group member list is only kept in
memory and not written to disk.

Ganeti 2.5 starts to make use of the data kept in the per-group member
list and consequently fails when it is out of date. The following
commands can be used to reproduce the issue in 2.5 (in 2.4 the issue was
confirmed using additional logging):

  $ gnt-group add foo
  $ gnt-group assign-nodes foo $(gnt-node list --no-header -o name)
  $ gnt-cluster verify  # Fails with KeyError

This patch moves the code modifying node and group objects into
“config.ConfigWriter” to do the complete operation under the config
lock, and also to avoid making use of side-effects of modifying objects
without calling “ConfigWriter.Update”. A unittest is included.

Signed-off-by: Michael Hanselmann <hansmi@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>
(cherry picked from commit 218f4c3d)

Note: This bug only manifests itself in Ganeti 2.5, but since the
problematic code also exists in 2.4, I decided to fix it there.

If a node was assigned to a new group using “gnt-group assign-nodes” the
node object's group would be changed, but not the duplicate member list
in the group object. The latter is an optimization to require fewer
locks for other operations. The per-group member list is only kept in
memory and not written to disk.

Ganeti 2.5 starts to make use of the data kept in the per-group member
list and consequently fails when it is out of date. The following
commands can be used to reproduce the issue in 2.5 (in 2.4 the issue was
confirmed using additional logging):

  $ gnt-group add foo
  $ gnt-group assign-nodes foo $(gnt-node list --no-header -o name)
  $ gnt-cluster verify  # Fails with KeyError

This patch moves the code modifying node and group objects into
“config.ConfigWriter” to do the complete operation under the config
lock, and also to avoid making use of side-effects of modifying objects
without calling “ConfigWriter.Update”. A unittest is included.

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

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

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

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

Add the use_external_mip_script cluster parameter, that represents
whether the master IP address turnup/turndown procedures must use a
script provided by the user (True) or the one provided by Ganeti
(False).

Signed-off-by: Andrea Spadaccini <spadaccio@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Ensure ports previously allocated by calling ConfigWriter's AllocatePort() are
returned to the pool of free ports when no longer needed:

 * Return the network_port of an instance when it is removed
 * Return the port used by a DRBD-based disk when it is removed

Signed-off-by: Vangelis Koukis <vkoukis@grnet.gr>
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Until now LUNodeSetParams would lock all instances if a node's
secondary IP address was to be changed and would then release
all instances it didn't actually need. With this patch the LU
optimistically locks instances and, once it got the locks,
checks whether they're still correct. This is similar to how
node group locking is done.

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

With this patch all RPC calls at runtime of masterd will show up in the
lock monitor. There is a chicken-and-egg issue with initializing the
configuration with a context since the lock manager, containing the
monitor, requires the configuration. This is worked around by setting
the config's context only once the lock monitor is available.

Example:
rpc/node19.example.com/write_ssconf_files  Jq9/Job32/N_SET_PARAMjobqueue_update  Jq2/Job27/C_VERIFY_CONFIG

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

Pass instances of objects.MasterNetworkParameters when calling RPCs for
activation and deactivation of master IP.

Signed-off-by: Andrea Spadaccini <spadaccio@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Instead of manually unpacking the return values of
cfg.GetMasterNetworkParameters, let it return an instance of
objects.MasterNetworkParameters and pass its attributes.

Signed-off-by: Andrea Spadaccini <spadaccio@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Signed-off-by: Andrea Spadaccini <spadaccio@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>

This will be used for resolving node names.

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

These two calls, “upload_file” and “write_ssconf_files” are treated
separated as they're used by the configuration, where we can't use the
normal resolver.

There's still some duplicated code in rpc.py, but that will be taken
care of in future patches.

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

Add the master_netmask cluster parameter, that represents the netmask of
the master IP, encoded as a CIDR suffix.

This parameter can be set via the --master-netmask of gnt-cluster init
and gnt-cluster modify. The default behaviour is to be consistent with
the old default (/32 for IPv4 and /128 for IPv6).

Signed-off-by: Andrea Spadaccini <spadaccio@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

In version 0.21, pylint unified all the disable-* (and enable-*)
directives to disable (resp. enable). This leads to a lot of
DeprecationWarning being emitted even if one uses the recommended
version of pylint (0.21.1, as stated in devnotes.rst).

This commit changes all the disable-msg directives to disable.

Signed-off-by: Andrea Spadaccini <spadaccio@google.com>
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

Identified using the “pep8” utility.

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

The module is called “objects”, not “object”.

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

Similar to the previous patch, this adds a helper function to
eliminate repeated calls info ConfigWriter.

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

This adds a new ConfigWriter.GetMultiNodeInfo function and replaces
multiple/looping calls to GetNodeInfo with it.

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

ConfigWriter.ReserveLV() and Configwriter.ReserveMAC() called
TemporaryReservationManager.Reserve() with the ec_id and resource arguments
swapped. As a result, two reservation attempts for the same resource type
within the same LU would fail, even if the resources requested were different,
e.g.:

  $ gnt-instance add -t sharedfile -o debootstrap+default \
       --net 0:mac=00:01:02:03:04:00 \
       --net 1:mac=00:01:02:03:04:ff \
       --disk 0:size=2g  test_instance
  Failure: prerequisites not met for this operation:
  error type: resource_not_unique, error details:
  MAC address 00:01:02:03:04:ff already in use in cluster

This patch fixes the argument order in the call to Reserve().

Signed-off-by: Apollon Oikonomopoulos <apollon@noc.grnet.gr>
Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

This will be used for locking during node evacuation.

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

This will be used for evacuating instances in a node group.

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