Some instance test functions took two node arguments, some took one, and some took two but the second argument could be None. This patch makes such functions uniform by using a list of nodes as an argument. This simplifies the following refactoring.
Signed-off-by: Bernardo Dal Seno <[email protected]> --- qa/ganeti-qa.py | 74 ++++++++++++++++++++++++++++--------------------------- qa/qa_instance.py | 35 ++++++++++++++------------ qa/qa_rapi.py | 16 +++++++----- 3 files changed, 67 insertions(+), 58 deletions(-) diff --git a/qa/ganeti-qa.py b/qa/ganeti-qa.py index a3d51d3..cc29d85 100755 --- a/qa/ganeti-qa.py +++ b/qa/ganeti-qa.py @@ -340,17 +340,17 @@ def RunGroupRwTests(): qa_group.GetDefaultGroup()) -def RunExportImportTests(instance, pnode, snode): +def RunExportImportTests(instance, inodes): """Tries to export and import the instance. - @param pnode: current primary node of the instance - @param snode: current secondary node of the instance, if any, - otherwise None + @type inodes: list of nodes + @param inodes: current nodes of the instance """ if qa_config.TestEnabled("instance-export"): RunTest(qa_instance.TestInstanceExportNoTarget, instance) + pnode = inodes[0] expnode = qa_config.AcquireNode(exclude=pnode) try: name = RunTest(qa_instance.TestInstanceExport, instance, expnode) @@ -373,14 +373,10 @@ def RunExportImportTests(instance, pnode, snode): if qa_config.TestEnabled(["rapi", "inter-cluster-instance-move"]): newinst = qa_config.AcquireInstance() try: - if snode is None: - excl = [pnode] - else: - excl = [pnode, snode] - tnode = qa_config.AcquireNode(exclude=excl) + tnode = qa_config.AcquireNode(exclude=inodes) try: RunTest(qa_rapi.TestInterClusterInstanceMove, instance, newinst, - pnode, snode, tnode) + inodes, tnode) finally: qa_config.ReleaseNode(tnode) finally: @@ -414,7 +410,7 @@ def RunSingleHomedHardwareFailureTests(instance, pnode): qa_config.ReleaseNode(othernode) -def RunHardwareFailureTests(instance, pnode, snode): +def RunHardwareFailureTests(instance, inodes): """Test cluster internal hardware failure recovery. """ @@ -427,32 +423,37 @@ def RunHardwareFailureTests(instance, pnode, snode): qa_rapi.TestRapiInstanceMigrate, instance) if qa_config.TestEnabled("instance-replace-disks"): - othernode = qa_config.AcquireNode(exclude=[pnode, snode]) + othernodes = qa_config.AcquireManyNodes(len(inodes) - 1, exclude=inodes) try: RunTestIf("rapi", qa_rapi.TestRapiInstanceReplaceDisks, instance) RunTest(qa_instance.TestReplaceDisks, - instance, pnode, snode, othernode) + instance, inodes, othernodes) finally: - qa_config.ReleaseNode(othernode) + qa_config.ReleaseManyNodes(othernodes) + del othernodes if qa_config.TestEnabled("instance-recreate-disks"): - othernode1 = qa_config.AcquireNode(exclude=[pnode, snode]) try: - othernode2 = qa_config.AcquireNode(exclude=[pnode, snode, othernode1]) + acquirednodes = qa_config.AcquireManyNodes(len(inodes), exclude=inodes) + othernodes = acquirednodes except qa_error.OutOfNodesError: - # Let's reuse one of the nodes if the cluster is not big enough - othernode2 = pnode + if len(inodes) > 1: + # If the cluster is not big enough, let's reuse some of the nodes, but + # with different roles. In this way, we can test a DRBD instance even on + # a 3-node cluster. + acquirednodes = [qa_config.AcquireNode(exclude=inodes)] + othernodes = acquirednodes + inodes[:-1] + else: + raise try: RunTest(qa_instance.TestRecreateDisks, - instance, pnode, snode, [othernode1, othernode2]) + instance, inodes, othernodes) finally: - qa_config.ReleaseNode(othernode1) - if othernode2 != pnode: - qa_config.ReleaseNode(othernode2) - - RunTestIf("node-evacuate", qa_node.TestNodeEvacuate, pnode, snode) + qa_config.ReleaseManyNodes(acquirednodes) - RunTestIf("node-failover", qa_node.TestNodeFailover, pnode, snode) + if len(inodes) >= 2: + RunTestIf("node-evacuate", qa_node.TestNodeEvacuate, inodes[0], inodes[1]) + RunTestIf("node-failover", qa_node.TestNodeFailover, inodes[0], inodes[1]) def RunExclusiveStorageTests(): @@ -470,8 +471,8 @@ def RunExclusiveStorageTests(): if qa_config.TestEnabled("instance-add-plain-disk"): # Make sure that the cluster doesn't have any pre-existing problem qa_cluster.AssertClusterVerify() - instance1 = qa_instance.TestInstanceAddWithPlainDisk(node) - instance2 = qa_instance.TestInstanceAddWithPlainDisk(node) + instance1 = qa_instance.TestInstanceAddWithPlainDisk([node]) + instance2 = qa_instance.TestInstanceAddWithPlainDisk([node]) # cluster-verify checks that disks are allocated correctly qa_cluster.AssertClusterVerify() qa_instance.TestInstanceRemove(instance1) @@ -480,7 +481,7 @@ def RunExclusiveStorageTests(): snode = qa_config.AcquireNode() try: qa_cluster.TestSetExclStorCluster(False) - instance = qa_instance.TestInstanceAddWithDrbdDisk(node, snode) + instance = qa_instance.TestInstanceAddWithDrbdDisk([node, snode]) qa_cluster.TestSetExclStorCluster(True) exp_err = [constants.CV_EINSTANCEUNSUITABLENODE] qa_cluster.AssertClusterVerify(fail=True, errors=exp_err) @@ -541,11 +542,11 @@ def RunQa(): del rapi_instance if qa_config.TestEnabled("instance-add-plain-disk"): - instance = RunTest(qa_instance.TestInstanceAddWithPlainDisk, pnode) + instance = RunTest(qa_instance.TestInstanceAddWithPlainDisk, [pnode]) RunCommonInstanceTests(instance) RunGroupListTests() RunTestIf("cluster-epo", qa_cluster.TestClusterEpo) - RunExportImportTests(instance, pnode, None) + RunExportImportTests(instance, [pnode]) RunDaemonTests(instance) RunRepairDiskSizes() RunSingleHomedHardwareFailureTests(instance, pnode) @@ -561,7 +562,7 @@ def RunQa(): if qa_config.TestEnabled(name): snode = qa_config.AcquireNode(exclude=pnode) try: - instance = RunTest(func, pnode, snode) + instance = RunTest(func, [pnode, snode]) RunTestIf("haskell-confd", qa_node.TestNodeListDrbd, pnode) RunTestIf("haskell-confd", qa_node.TestNodeListDrbd, snode) RunCommonInstanceTests(instance) @@ -571,10 +572,11 @@ def RunQa(): pnode["primary"], snode["primary"]) if qa_config.TestEnabled("instance-convert-disk"): RunTest(qa_instance.TestInstanceShutdown, instance) - RunTest(qa_instance.TestInstanceConvertDisk, instance, snode) + RunTest(qa_instance.TestInstanceConvertDiskToPlain, instance, + [pnode, snode]) RunTest(qa_instance.TestInstanceStartup, instance) - RunExportImportTests(instance, pnode, snode) - RunHardwareFailureTests(instance, pnode, snode) + RunExportImportTests(instance, [pnode, snode]) + RunHardwareFailureTests(instance, [pnode, snode]) RunRepairDiskSizes() RunTest(qa_instance.TestInstanceRemove, instance) del instance @@ -591,7 +593,7 @@ def RunQa(): try: pnode = qa_config.AcquireNode(exclude=snode) try: - instance = qa_instance.TestInstanceAddWithDrbdDisk(pnode, snode) + instance = qa_instance.TestInstanceAddWithDrbdDisk([pnode, snode]) qa_node.MakeNodeOffline(snode, "yes") try: RunTest(qa_instance.TestInstanceRemove, instance) @@ -606,7 +608,7 @@ def RunQa(): try: if qa_config.TestEnabled(["instance-add-plain-disk", "instance-export"]): for shutdown in [False, True]: - instance = RunTest(qa_instance.TestInstanceAddWithPlainDisk, pnode) + instance = RunTest(qa_instance.TestInstanceAddWithPlainDisk, [pnode]) expnode = qa_config.AcquireNode(exclude=pnode) try: if shutdown: diff --git a/qa/qa_instance.py b/qa/qa_instance.py index 6dcc58c..91a4442 100644 --- a/qa/qa_instance.py +++ b/qa/qa_instance.py @@ -177,15 +177,17 @@ def IsDiskReplacingSupported(instance): @InstanceCheck(None, INST_UP, RETURN_VALUE) -def TestInstanceAddWithPlainDisk(node): +def TestInstanceAddWithPlainDisk(nodes): """gnt-instance add -t plain""" - return _DiskTest(node["primary"], "plain") + assert len(nodes) == 1 + return _DiskTest(nodes[0]["primary"], "plain") @InstanceCheck(None, INST_UP, RETURN_VALUE) -def TestInstanceAddWithDrbdDisk(node, node2): +def TestInstanceAddWithDrbdDisk(nodes): """gnt-instance add -t drbd""" - return _DiskTest("%s:%s" % (node["primary"], node2["primary"]), + assert len(nodes) == 2 + return _DiskTest("%s:%s" % (nodes[0]["primary"], nodes[1]["primary"]), "drbd") @@ -481,7 +483,7 @@ def TestInstanceStoppedModify(instance): @InstanceCheck(INST_DOWN, INST_DOWN, FIRST_ARG) -def TestInstanceConvertDisk(instance, snode): +def TestInstanceConvertDiskToPlain(instance, inodes): """gnt-instance modify -t""" name = instance["name"] template = qa_config.GetInstanceTemplate(instance) @@ -489,9 +491,10 @@ def TestInstanceConvertDisk(instance, snode): print qa_utils.FormatInfo("Unsupported template %s, skipping conversion" " test" % template) return + assert len(inodes) == 2 AssertCommand(["gnt-instance", "modify", "-t", "plain", name]) AssertCommand(["gnt-instance", "modify", "-t", "drbd", - "-n", snode["primary"], name]) + "-n", inodes[1]["primary"], name]) @InstanceCheck(INST_DOWN, INST_DOWN, FIRST_ARG) @@ -537,11 +540,8 @@ def TestInstanceConsole(instance): @InstanceCheck(INST_UP, INST_UP, FIRST_ARG) -def TestReplaceDisks(instance, pnode, snode, othernode): +def TestReplaceDisks(instance, curr_nodes, other_nodes): """gnt-instance replace-disks""" - # pylint: disable=W0613 - # due to unused pnode arg - # FIXME: should be removed from the function completely def buildcmd(args): cmd = ["gnt-instance", "replace-disks"] cmd.extend(args) @@ -553,6 +553,12 @@ def TestReplaceDisks(instance, pnode, snode, othernode): " skipping test") return + # Currently all supported templates have one primary and one secondary node + assert len(curr_nodes) == 2 + snode = curr_nodes[1] + assert len(other_nodes) == 1 + othernode = other_nodes[0] + options = qa_config.get("options", {}) use_ialloc = options.get("use-iallocators", True) for data in [ @@ -604,21 +610,18 @@ def _AssertRecreateDisks(cmdargs, instance, fail=False, check=True, @InstanceCheck(INST_UP, INST_UP, FIRST_ARG) -def TestRecreateDisks(instance, pnode, snode, othernodes): +def TestRecreateDisks(instance, inodes, othernodes): """gnt-instance recreate-disks @param instance: Instance to work on - @param pnode: Primary node - @param snode: Secondary node, or None for sigle-homed instances + @param inodes: List of the current nodes of the instance @param othernodes: list/tuple of nodes where to temporarily recreate disks """ options = qa_config.get("options", {}) use_ialloc = options.get("use-iallocators", True) other_seq = ":".join([n["primary"] for n in othernodes]) - orig_seq = pnode["primary"] - if snode: - orig_seq = orig_seq + ":" + snode["primary"] + orig_seq = ":".join([n["primary"] for n in inodes]) # These fail because the instance is running _AssertRecreateDisks(["-n", other_seq], instance, fail=True, destroy=False) if use_ialloc: diff --git a/qa/qa_rapi.py b/qa/qa_rapi.py index b34bb25..4ffa916 100644 --- a/qa/qa_rapi.py +++ b/qa/qa_rapi.py @@ -759,7 +759,7 @@ def GetOperatingSystems(): def TestInterClusterInstanceMove(src_instance, dest_instance, - pnode, snode, tnode): + inodes, tnode): """Test tools/move-instance""" master = qa_config.GetMasterNode() @@ -772,18 +772,22 @@ def TestInterClusterInstanceMove(src_instance, dest_instance, # TODO: Run some instance tests before moving back - if snode is None: + if len(inodes) > 1: + # No disk template currently requires more than 1 secondary node. If this + # changes, either this test must be skipped or the script must be updated. + assert len(inodes) == 2 + snode = inodes[1] + else: # instance is not redundant, but we still need to pass a node # (which will be ignored) - fsec = tnode - else: - fsec = snode + snode = tnode + pnode = inodes[0] # note: pnode:snode are the *current* nodes, so we move it first to # tnode:pnode, then back to pnode:snode for si, di, pn, sn in [(src_instance["name"], dest_instance["name"], tnode["primary"], pnode["primary"]), (dest_instance["name"], src_instance["name"], - pnode["primary"], fsec["primary"])]: + pnode["primary"], snode["primary"])]: cmd = [ "../tools/move-instance", "--verbose", -- 1.8.1 -- You received this message because you are subscribed to the Google Groups "ganeti-devel" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/groups/opt_out.
