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)

Reply via email to