OK, thanks for the explanation. LGTM.
On Mon, Feb 24, 2014 at 11:34 AM, Hrvoje Ribicic <[email protected]> wrote: > > > On Mon, Feb 24, 2014 at 11:34 AM, Hrvoje Ribicic <[email protected]> wrote: > >> I was not the one who wrote that code, but I guess that it is - the >> move-instance tool will fail if the destination instance exists, and seeing >> exactly how it failed could be of interest. >> >> >> On Fri, Feb 21, 2014 at 3:51 PM, Petr Pudlák <[email protected]> wrote: >> >>> I'm missing the check >>> >>> if perform_checks: >>> qa_utils.RunInstanceCheck(current_dest_inst, False) >>> >>> just before ...StartLocalCommand..., is it intentional? >>> >>> >>> On Tue, Feb 18, 2014 at 3:39 PM, Hrvoje Ribicic <[email protected]> wrote: >>> >>>> The move-instance QA test will have to be changed in the following >>>> patches to allow testing opportunistic locking. >>>> >>>> This patch retains the same functionality as before, but allows an >>>> iallocator to be used, and splits the move back and forth into two >>>> invocations, the first of which will be wrapped later on. >>>> >>>> Signed-off-by: Hrvoje Ribicic <[email protected]> >>>> --- >>>> qa/qa_rapi.py | 81 >>>> +++++++++++++++++++++++++++++++++++++---------------------- >>>> 1 file changed, 51 insertions(+), 30 deletions(-) >>>> >>>> diff --git a/qa/qa_rapi.py b/qa/qa_rapi.py >>>> index 328abec..f5d309a 100644 >>>> --- a/qa/qa_rapi.py >>>> +++ b/qa/qa_rapi.py >>>> @@ -890,6 +890,48 @@ def GetOperatingSystems(): >>>> return _rapi_client.GetOperatingSystems() >>>> >>>> >>>> +def _InvokeMoveInstance(current_dest_inst, current_src_inst, >>>> rapi_pw_filename, >>>> + joint_master, perform_checks, >>>> target_nodes=None): >>>> + """ Invokes the move-instance tool for testing purposes. >>>> + >>>> + """ >>>> + # 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(current_dest_inst, False) >>>> + >>>> + cmd = [ >>>> + "../tools/move-instance", >>>> + "--verbose", >>>> + "--src-ca-file=%s" % _rapi_ca.name, >>>> + "--src-username=%s" % _rapi_username, >>>> + "--src-password-file=%s" % rapi_pw_filename, >>>> + "--dest-instance-name=%s" % current_dest_inst, >>>> + ] >>>> + >>>> + if target_nodes: >>>> + pnode, snode = target_nodes >>>> + cmd.extend([ >>>> + "--dest-primary-node=%s" % pnode, >>>> + "--dest-secondary-node=%s" % snode, >>>> + ]) >>>> + else: >>>> + cmd.append("--iallocator=%s" % constants.IALLOC_HAIL) >>>> + >>>> + cmd.extend([ >>>> + "--net=0:mac=%s" % constants.VALUE_GENERATE, >>>> + joint_master, >>>> + joint_master, >>>> + current_src_inst, >>>> + ]) >>>> + >>>> + AssertEqual(StartLocalCommand(cmd).wait(), 0) >>>> + >>>> + if perform_checks: >>>> + qa_utils.RunInstanceCheck(current_src_inst, False) >>>> + qa_utils.RunInstanceCheck(current_dest_inst, True) >>>> + >>>> + >>>> def TestInterClusterInstanceMove(src_instance, dest_instance, >>>> inodes, tnode, perform_checks=True): >>>> """Test tools/move-instance""" >>>> @@ -911,38 +953,17 @@ def TestInterClusterInstanceMove(src_instance, >>>> dest_instance, >>>> assert len(inodes) == 2 >>>> snode = inodes[1] >>>> else: >>>> - # instance is not redundant, but we still need to pass a node >>>> + # Instance is not redundant, but we still need to pass a node >>>> # (which will be ignored) >>>> 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 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" % 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, >>>> - current_src_inst, >>>> - ] >>>> >>>> - # Some uses of this test might require that RAPI-only commands are >>>> used, >>>> - # and the checks are command-line based. >>>> + # pnode:snode are the *current* nodes, so we move it first to >>>> tnode:pnode >>>> + _InvokeMoveInstance(dest_instance.name, src_instance.name, >>>> rapi_pw_file.name, >>>> + master.primary, perform_checks, >>>> + target_nodes=(tnode.primary, pnode.primary)) >>>> >>>> - if perform_checks: >>>> - qa_utils.RunInstanceCheck(current_dest_inst, False) >>>> - >>>> - AssertEqual(StartLocalCommand(cmd).wait(), 0) >>>> - >>>> - if perform_checks: >>>> - qa_utils.RunInstanceCheck(current_src_inst, False) >>>> - qa_utils.RunInstanceCheck(current_dest_inst, True) >>>> + # And then back to pnode:snode >>>> + _InvokeMoveInstance(src_instance.name, dest_instance.name, >>>> rapi_pw_file.name, >>>> + master.primary, perform_checks, >>>> + target_nodes=(pnode.primary, snode.primary)) >>>> -- >>>> 1.9.0.rc1.175.g0b1dcb5 >>>> >>>> >>> >> >
