On May 22 14:56, 'Helga Velroyen' via ganeti-devel wrote:
> This patch fixes some lint errors in the 'move-instance'
> tool that showed up with a newer version of lint.
>
> Signed-off-by: Helga Velroyen <[email protected]>
> ---
> tools/move-instance | 207
> +++++++++++++++++++++++++++-------------------------
> 1 file changed, 109 insertions(+), 98 deletions(-)
>
> diff --git a/tools/move-instance b/tools/move-instance
> index 4d9abf4..02b1c8b 100755
> --- a/tools/move-instance
> +++ b/tools/move-instance
> @@ -43,7 +43,6 @@ from ganeti import rapi
> from ganeti import errors
>
> import ganeti.rapi.client # pylint: disable=W0611
> -import ganeti.rapi.client_utils
> from ganeti.rapi.client import UsesRapiClient
>
>
> @@ -551,6 +550,48 @@ class MoveDestExecutor(object):
> mrt.dest_to_source.release()
>
> @staticmethod
> + def GetDisks(instance):
> + disks = []
> + for idisk in instance["disks"]:
> + odisk = {
> + constants.IDISK_SIZE: idisk["size"],
> + constants.IDISK_MODE: idisk["mode"],
> + constants.IDISK_NAME: str(idisk.get("name")),
> + }
> + spindles = idisk.get("spindles")
> + if spindles is not None:
> + odisk[constants.IDISK_SPINDLES] = spindles
> + disks.append(odisk)
> + return disks
> +
> + @staticmethod
> + def GetNics(instance, override_nics):
> + try:
> + nics = [{
> + constants.INIC_IP: ip,
> + constants.INIC_MAC: mac,
> + constants.INIC_MODE: mode,
> + constants.INIC_LINK: link,
> + constants.INIC_VLAN: vlan,
> + constants.INIC_NETWORK: network,
> + constants.INIC_NAME: nic_name
> + } for nic_name, _, ip, mac, mode, link, vlan, network, _
> + in instance["nics"]]
> + except ValueError:
> + raise Error("Received NIC information does not match expected format; "
> + "Do the versions of this tool and the source cluster
> match?")
> +
> + if len(override_nics) > len(nics):
> + raise Error("Can not create new NICs")
> +
> + if override_nics:
> + assert len(override_nics) <= len(nics)
> + for idx, (nic, override) in enumerate(zip(nics, override_nics)):
> + nics[idx] = objects.FillDict(nic, override)
> +
> + return nics
> +
> + @staticmethod
> def _CreateInstance(cl, name, pnode, snode, compress, iallocator,
> dest_disk_template, instance, expinfo,
> override_hvparams,
> override_beparams, override_osparams, override_nics,
> @@ -593,45 +634,9 @@ class MoveDestExecutor(object):
> else:
> disk_template = instance["disk_template"]
>
> - disks = []
> - for idisk in instance["disks"]:
> - odisk = {
> - constants.IDISK_SIZE: idisk["size"],
> - constants.IDISK_MODE: idisk["mode"],
> - constants.IDISK_NAME: str(idisk.get("name")),
> - }
> - spindles = idisk.get("spindles")
> - if spindles is not None:
> - odisk[constants.IDISK_SPINDLES] = spindles
> - disks.append(odisk)
> -
> - try:
> - nics = [{
> - constants.INIC_IP: ip,
> - constants.INIC_MAC: mac,
> - constants.INIC_MODE: mode,
> - constants.INIC_LINK: link,
> - constants.INIC_VLAN: vlan,
> - constants.INIC_NETWORK: network,
> - constants.INIC_NAME: nic_name
> - } for nic_name, _, ip, mac, mode, link, vlan, network, _
> - in instance["nics"]]
> - except ValueError:
> - raise Error("Received NIC information does not match expected format; "
> - "Do the versions of this tool and the source cluster
> match?")
> -
> - if len(override_nics) > len(nics):
> - raise Error("Can not create new NICs")
> -
> - if override_nics:
> - assert len(override_nics) <= len(nics)
> - for idx, (nic, override) in enumerate(zip(nics, override_nics)):
> - nics[idx] = objects.FillDict(nic, override)
> -
> - if instance["os"]:
> - os_type = instance["os"]
> - else:
> - os_type = None
> + disks = cl.GetDisks(instance)
> + nics = cl.GetNicks(instance, override_nics)
> + os_type = instance.get("os", default=None)
>
> # TODO: Should this be the actual up/down status? (run_state)
> start = (instance["config_state"] == "up")
> @@ -639,17 +644,9 @@ class MoveDestExecutor(object):
> assert len(disks) == len(instance["disks"])
> assert len(nics) == len(instance["nics"])
>
> - inst_beparams = instance["be_instance"]
> - if not inst_beparams:
> - inst_beparams = {}
> -
> - inst_hvparams = instance["hv_instance"]
> - if not inst_hvparams:
> - inst_hvparams = {}
> -
> - inst_osparams = instance["os_instance"]
> - if not inst_osparams:
> - inst_osparams = {}
> + inst_beparams = instance.get("be_instance", default={})
> + inst_hvparams = instance.get("hv_instance", default={})
> + inst_osparams = instance.get("os_instance", default={})
Just to raise a note (as discussed offline) that that the new code is
not equivalent to the one before.
LGTM.
Thanks,
Jose
> return cl.CreateInstance(constants.INSTANCE_REMOTE_IMPORT,
> name, disk_template, disks, nics,
> @@ -690,7 +687,7 @@ class MoveSourceExecutor(object):
> logging.info("Retrieving instance information from source cluster")
> instinfo = self._GetInstanceInfo(src_client, mrt.PollJob,
> mrt.move.src_instance_name)
> - if (instinfo["disk_template"] in constants.DTS_FILEBASED):
> + if instinfo["disk_template"] in constants.DTS_FILEBASED:
> raise Error("Inter-cluster move of file-based instances is not"
> " supported.")
>
> @@ -894,35 +891,17 @@ def ParseOptions():
> return (parser, options, args)
>
>
> -def CheckOptions(parser, options, args):
> - """Checks options and arguments for validity.
> -
> - """
> - if len(args) < 3:
> - parser.error("Not enough arguments")
> -
> - src_cluster_name = args.pop(0)
> - dest_cluster_name = args.pop(0)
> - instance_names = args
> -
> - assert len(instance_names) > 0
> -
> - # TODO: Remove once using system default paths for SSL certificate
> - # verification is implemented
> - if not options.src_ca_file:
> - parser.error("Missing source cluster CA file")
> -
> - if options.parallel < 1:
> - parser.error("Number of simultaneous moves must be >= 1")
> -
> +def _CheckAllocatorOptions(parser, options):
> if (bool(options.iallocator) and
> bool(options.dest_primary_node or options.dest_secondary_node)):
> parser.error("Destination node and iallocator options exclude each
> other")
>
> - if (not options.iallocator and (options.opportunistic_tries > 0)):
> + if not options.iallocator and (options.opportunistic_tries > 0):
> parser.error("Opportunistic instance creation can only be used with an"
> " iallocator")
>
> +
> +def _CheckOpportunisticLockingOptions(parser, options):
> tries_specified = options.opportunistic_tries is not None
> delay_specified = options.opportunistic_delay is not None
> if tries_specified:
> @@ -939,6 +918,8 @@ def CheckOptions(parser, options, args):
> # The default values will be provided later
> pass
>
> +
> +def _CheckInstanceOptions(parser, options, instance_names):
> if len(instance_names) == 1:
> # Moving one instance only
> if options.hvparams:
> @@ -959,6 +940,32 @@ def CheckOptions(parser, options, args):
> " --backend-parameters, --os-parameters and --net can"
> " only be used when moving exactly one instance")
>
> +
> +def CheckOptions(parser, options, args):
> + """Checks options and arguments for validity.
> +
> + """
> + if len(args) < 3:
> + parser.error("Not enough arguments")
> +
> + src_cluster_name = args.pop(0)
> + dest_cluster_name = args.pop(0)
> + instance_names = args
> +
> + assert len(instance_names) > 0
> +
> + # TODO: Remove once using system default paths for SSL certificate
> + # verification is implemented
> + if not options.src_ca_file:
> + parser.error("Missing source cluster CA file")
> +
> + if options.parallel < 1:
> + parser.error("Number of simultaneous moves must be >= 1")
> +
> + _CheckAllocatorOptions(parser, options)
> + _CheckOpportunisticLockingOptions(parser, options)
> + _CheckInstanceOptions(parser, options, instance_names)
> +
> return (src_cluster_name, dest_cluster_name, instance_names)
>
>
> @@ -979,6 +986,33 @@ def ExitWithError(message):
> sys.exit(constants.EXIT_FAILURE)
>
>
> +def _PrepareListOfInstanceMoves(options, instance_names):
> + moves = []
> + for src_instance_name in instance_names:
> + if options.dest_instance_name:
> + assert len(instance_names) == 1
> + # Rename instance
> + dest_instance_name = options.dest_instance_name
> + else:
> + dest_instance_name = src_instance_name
> +
> + moves.append(InstanceMove(src_instance_name, dest_instance_name,
> + options.dest_primary_node,
> + options.dest_secondary_node,
> + options.compress,
> + options.iallocator,
> + options.dest_disk_template,
> + options.hvparams,
> + options.beparams,
> + options.osparams,
> + options.nics,
> + options.opportunistic_tries,
> + options.opportunistic_delay))
> +
> + assert len(moves) == len(instance_names)
> + return moves
> +
> +
> @UsesRapiClient
> def main():
> """Main routine.
> @@ -1011,30 +1045,7 @@ def main():
> ExitWithError("Target cluster does not have a default iallocator, "
> "please specify either destination nodes or an
> iallocator.")
>
> - # Prepare list of instance moves
> - moves = []
> - for src_instance_name in instance_names:
> - if options.dest_instance_name:
> - assert len(instance_names) == 1
> - # Rename instance
> - dest_instance_name = options.dest_instance_name
> - else:
> - dest_instance_name = src_instance_name
> -
> - moves.append(InstanceMove(src_instance_name, dest_instance_name,
> - options.dest_primary_node,
> - options.dest_secondary_node,
> - options.compress,
> - options.iallocator,
> - options.dest_disk_template,
> - options.hvparams,
> - options.beparams,
> - options.osparams,
> - options.nics,
> - options.opportunistic_tries,
> - options.opportunistic_delay))
> -
> - assert len(moves) == len(instance_names)
> + moves = _PrepareListOfInstanceMoves(options, instance_names)
>
> # Start workerpool
> wp = workerpool.WorkerPool("Move", options.parallel, MoveSourceWorker)
> --
> 1.9.1.423.g4596e3a
>
--
Jose Antonio Lopes
Ganeti Engineering
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
Steuernummer: 48/725/00206
Umsatzsteueridentifikationsnummer: DE813741370
