Stricter check for OS modifications passed to OpClusterSetParams

Don't just check the first element of each item, but also make sure the
OS name is a string and not empty.

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

LUInstanceSetParams: Allow no-op change of instance offline status

With this patch marking an instance already marked offline (or online)
as offline/online again becomes a no-op. Also removed the unused
INSTANCE_UP variable.

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

OpInstanceSetParams: Merge {off,on}line_inst parameters

Instead of having two separate parameters, a single boolean parameter is
used. Unfortunately we need a third state to say “no change”, so the
value can be None, True or False (similar to other parameters). There
are no user interface changes.

New QA tests are added, too.

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

ipolicy: Make the keys of the dict consistent

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

walkthrough.rst: updates for maxmem/minmem

- Fix two example outputs
- Improve N+1 resolution section specifying how different memories
  affect N+1 calculation depending on the node

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

admin.rst: add information about maxmem/minmem

- Specify how maxmem and minmem are used at startup, failover and
  migrate
- Add a paragraph about how to manually change the runtime memory, and
  what Ganeti will and won't do automatically as of version 2.6

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

N+1: verify using minimum memory

Since instances can be started, failed over and migrated with less than
their maximum memory N+1 will use the minimum memory for verification.

Note that this accounts only for the instances being moved being
resized, and not for the ones already on the node, as ganeti will not
automatically resize other instances on the target node now when trying
to start/failover/migrate an instance.

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

Fix integer overflow in Node unittest

When using a really random "extra cpus" variable, this can overflow
(especially on 32 bit) and thus the final value can be < 0, thus the
test will fail to fail.

It doesn't make sense to test with huge values, so let's just generate
a reasonable value.

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

Use allow_runtime_chgs in cmdlib

- Report the attribute value in the hooks runtime variable.
- Use it to abort migration if not enough memory is available on the
  target node.

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

cli/opcodes: add a --no-runtime-changes migrate opt

This will be used for now to avoid ballooning memory at live migration time.

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

Migrate/Failover: allow less-than-max-mem op

Failover works magically after we relax the check: the start operation
will just use less memory, if less memory is available.

For migrate we need to balloon down the source memory before the
migration, if the target node doesn't have enough capacity.

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

Failover and Migrate: acquire node resource locks

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

cmdlib._CheckNodeFreeMemory: return free memory

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

doc/rapi.rst: Document ipolicy parameter

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

Expand the tiered alloc unittest to check allocation stats

This was missing before, so with the new metric (NCpus), it's a good
time to test this.

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

Switch over some hspace stats from RSpec to AllocInfo

This patch changes the allocation delta stats from RSpec to
AllocInfo. There's nothing interesting about it, but it should allow
more correct computations of Npus in hspace.

I also moved the AllocStats type alias from Cluster.hs to Types.hs
(just makes more sense).

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

Add a new type for holding allocation statistics

The current RSpec type doesn't cut it anymore (and it's used in other
places), so let's introduce a separate type for holding these
statistics.

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

Track 'normalised' used CPUs in Cluster.CStats

This will be used in hspace for proper tracking of used/pool/unavail
npus.

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

Re-indent code for readability

Otherwise the docstrings for members take too many lines.

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

Remove obsolete constant for vcpu_ratio

This is superseded now.

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

Change meaning of the --max-cpu option

Instead of always overriding, we should update the nodes's maxcpu
setting only when the option is specified. Otherwise this will be read
from the cluster or from the Ganeti constant for non-live backends.

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

Temporary fix for hspace's NPU calculation

hspace computes the NPU (normalised CPU units) via the
(used|pool|avail) VCPU divided by the (currently cluster-wide) max
vcpu_ratio.

However, in the future we'll have variable vcpu_ratios, so we can't
the calculation as such. We temporarily change to computing the
average vcpu_ratio across the cluster, however long-term this needs to
be improved, as ∑ (Vi/Pi) ≠ ∑ Vi / ∑ Pi. In the meantime we implement
this patch to remove the use of a global mcpu variable.

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

Remove Node.mCpu, replaced with the ipol vcpu_ratio

Since the iPolicy contains the vcpu_ratio now, we can remove the
per-node mCpu value (which was the initial way to set this attribute).

Furthermore, we need to re-set the hiCpu value when changing the
policy.

And of course some associated unittest changes.

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

Add vcpu_ratio definition to the IPolicy type

This will be "naturally" read in the JSON-based backends (Luxi/Rapi),
and we add explicit code for handling it in the Text backend.

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

Export the cluster ipolicy in iallocator

And remove a strange comment (the commit which introduced it,
d61df03e, doesn't explain it). Also document these new keys in
iallocator.rst.

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

Fix gnt-cluster modify check of passed options

… and add the disk_templates/vcpu_ratio new entries. Since the default
ispecs_* are dicts, we can't compare them to none.

Additionally, add a QA test for this case (empty cluster modify).

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

Add ipolicy_vcpu_ratio to command line

This allows changing this value from the command line.

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

Add new ipolicy parameter vcpu_ratio

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>

Clarification on what objects.InstancePolicy is

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

Rename IPOLICY_PARAMETERS to IPOLICY_ISPECS

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>

Remove two extraneous imports

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

gnt-instance recreate-disks: Allow specifying new size

With this change a new disk size can be specified when recreating disks.

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

LUClusterSetParams: When ipolicy is updated warn for new violations

We need to loop over all node groups to apply their modification to make
sure that the new ipolicy actually affects their instance

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

LUGroupSetParams: Warn if the new ipolicy will violate some ipolicies

If we modify the ipolicy we need to verify if any instance would now
violate the new ipolicy and warn about it. Any instance which already
violates the old one is ignored.

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

cmdlib: Add a helper function for a set of violating instances

This helper will be used with the Op*SetParams and ipolicy verifications

Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: Guido Trotter <ultrotter@google.com>

Rename ispecs-disk-templates to ipolicy-disk-templates

The ispecs (min/std/max) are different from the other ipolicy
variables (disk templates for now, more in the future). As such, let's
rename the ispecs-disk-templates to ipolicy-disk-templates for
consistency with this logical difference.

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

fmtjson: A tool to format JSON data

While debugging an issue recently I had the pleasure of looking at a
long, single-line string of JSON data. This simple utility makes it
possible to format such data into a readable format.

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

Implement rbd disk template

Introduce the rbd disk template, which handles provisioning and
management of instance disks as block devices mapped to rbd volumes
on a RADOS cluster.

The rbd template is of type DTS_EXT_MIRROR, meaning rbd-based
instances can failover or migrate to any node, without
primary-secondary limitations.

Use of this template requires the rbd kernel driver and rbd tool
on all VM-capable nodes, as well as an operational RADOS cluster,
accessible by them.

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

Open VDI Port when spice_use_vdagent is enabled

To use the features spice agent provides, a dedicated virtio-serial
channel must be created in qemu-kvm. The communication between the agent
and the other spice components takes place over this channel.

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

Fix iallocator group objects

… this is yet another manual serialisation of objects :(

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

Fix upgrading the cluster ipolicy

When running from git HEAD, just initialising the ipolicy when
non-existing is not good enough; we need to upgrade also missing keys,
etc.

This has downsides though, as it means we'll always 'rewrite' the
ipolicy, which might create silent data loss for unhandled changes
(e.g. key removal).

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

LUInstanceRecreateDisks: Actually acquire node resource locks

Before this change an assertion would fail as the resource locks weren't
actually acquired.

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

cmdlib._CheckInstanceState: Use instance name in error message

The variable “instance” is an instance object, so to not get a
serialized version of the instance, the “name” attribute must be
used.

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

Unbreak build after merge 2ea64524ce

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

gnt-cluster command: Add option to prepend node name to output

With this patch a new option (“-M”, like dsh) is added to “gnt-cluster
command” to prepend the node name to all output lines.

$ gnt-cluster command -M uname -a | grep Linux
node18.example.com: Linux node18.example.com [...]
node19.example.com: Linux node19.example.com [...]

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

cmdlib: Drop use of “len(…) != 0”

Found by Constantinos Venetsanopoulos.

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

Merge branch 'devel-2.5'

* devel-2.5:
  Migrate: don't check for free memory on cleanup
  Revert "cli: Disable abbreviation matching for options"

Conflicts:
lib/cli.py
          - trivial
lib/cmdlib.py
          - merge new condition

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

Merge branch 'stable-2.5' into devel-2.5

* stable-2.5:
  Migrate: don't check for free memory on cleanup

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

cli: Log fully quoted command line

Instead of logging the concatenated arguments, with this patch every
command will properly quote its full command line. This makes it easier
to just copy-and-paste commands from “commands.log” to re-run them. Also
remove a case of “x = a or b” for non-boolean values.

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

Migrate: don't check for free memory on cleanup

Cleanup just updates the config with the correct location of the
instance, or informs of its down status, but never starts it. As such
there's no point in checking for enough free memory. Actually this check
could prevent a perfectly safe cleanup operation if a node is busy.

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

LUInstanceStartup: check for minimum memory

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

hypervisors: start instances with max available mem

If we have available more than MINMEM but less than MAXMEM, start a new
instance with that value, rather than MAXMEM.

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

Instance startup: lock primary node

This will be used to balloon other instances on the node if needed.

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

kvm: always start in paused state

Currently kvm starts in a frozen state if cpu pinning is set.
This is useful for other purposes as well (eg. ballooning before the
instance uses the memory), so we move the functionality out of the cpu
pinning code.

Note that before the "continue" command was executed in a finally after
trying to set the cpu pinning. This didn't help if anything else in the
function failed before that, though. As such we just move it out, as
anyway a failure above will cause an exception, so the user will know
their start command hasn't succeeded anyway.

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

Revert "cli: Disable abbreviation matching for options"

This reverts commit 232aab3f4f602a19f1226e85c3a3ecb245d60af4. We
shouldn't change the parsing of command line options in the middle of
the 2.5.x series.

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

Merge branch 'devel-2.5'

* devel-2.5:
  Fix wrong option names in QA and cluster-merge
  Bump version to 2.5.0~rc5, update NEWS
  Add UnescapeAndSplit unittest for multi-escapes
  Fix a bug in command line option parsing code
  cli: Disable abbreviation matching for options
  Bump version to 2.5.0~rc5, update NEWS
  Add UnescapeAndSplit unittest for multi-escapes
  Fix a bug in command line option parsing code

Conflicts:
lib/cli.py: Trivial

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

Merge branch 'stable-2.5' into devel-2.5

* stable-2.5:
  Bump version to 2.5.0~rc5, update NEWS
  Add UnescapeAndSplit unittest for multi-escapes
  Fix a bug in command line option parsing code

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

Fix wrong option names in QA and cluster-merge

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

Bump version to 2.5.0~rc5, update NEWS

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

Add UnescapeAndSplit unittest for multi-escapes

This would have caught the bug in the first place. Argh,
hand-generated test cases!

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

Fix a bug in command line option parsing code

Fix bug affecting command line options of "keyval" type. Although
escaping commands with \ is supported, it is is not applied to the
input recursively.

Signed-off-by: Nikos Skalkotos <skalkoto@grnet.gr>
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

cli: Disable abbreviation matching for options

Python's “optparse” module does option name prefix matching by default.
Since this can lead to confusing behaviour, e.g. by specifying “--force”
for a command which only has a “--force-multi” option, this patch
disables the prefix matching.

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

qa: perform burnin with max/min mem from config

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

Burnin: test set-mem

Add new --maxmem-size and --minmem-size options.
The old --mem-size option allows setting both at the same time.
If --minmem-size is lower than --maxmem-size execute a runtime memory
change on all instances, bringing them to their minimum memory size.

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

gnt-instance modify -m|--runtime-memory

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

Add instance_balloon_memory rpc

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

No-op memory ballooning in chroot and lxc

Currently they have no memory limits set.

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

Implement memory ballooning in the fake hv

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

xen: add the "maxmem" parameter in instances config

This is set to the same amount as the memory for now, but will allow
starting instances with less memory than their maximum.

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

Implement memory ballooning in kvm

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

Implement memory ballooning in xen

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

Hypervisor: allow ballooning of instance memory

This new hypervisor call sets the memory of an instance to a new value,
through ballooning.

Signed-off-by: Guido Trotter <ultrotter@google.com>
Reviewed-by: René Nussbaumer <rn@google.com>

Accept both PUT and POST in noded

Currently, noded requires PUT, even though the semantics of the RPC
calls do not match a PUT. We change the code accept both PUT and POST,
with the intention to remove the PUT support in a later version.

On the client side, the RPC code is changed to send POST requests.

Additionally, we add a message to the HttpBadRequest exception to make
clear the failure mode (not seeing any error message was what made me
send this patch…). This was the only description-less use of this
exception, by the way.

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

Rework instance rename QA tests

After looking again at the QA speeds, it turned out that instance
rename was run 12 times via the command line and 12 times via RAPI;
outside of start/stop which we need for maintenance during QA, this is
the most used test.

This patch does a few changes with regard to instance rename:

- it changes rename functions to rename the instance back to the
  original name
- which allows us not to run said functions twice, A→B and then B→A

This brings down the number of rename tests from 12 to 8 (for both
cmdline and RAPI).

Furthermore, the CLI rename test was doing too many actual instance
renames (at Ganeti level), so we rework it to be simpler (brings down
the average duration from ~27s to ~21s), and we reorganise the run of
various tests such that we do fewer instances start/stop tests (10
instead of 14).

Overall we only gain about 3 minutes (~7%), but still it's a speedup.

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

Another small optimisation in test data generation

It seems that QuickCheck will happily generate huge data structures,
if that's what one requests. For this test, where we ask for [[Char]],
it could happen that we get a huge number of chars, so let's limit
them to some (arbitrary, but low) limits. This doesn't "ruin" the
test, since the implementation of sepSplit/commaJoin is not
size-based.

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

Generate tags using the helper in Node tests

Otherwise we test very big lists (slow) without any advantage in doing
so.

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

Combine two Node unittests

These two tests are very similar, so let's run them together instead
of generating the data twice. Also improve the test a bit.

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

Add unit-tests for --help/--version as common options

After reorganizing the way we process options, we can finally add a
test that all program personalities accept --help and --version and
that they exit early for these two.

This trivial patch has a higher than expected impact on the coverage,
as the use of short options finally enforces evaluation of the short
option names, thus it affects more than just the help/version options.

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

Split the list of program personalities into a module

… a mini-module, actually. This is only done for unit-testing, as we
don't want to duplicate the personalities list in the tests.

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

Parse options in htools.hs, not in each program

This patch moves the call to getArgs/parseOpts into htools.hs, as
opposed to the current situation where htools.hs only calls the main
file of each program, which in turn calls these two functions.

By doing it only once we remove some duplicate code and we also export
each program's options, which allows us to test some common behaviour
in the future.

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

Add some very basic options test

Just tests that passing a string argument returns it correctly. We're
not interested in the flags themselves that much just that the
function itself parses some options at least correctly.

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

Split CLI.parseOpts into IO/non-IO parts

This will allow unittesting the CLI options much more easily. The
patch also fixes an inconsistency: usage help was displayed with an
extra new line in the error case (but not in --help).

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

Add test for CLI.parseYesNo

Also fix a tiny typo in the docstring.

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

Fix bug in CLI.parseISpecString and add test

'zip' finishes on the shortest list, so parseISpecString accepted any
string of 3 values or more, which is not good. Fix this and add test
for both successful and failed parsing.

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

Add a unittest for the 'Simu' backend

This only test successful parsing, but for this case it tests
reasonably well the resulting cluster properties, I think (coverage
is >70% for expressions).

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

Further optimise instance test data generation

Following on the lessons from this week (commit 7806125e), this patch
replaces generation of instances `suchThat` they are smaller than on
node with direct generation of instances with upper bounds on
memory/disk/cpu.

For a particular case I was investigating, this reduces the overall
duration of tests by almost 50%. I don't expect it to speed up this
nice in most cases, but it's too good an optimisation to pass up.

The patch also does a trivial simplification (if … then True else
False, d'oh).

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

Add a Makefile target for profiling targets

This simplifies the building of profiling-enabled htools binaries;
there are two targets, one used for the initial two-stage build, and
one that can be used for incremental builds.

The patch adds a new internal variable for the htools/test binary,
which should also simplify custom builds for the non-profiling
targets.

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

gnt-instance: Adapt replace-disks to the new opcode

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

TLReplaceDisk: Add ipolicy checks

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

Fix very slow unit-test data generation in some cases

Learning, learning. It turns out that for objects with even just five
fields, generating them and filtering can be costly, especially when
the filtering is nested (as it was in this case, arbitrary1 →
arbitrary2 + filter on arbitrary1 → arbitrary plus filter on
aribitrary2).

Changing the code to generate directly the good values speeds things
up significantly. Additionally, we also generate correctly unique
lists of disk templates (again, this should be a set, not a list,
but…).

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

Fix one bad unit-test data generation case

It could happen that the node is generated with 0 disk available, in
which case the operation will fail due to disk issues, not CPU. Fix
this by using the recently-introduced genOnlineNode helper.

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

Further fixes to instance policy validation

As a followup from "Remove extraneous check in policy creation", there
are more places where we build an ipolicy, and then manually check for
its validity. This is very bad style, as it duplicates the
verification code across many places.

This patch removes all such explicit checks (except for one in
cmdlib.py which is correct), and instead does a bit more validation in
the builder functions or in the actual dedicated verification
functions. It also fixes cluster init which used the wrong,
non-completed ipolicy (this was not detected before as we did call
check on it, but otherwise we ignored it), and fixes a too-strong
assert (due to the call chain, we first create the ipolicy from
cmdline params, and only then we fill it).

Finally, it removes an extraneous logging.info which I forgot from
debugging.

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

Fix another data generation issue in tests

Another "good" seed value manages to generate nodes with identical
names, which of course makes the sets be of smaller value than the
lists. Avoid this issue by generating unique node names.

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

Remove some dead code

While wondering why Cluster.tryReloc is not unit-tested, I realised
that this is no longer uses. Apparently, last summer's new IAllocator
modes and related changes have silently deprecated this function.

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

Fix a few issues in unittest data generation

This patch fixes two ugly but non-critical issues:

- the getFQDN wrongly generated only the first component as
  limited-length name, the rest could be any length, and that makes
  the generated name bad for display purposes
- the random generator had real issues in finding an offline instance
  with the right properties when starting from certain seeds (found
  accidentally); this meant the test took 1 minute, instead of ~300ms,
  for the node_addSecOffline

Furthermore, for the same evil seed, the Container_findByName test
wasn't able to generate enough good data, so we change its
implementation to directly generate an uniques list, instead of
generating random lists and filtering the unique ones.

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

Extend node evacuation unittests

Currently they only check for change secondary, via iallocator's
relocate method. We add two separate unittests that checks all the
node evacuate modes (ChangeAll, ChangePrimary) and also the group
change request. All of these are done only for DRBD instances.

The patch also enhances an error message in Cluster.findBestAllocGroup
for invalid input data (guess how this was detected…).

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

Add a unit-test helper function

… instead of always using "printTestCase … False".

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

Further JSON unit-tests for various Types.hs types

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

Further unittests for ipolicy/ispec text serialisation

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

Implement saving/restoring of policies in Text backend

This adds a new optional (so we're backwards-compat) section to the
cluster data so that we can reliably save/restore the policy data.

Note that different from the other sections, we save all policies
together and separately from the group data. I thought this is cleaner
than extending adding the policy data near each group, and a new
section just for the cluster policy.

This change also allows simplification of the unittest for cluster
create/serialize, although we would need a more complete test (right
now we only dump/load the defIPolicy or nullIPolicy).

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