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>

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>

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>

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>

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>

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>

… 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>

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>

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>

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>

Also fix a tiny typo in the docstring.

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

'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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

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>

… instead of always using "printTestCase … False".

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

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

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

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>

This is unfortunately a complex test that checks if:

- starting from a empty cluster
- allocating a few instances on it
- serialising it via the Text backend
- loading it back into internal data structures

we end up with the same thing. More precisely, it checks that the
cluster after dump/load is the same as after allocation.

While this is in the Text unittests, as the docstring saying, this
check more stuff than just the dump/load. It increases the overall
coverage by about 3%.

There were a few changes needed for this unittest: adding some Eq
instances for some data types, and fixing a cosmetic bug in
Cluster.iterateAlloc, where the instance indices were growing by 2
instead of by 1 in each round.

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

Nothing breaks otherwise in the cluster routines, but serialisation
to/from text gets confused otherwise.

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

This abstracts the concept of "give an sane, online node" in a single
place, rather than spread around the tests. We also change the
signature for isNodeBig for easier usage.

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

The addition of the offline state for instances has introduced a few
bugs related to their handling. The current code adds a helper
function for making the check whether an instances uses reserved
memory on the secondary (and uses it instead of just autoBalance),
and furthermore uses consistently the state for primary node
operations.

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

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

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

Currently, in the human-readable mode, hspace doesn't show any
messages when no instances can be allocated in tiered-alloc mode. This
is confusing, so let's add an explicit message.

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

This currently checks the minimum/maximum sizes and the disk
template. Unittests will come later.

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

In order to be able to use the group policy during allocation, we have
a few choices:

- pass the group policy as a separate argument to all involved
  functions, or
- add the policy as an attribute to nodes, or
- add the group (not only the group index) as an attribute to nodes

The first and last one would change more than wanted for this patch,
so I went with the second one; it will allow easily checking the
target policy at runtime (based on the instance's primary node), with
the only downside that without a node, we can't check the policy.

Note that the first option also would complicate the allocation
routines, which examine (big) lists of node/node pairs, so adding the
group to this would complicate things even more.

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

During loading, we can eliminate some intermediate data
structures. This only matters for big clusters, and even there it's
less than 1% less memory allocated, but still, it's a no-brainer.

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

This is not in the hot-path, but still, no sense in doing O(n) instead
of O(1).

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

This is straightforward; as before, so far the text and simu backends
have no way to use a non-default ipolicy.

Furthermore, we do some unit-test preparation for future changes
(these seem a no-op for now, but they are needed for later patches).

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