On Fri, Dec 20, 2013 at 2:44 PM, Hrvoje Ribicic <[email protected]> wrote:
> Well, the point of this test is not correctness, but compatibility - we > assume that on the reference version, the workload, well, works. > Which might be a silly assumption, and there could be some very subtle > interactions where an error or unexpected behavior propagates and makes > later operations fail erroneously. > But I do not find this so bad as this is yet another thing we'd like to > detect. > > There are some prerequisites that are supposed to hold between tests, and > these are enforced by designing every test to be reversible - it has to > clean up after itself. > Explicit checking between tests might be doable, but it would come at the > cost of either reporting false positives, or making the comparison logic > too complex and hard to maintain. > > One thing to do in the future is probably to add qualifiers for the > desired outcome of each call, and have a command-line option allowing the > test to fail and send out an error if the workload does not execute as > expected on the reference side. However, this is not doable until we add a > new reference version - this workload is meant for both 2.6 and 2.11, and > that means that some 2.11-only functionality such as networks would always > fail. > Thanks for the explanation, I see your point. Cheers, Helga > > On Fri, Dec 20, 2013 at 11:14 AM, Helga Velroyen <[email protected]>wrote: > >> >> >> >> On Fri, Dec 20, 2013 at 10:09 AM, Hrvoje Ribicic <[email protected]> wrote: >> >>> To further expand the number of RAPI methods in the workload, the >>> single instance operations are added in this patch. An instance is >>> created, deleted, shutdown, restarted, reinstalled, renamed, and >>> has its disks activated and deactivated and grown. >>> >>> As this increases the complexity of the code, similarly themed RAPI >>> operations have been extracted into separate functions. >>> >>> Signed-off-by: Hrvoje Ribicic <[email protected]> >>> --- >>> qa/rapi-workload.py | 152 >>> +++++++++++++++++++++++++++++++++++++++++++++++++--- >>> 1 file changed, 146 insertions(+), 6 deletions(-) >>> >>> diff --git a/qa/rapi-workload.py b/qa/rapi-workload.py >>> index 71cae24..6d33802 100755 >>> --- a/qa/rapi-workload.py >>> +++ b/qa/rapi-workload.py >>> @@ -190,16 +190,13 @@ def TestTags(client, get_fn, add_fn, delete_fn, >>> *args): >>> get_fn(*args) >>> >>> >>> -def Workload(client): >>> - """ The actual RAPI workload used for tests. >>> +def TestGetters(client): >>> + """ Tests the various get functions which only retrieve information >>> about the >>> + cluster. >>> >>> @type client C{GanetiRapiClientWrapper} >>> - @param client A wrapped RAPI client. >>> >>> """ >>> - nodes, _instance_names = parseConfig(config) >>> - >>> - # First just the simple information retrievals >>> client.GetVersion() >>> client.GetFeatures() >>> client.GetOperatingSystems() >>> @@ -216,6 +213,135 @@ def Workload(client): >>> client.GetGroups() >>> client.GetGroups(bulk=True) >>> >>> + >>> +def RemoveAllInstances(client): >>> + """ Queries for a list of instances, then removes them all. >>> + >>> + @type client C{GanetiRapiClientWrapper} >>> + @param client A wrapped RAPI client. >>> + >>> + """ >>> + instances = client.GetInstances() >>> + for inst in instances: >>> + Finish(client, client.DeleteInstance, inst) >>> + >>> + instances = client.GetInstances() >>> + assert len(instances) == 0 >>> + >>> + >>> +def TestSingleInstance(client, instance_name, alternate_name, node_one, >>> + node_two): >>> + """ Creates an instance, performs operations involving it, and then >>> deletes >>> + it. >>> + >>> + @type client C{GanetiRapiClientWrapper} >>> + @param client A wrapped RAPI client. >>> + @type instance_name string >>> + @param instance_name The hostname to use. >>> + @type instance_name string >>> + @param instance_name Another valid hostname to use. >>> + @type node_one string >>> + @param node_one A node on which an instance can be added. >>> + @type node_two string >>> + @param node_two A node on which an instance can be added. >>> + >>> + """ >>> + >>> + # Check that a dry run works, use string with size and unit >>> + Finish(client, client.CreateInstance, >>> + "create", instance_name, "plain", [{"size":"1gb"}], [], >>> dry_run=True, >>> + os="debian-image", pnode=node_one) >>> + >>> + # Another dry run, numeric size, should work, but still a dry run >>> + Finish(client, client.CreateInstance, >>> + "create", instance_name, "plain", [{"size": "1000"}], [{}], >>> + dry_run=True, os="debian-image", pnode=node_one) >>> + >>> + # Create a smaller instance, and delete it immediately >>> + Finish(client, client.CreateInstance, >>> + "create", instance_name, "plain", [{"size":800}], [{}], >>> + os="debian-image", pnode=node_one) >>> + >>> + Finish(client, client.DeleteInstance, instance_name) >>> + >>> + # Create one instance to use in further tests >>> + Finish(client, client.CreateInstance, >>> + "create", instance_name, "plain", [{"size":1200}], [{}], >>> + os="debian-image", pnode=node_one) >>> + >>> + client.GetInstance(instance_name) >>> + >>> + Finish(client, client.GetInstanceInfo, instance_name) >>> + >>> + Finish(client, client.GetInstanceInfo, instance_name, static=True) >>> + >>> + TestTags(client, client.GetInstanceTags, client.AddInstanceTags, >>> + client.DeleteInstanceTags, instance_name) >>> + >>> + Finish(client, client.GrowInstanceDisk, >>> + instance_name, 0, 100, wait_for_sync=True) >>> + >>> + Finish(client, client.RebootInstance, >>> + instance_name, "soft", ignore_secondaries=True, dry_run=True, >>> + reason="Hulk smash gently!") >>> + >>> + Finish(client, client.ShutdownInstance, >>> + instance_name, dry_run=True, no_remember=False, >>> + reason="Hulk smash hard!") >>> + >>> + Finish(client, client.StartupInstance, >>> + instance_name, dry_run=True, no_remember=False, >>> + reason="Not hard enough!") >>> + >>> + Finish(client, client.RebootInstance, >>> + instance_name, "soft", ignore_secondaries=True, dry_run=False) >>> + >>> + Finish(client, client.ShutdownInstance, >>> + instance_name, dry_run=False, no_remember=False) >>> + >>> + Finish(client, client.ModifyInstance, >>> + instance_name, disk_template="drbd", remote_node=node_two) >>> + >>> + Finish(client, client.ModifyInstance, >>> + instance_name, disk_template="plain") >>> + >>> + Finish(client, client.RenameInstance, >>> + instance_name, alternate_name, ip_check=True, name_check=True) >>> + >>> + Finish(client, client.RenameInstance, alternate_name, instance_name) >>> + >>> + Finish(client, client.DeactivateInstanceDisks, instance_name) >>> + >>> + Finish(client, client.ActivateInstanceDisks, instance_name) >>> + >>> + Finish(client, client.RecreateInstanceDisks, >>> + instance_name, [0], [node_one]) >>> + >>> + Finish(client, client.StartupInstance, >>> + instance_name, dry_run=False, no_remember=False) >>> + >>> + client.GetInstanceConsole(instance_name) >>> + >>> + Finish(client, client.ReinstallInstance, >>> + instance_name, os=None, no_startup=False, osparams={}) >>> + >>> + Finish(client, client.DeleteInstance, instance_name, dry_run=True) >>> + >>> + Finish(client, client.DeleteInstance, instance_name) >>> + >>> + >>> +def Workload(client): >>> + """ The actual RAPI workload used for tests. >>> + >>> + @type client C{GanetiRapiClientWrapper} >>> + @param client A wrapped RAPI client. >>> + >>> + """ >>> + >>> + # First just the simple information retrievals >>> + TestGetters(client) >>> + >>> + # Then the only remaining function which is parameter-free >>> Finish(client, client.RedistributeConfig) >>> >>> TestTags(client, client.GetClusterTags, client.AddClusterTags, >>> @@ -227,6 +353,20 @@ def Workload(client): >>> client.DeleteNodeTags, node.primary) >>> node.Release() >>> >>> + # Instance tests >>> + >>> + # First remove all instances the QA might have created >>> + RemoveAllInstances(client) >>> + >>> + nodes = qa_config.AcquireManyNodes(2) >>> + instance_one = qa_config.AcquireInstance() >>> + instance_two = qa_config.AcquireInstance() >>> + TestSingleInstance(client, instance_one.name, instance_two.name, >>> + nodes[0].primary, nodes[1].primary) >>> + instance_two.Release() >>> + instance_one.Release() >>> + qa_config.ReleaseManyNodes(nodes) >>> + >>> >> >> >> >>> >>> def Usage(): >>> sys.stderr.write("Usage:\n\trapi-workload.py qa-config-file") >>> -- >>> 1.8.5.1 >>> >>> >> >> LGTM to this patch, just a general remark: >> >> Maybe this is covered in further patches, but I am missing tests for >> errors due to instances 'interacting' with each other, for example if >> someone tries to create an instance that is already there. I would >> understand that this might be a too specific/detailed test (which might be >> covered better in the normal QA), so I am asking if it was a deliberate >> decision to not test that. >> >> Cheers, >> Helga >> >> -- >> -- >> Helga Velroyen | Software Engineer | [email protected] | >> >> Google Germany GmbH >> Dienerstr. 12 >> 80331 München >> >> Registergericht und -nummer: Hamburg, HRB 86891 >> Sitz der Gesellschaft: Hamburg >> Geschäftsführer: Graham Law, Christine Elizabeth Flores >> > > -- -- Helga Velroyen | Software Engineer | [email protected] | Google Germany GmbH Dienerstr. 12 80331 München Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Graham Law, Christine Elizabeth Flores
