LGTM, thanks
On Fri, Dec 20, 2013 at 6:38 PM, Hrvoje Ribicic <[email protected]> wrote: > On Fri, Dec 20, 2013 at 12:28 PM, Helga Velroyen <[email protected]>wrote: > >> >> >> >> On Fri, Dec 20, 2013 at 10:09 AM, Hrvoje Ribicic <[email protected]> wrote: >> >>> The inter-cluster instance move test is very interesting for the RAPI >>> compatibility tests, as it uses many RAPI requests that are otherwise >>> hard to exercise. It uses no command-line functionality apart from >>> the standard QA instance checks. >>> >>> As the move-instance tool does its own checks of all the prerequisites >>> and desired results, not using the QA checks is acceptable for the RAPI >>> workload. This patch allows the checks to be skipped via an optional >>> argument. >>> >>> Signed-off-by: Hrvoje Ribicic <[email protected]> >>> --- >>> qa/qa_rapi.py | 19 ++++++++++++++----- >>> 1 file changed, 14 insertions(+), 5 deletions(-) >>> >>> diff --git a/qa/qa_rapi.py b/qa/qa_rapi.py >>> index ea090d0..0c06e5c 100644 >>> --- a/qa/qa_rapi.py >>> +++ b/qa/qa_rapi.py >>> @@ -815,7 +815,7 @@ def GetOperatingSystems(): >>> >>> >>> def TestInterClusterInstanceMove(src_instance, dest_instance, >>> - inodes, tnode): >>> + inodes, tnode, perform_checks=True): >>> """Test tools/move-instance""" >>> master = qa_config.GetMasterNode() >>> >>> @@ -823,7 +823,9 @@ def TestInterClusterInstanceMove(src_instance, >>> dest_instance, >>> rapi_pw_file.write(_rapi_password) >>> rapi_pw_file.flush() >>> >>> - dest_instance.SetDiskTemplate(src_instance.disk_template) >>> + # Needed only if checks are to be performed >>> + if perform_checks: >>> + dest_instance.SetDiskTemplate(src_instance.disk_template) >>> >>> # TODO: Run some instance tests before moving back >>> >>> @@ -858,7 +860,14 @@ def TestInterClusterInstanceMove(src_instance, >>> dest_instance, >>> si, >>> ] >>> >>> - qa_utils.RunInstanceCheck(di, False) >>> + # Some uses of this test might require that RAPI-only commands are >>> used, >>> + # and the checks are command-line based. >>> + >>> + if perform_checks: >>> + qa_utils.RunInstanceCheck(di, False) >>> + >>> AssertEqual(StartLocalCommand(cmd).wait(), 0) >>> - qa_utils.RunInstanceCheck(si, False) >>> - qa_utils.RunInstanceCheck(di, True) >>> + >>> + if perform_checks: >>> + qa_utils.RunInstanceCheck(si, False) >>> + qa_utils.RunInstanceCheck(di, True) >>> >> >> I'd prefer more expressive variable names than 'si' and 'di'. >> > > Yep, I should take the opportunity to improve the code. > > Interdiff is: > > diff --git a/qa/qa_rapi.py b/qa/qa_rapi.py > index 503174e..cd02633 100644 > --- a/qa/qa_rapi.py > +++ b/qa/qa_rapi.py > @@ -844,33 +844,32 @@ def TestInterClusterInstanceMove(src_instance, > dest_instance, > 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, snode.primary)]: > + for current_src_inst, current_dest_inst, target_pnode, target_snode in \ > + [(src_instance.name, dest_instance.name, tnode.primary, > pnode.primary), > + (dest_instance.name, src_instance.name, pnode.primary, > snode.primary)]: > cmd = [ > "../tools/move-instance", > "--verbose", > "--src-ca-file=%s" % _rapi_ca.name, > "--src-username=%s" % _rapi_username, > "--src-password-file=%s" % rapi_pw_file.name, > - "--dest-instance-name=%s" % di, > - "--dest-primary-node=%s" % pn, > - "--dest-secondary-node=%s" % sn, > + "--dest-instance-name=%s" % current_dest_inst, > + "--dest-primary-node=%s" % target_pnode, > + "--dest-secondary-node=%s" % target_snode, > "--net=0:mac=%s" % constants.VALUE_GENERATE, > master.primary, > master.primary, > - si, > + current_src_inst, > ] > > # Some uses of this test might require that RAPI-only commands are > used, > # and the checks are command-line based. > > if perform_checks: > - qa_utils.RunInstanceCheck(di, False) > + qa_utils.RunInstanceCheck(current_dest_inst, False) > > AssertEqual(StartLocalCommand(cmd).wait(), 0) > > if perform_checks: > - qa_utils.RunInstanceCheck(si, False) > - qa_utils.RunInstanceCheck(di, True) > + qa_utils.RunInstanceCheck(current_src_inst, False) > + qa_utils.RunInstanceCheck(current_dest_inst, True) > > > > -- -- 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
