LGTM, thanks.

On Mon, Feb 24, 2014 at 11:20 AM, Hrvoje Ribicic <[email protected]> wrote:

> Interdiff necessary due to previous patch:
>
> diff --git a/tools/move-instance b/tools/move-instance
> index 30d453d..6eb543a 100755
> --- a/tools/move-instance
> +++ b/tools/move-instance
> @@ -329,10 +329,10 @@ class InstanceMove(object):
>      @param osparams: OS parameters to override
>      @type nics: dict or None
>      @param nics: NICs to override
> -    @type opportunistic_tries: int
> +    @type opportunistic_tries: int or None
>      @param opportunistic_tries: Number of opportunistic creation attempts
> to
>                                  perform
> -    @type opportunistic_delay: int
> +    @type opportunistic_delay: int or None
>      @param opportunistic_delay: Delay between successive creation
> attempts, in
>                                  seconds
>
> @@ -348,8 +348,16 @@ class InstanceMove(object):
>      self.beparams = beparams
>      self.osparams = osparams
>      self.nics = nics
> -    self.opportunistic_tries = opportunistic_tries
> -    self.opportunistic_delay = opportunistic_delay
> +
> +    if self.opportunistic_tries is not None:
> +      self.opportunistic_tries = opportunistic_tries
> +    else:
> +      self.opportunistic_tries = 0
> +
> +    if self.opportunistic_delay is not None:
> +      self.opportunistic_delay = opportunistic_delay
> +    else:
> +      self.opportunistic_delay =
> constants.DEFAULT_OPPORTUNISTIC_RETRY_INTERVAL
>
>      self.error_message = None
>
>
>
>
> On Fri, Feb 21, 2014 at 3:50 PM, Petr Pudlák <[email protected]> wrote:
>
>> LGTM, thanks.
>>
>>
>> On Tue, Feb 18, 2014 at 3:39 PM, Hrvoje Ribicic <[email protected]> wrote:
>>
>>> This patch allows opportunistic instance creations to be attempted,
>>> with a delay between them giving nodes the opportunity to become
>>> available. After the given number of opportunistic tries has been
>>> exhausted, a normal and blocking instance creation request is issued.
>>>
>>> Signed-off-by: Hrvoje Ribicic <[email protected]>
>>> ---
>>>  tools/move-instance | 76
>>> ++++++++++++++++++++++++++++++++++++++++++-----------
>>>  1 file changed, 60 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/tools/move-instance b/tools/move-instance
>>> index aa5e83d..60a9afa 100755
>>> --- a/tools/move-instance
>>> +++ b/tools/move-instance
>>> @@ -39,6 +39,7 @@ from ganeti import workerpool
>>>  from ganeti import objects
>>>  from ganeti import compat
>>>  from ganeti import rapi
>>> +from ganeti import errors
>>>
>>>  import ganeti.rapi.client # pylint: disable=W0611
>>>  import ganeti.rapi.client_utils
>>> @@ -297,7 +298,8 @@ class InstanceMove(object):
>>>    def __init__(self, src_instance_name, dest_instance_name,
>>>                 dest_pnode, dest_snode, compress, dest_iallocator,
>>>                 dest_disk_template, hvparams,
>>> -               beparams, osparams, nics):
>>> +               beparams, osparams, nics, opportunistic_tries,
>>> +               opportunistic_delay):
>>>      """Initializes this class.
>>>
>>>      @type src_instance_name: string
>>> @@ -323,6 +325,12 @@ class InstanceMove(object):
>>>      @param osparams: OS parameters to override
>>>      @type nics: dict or None
>>>      @param nics: NICs to override
>>> +    @type opportunistic_tries: int
>>> +    @param opportunistic_tries: Number of opportunistic creation
>>> attempts to
>>> +                                perform
>>> +    @type opportunistic_delay: int
>>> +    @param opportunistic_delay: Delay between successive creation
>>> attempts, in
>>> +                                seconds
>>>
>>>      """
>>>      self.src_instance_name = src_instance_name
>>> @@ -336,6 +344,8 @@ class InstanceMove(object):
>>>      self.beparams = beparams
>>>      self.osparams = osparams
>>>      self.nics = nics
>>> +    self.opportunistic_tries = opportunistic_tries
>>> +    self.opportunistic_delay = opportunistic_delay
>>>
>>>      self.error_message = None
>>>
>>> @@ -458,18 +468,45 @@ class MoveDestExecutor(object):
>>>
>>>      logging.info("Creating instance %s in remote-import mode",
>>>                   mrt.move.dest_instance_name)
>>> -    job_id = self._CreateInstance(dest_client,
>>> mrt.move.dest_instance_name,
>>> -                                  mrt.move.dest_pnode,
>>> mrt.move.dest_snode,
>>> -                                  mrt.move.compress,
>>> -                                  mrt.move.dest_iallocator,
>>> -                                  mrt.move.dest_disk_template,
>>> -                                  mrt.src_instinfo, mrt.src_expinfo,
>>> -                                  mrt.move.hvparams, mrt.move.beparams,
>>> -                                  mrt.move.beparams, mrt.move.nics)
>>> -    mrt.PollJob(dest_client, job_id,
>>> -                remote_import_fn=compat.partial(self._SetImportInfo,
>>> mrt))
>>> -
>>> -    logging.info("Import successful")
>>> +
>>> +    # Depending on whether opportunistic tries are enabled, we may have
>>> to
>>> +    # make multiple creation attempts
>>> +    creation_attempts = [True] * mrt.move.opportunistic_tries
>>> +
>>> +    # But the last one is never opportunistic, and will block until
>>> completion
>>> +    # or failure
>>> +    creation_attempts.append(False)
>>> +
>>> +    for is_attempt_opportunistic in creation_attempts:
>>> +      job_id = self._CreateInstance(dest_client,
>>> mrt.move.dest_instance_name,
>>> +                                    mrt.move.dest_pnode,
>>> mrt.move.dest_snode,
>>> +                                    mrt.move.compress,
>>> +                                    mrt.move.dest_iallocator,
>>> +                                    mrt.move.dest_disk_template,
>>> +                                    mrt.src_instinfo, mrt.src_expinfo,
>>> +                                    mrt.move.hvparams,
>>> mrt.move.beparams,
>>> +                                    mrt.move.beparams, mrt.move.nics,
>>> +                                    is_attempt_opportunistic
>>> +                                    )
>>> +
>>> +      try:
>>> +        # The completion of this block signifies that the import has
>>> been
>>> +        # completed successfullly
>>> +        mrt.PollJob(dest_client, job_id,
>>> +
>>>  remote_import_fn=compat.partial(self._SetImportInfo, mrt))
>>> +        logging.info("Import successful")
>>> +        return
>>> +      except errors.OpPrereqError, err:
>>> +        # Any exception in the non-opportunistic creation is to be
>>> passed on,
>>> +        # as well as exceptions apart from resources temporarily
>>> unavailable
>>> +        if not is_attempt_opportunistic or \
>>> +           err.args[1] != rapi.client.ECODE_TEMP_NORES:
>>> +          raise
>>> +
>>> +      logging.info("Opportunistic attempt unsuccessful, waiting %d
>>> seconds"
>>> +                   " before another creation attempt is made",
>>> +                   mrt.move.opportunistic_delay)
>>> +      time.sleep(mrt.move.opportunistic_delay)
>>>
>>>    @staticmethod
>>>    def _SetImportInfo(mrt, impinfo):
>>> @@ -490,7 +527,8 @@ class MoveDestExecutor(object):
>>>    @staticmethod
>>>    def _CreateInstance(cl, name, pnode, snode, compress, iallocator,
>>>                        dest_disk_template, instance, expinfo,
>>> override_hvparams,
>>> -                      override_beparams, override_osparams,
>>> override_nics):
>>> +                      override_beparams, override_osparams,
>>> override_nics,
>>> +                      is_attempt_opportunistic):
>>>      """Starts the instance creation in remote import mode.
>>>
>>>      @type cl: L{rapi.client.GanetiRapiClient}
>>> @@ -519,6 +557,8 @@ class MoveDestExecutor(object):
>>>      @param override_osparams: OS parameters to override
>>>      @type override_nics: dict or None
>>>      @param override_nics: NICs to override
>>> +    @type is_attempt_opportunistic: bool
>>> +    @param is_attempt_opportunistic: Whether to use opportunistic
>>> locking or not
>>>      @return: Job ID
>>>
>>>      """
>>> @@ -598,7 +638,9 @@ class MoveDestExecutor(object):
>>>                               hvparams=objects.FillDict(inst_hvparams,
>>>
>>> override_hvparams),
>>>                               osparams=objects.FillDict(inst_osparams,
>>> -
>>> override_osparams))
>>> +
>>> override_osparams),
>>> +
>>> opportunistic_locking=is_attempt_opportunistic
>>> +                             )
>>>
>>>
>>>  class MoveSourceExecutor(object):
>>> @@ -948,7 +990,9 @@ def main():
>>>                                options.hvparams,
>>>                                options.beparams,
>>>                                options.osparams,
>>> -                              options.nics))
>>> +                              options.nics,
>>> +                              options.opportunistic_tries,
>>> +                              options.opportunistic_delay))
>>>
>>>    assert len(moves) == len(instance_names)
>>>
>>> --
>>> 1.9.0.rc1.175.g0b1dcb5
>>>
>>>
>>
>

Reply via email to