On Tue, Feb 4, 2014 at 2:16 PM, Jose A. Lopes <[email protected]> wrote:
> On Tue, Feb 04, 2014 at 01:20:10PM +0100, Michele Tartara wrote:
>> On Fri, Jan 31, 2014 at 11:46 AM, Jose A. Lopes <[email protected]> wrote:
>> > * Add command-line interface flag to enable instance communication
>> > * Add instance communication parameter to the opcode that creates an
>> > instance
>> >
>> > Signed-off-by: Jose A. Lopes <[email protected]>
>> > ---
>> > lib/cli.py | 68
>> > +++++++++++++++++++++++++-----------------
>> > lib/client/gnt_instance.py | 3 +-
>> > src/Ganeti/Constants.hs | 6 +++-
>> > src/Ganeti/OpCodes.hs | 3 +-
>> > src/Ganeti/OpParams.hs | 8 ++++-
>> > test/hs/Test/Ganeti/OpCodes.hs | 3 +-
>> > 6 files changed, 58 insertions(+), 33 deletions(-)
>> >
>> > diff --git a/lib/cli.py b/lib/cli.py
>> > index ad6c5bc..21407ac 100644
>> > --- a/lib/cli.py
>> > +++ b/lib/cli.py
>> > @@ -1,7 +1,7 @@
>> > #
>> > #
>> >
>> > -# Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013 Google Inc.
>> > +# Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2013, 2014
>> > Google Inc.
>> > #
>> > # This program is free software; you can redistribute it and/or modify
>> > # it under the terms of the GNU General Public License as published by
>> > @@ -115,6 +115,7 @@ __all__ = [
>> > "IGNORE_SIZE_OPT",
>> > "INCLUDEDEFAULTS_OPT",
>> > "INTERVAL_OPT",
>> > + "INSTANCE_COMMUNICATION_OPT",
>> > "MAC_PREFIX_OPT",
>> > "MAINTAIN_NODE_HEALTH_OPT",
>> > "MASTER_NETDEV_OPT",
>> > @@ -1686,6 +1687,13 @@ HOTPLUG_IF_POSSIBLE_OPT =
>> > cli_option("--hotplug-if-possible",
>> > help="Hotplug devices in case"
>> > " hotplug is supported")
>> >
>> > +INSTANCE_COMMUNICATION_OPT = \
>> > + cli_option("-c", "--communication",
>> > + default=False,
>> > + dest="instance_communication",
>> > + help=constants.INSTANCE_COMMUNICATION_DOC,
>> > + type="bool")
>> > +
>> > #: Options provided by all commands
>> > COMMON_OPTS = [DEBUG_OPT, REASON_OPT]
>> >
>> > @@ -2727,6 +2735,7 @@ def GenericInstanceCreate(mode, opts, args):
>> > no_install = opts.no_install
>> > identify_defaults = False
>> > compress = constants.IEC_NONE
>> > + instance_communication = opts.instance_communication
>> > elif mode == constants.INSTANCE_IMPORT:
>> > start = False
>> > os_type = None
>> > @@ -2736,36 +2745,39 @@ def GenericInstanceCreate(mode, opts, args):
>> > no_install = None
>> > identify_defaults = opts.identify_defaults
>> > compress = opts.compress
>> > + instance_communication = False
>> > else:
>> > raise errors.ProgrammerError("Invalid creation mode %s" % mode)
>> >
>> > - op = opcodes.OpInstanceCreate(instance_name=instance,
>> > - disks=disks,
>> > - disk_template=opts.disk_template,
>> > - nics=nics,
>> > - conflicts_check=opts.conflicts_check,
>> > - pnode=pnode, snode=snode,
>> > - ip_check=opts.ip_check,
>> > - name_check=opts.name_check,
>> > - wait_for_sync=opts.wait_for_sync,
>> > - file_storage_dir=opts.file_storage_dir,
>> > - file_driver=opts.file_driver,
>> > - iallocator=opts.iallocator,
>> > - hypervisor=hypervisor,
>> > - hvparams=hvparams,
>> > - beparams=opts.beparams,
>> > - osparams=opts.osparams,
>> > - mode=mode,
>> > - start=start,
>> > - os_type=os_type,
>> > - force_variant=force_variant,
>> > - src_node=src_node,
>> > - src_path=src_path,
>> > - compress=compress,
>> > - tags=tags,
>> > - no_install=no_install,
>> > - identify_defaults=identify_defaults,
>> > - ignore_ipolicy=opts.ignore_ipolicy)
>> > + op = opcodes.OpInstanceCreate(
>> > + instance_name=instance,
>> > + disks=disks,
>> > + disk_template=opts.disk_template,
>> > + nics=nics,
>> > + conflicts_check=opts.conflicts_check,
>> > + pnode=pnode, snode=snode,
>> > + ip_check=opts.ip_check,
>> > + name_check=opts.name_check,
>> > + wait_for_sync=opts.wait_for_sync,
>> > + file_storage_dir=opts.file_storage_dir,
>> > + file_driver=opts.file_driver,
>> > + iallocator=opts.iallocator,
>> > + hypervisor=hypervisor,
>> > + hvparams=hvparams,
>> > + beparams=opts.beparams,
>> > + osparams=opts.osparams,
>> > + mode=mode,
>> > + start=start,
>> > + os_type=os_type,
>> > + force_variant=force_variant,
>> > + src_node=src_node,
>> > + src_path=src_path,
>> > + compress=compress,
>> > + tags=tags,
>> > + no_install=no_install,
>> > + identify_defaults=identify_defaults,
>> > + ignore_ipolicy=opts.ignore_ipolicy,
>> > + instance_communication=instance_communication)
>>
>> Usually, our style guide states that in python we should indent on the
>> same level as the open parenthesis.
>> I guess (I'm not counting it) that the line that is being added is too
>> long to be aligned in such a way.
>>
>> Provided this is the reason of the change, I agree with rewriting this
>> block of code this way, as long as it doesn't trigger lint errors.
>
> I will put it back as before as it turns out it does fit.
Even better!
LGTM, thanks.
Michele
>
> diff --git a/lib/cli.py b/lib/cli.py
> index 21407ac..cb28c09 100644
> --- a/lib/cli.py
> +++ b/lib/cli.py
> @@ -2749,35 +2749,34 @@ def GenericInstanceCreate(mode, opts, args):
> else:
> raise errors.ProgrammerError("Invalid creation mode %s" % mode)
>
> - op = opcodes.OpInstanceCreate(
> - instance_name=instance,
> - disks=disks,
> - disk_template=opts.disk_template,
> - nics=nics,
> - conflicts_check=opts.conflicts_check,
> - pnode=pnode, snode=snode,
> - ip_check=opts.ip_check,
> - name_check=opts.name_check,
> - wait_for_sync=opts.wait_for_sync,
> - file_storage_dir=opts.file_storage_dir,
> - file_driver=opts.file_driver,
> - iallocator=opts.iallocator,
> - hypervisor=hypervisor,
> - hvparams=hvparams,
> - beparams=opts.beparams,
> - osparams=opts.osparams,
> - mode=mode,
> - start=start,
> - os_type=os_type,
> - force_variant=force_variant,
> - src_node=src_node,
> - src_path=src_path,
> - compress=compress,
> - tags=tags,
> - no_install=no_install,
> - identify_defaults=identify_defaults,
> - ignore_ipolicy=opts.ignore_ipolicy,
> - instance_communication=instance_communication)
> + op = opcodes.OpInstanceCreate(instance_name=instance,
> + disks=disks,
> + disk_template=opts.disk_template,
> + nics=nics,
> + conflicts_check=opts.conflicts_check,
> + pnode=pnode, snode=snode,
> + ip_check=opts.ip_check,
> + name_check=opts.name_check,
> + wait_for_sync=opts.wait_for_sync,
> + file_storage_dir=opts.file_storage_dir,
> + file_driver=opts.file_driver,
> + iallocator=opts.iallocator,
> + hypervisor=hypervisor,
> + hvparams=hvparams,
> + beparams=opts.beparams,
> + osparams=opts.osparams,
> + mode=mode,
> + start=start,
> + os_type=os_type,
> + force_variant=force_variant,
> + src_node=src_node,
> + src_path=src_path,
> + compress=compress,
> + tags=tags,
> + no_install=no_install,
> + identify_defaults=identify_defaults,
> + ignore_ipolicy=opts.ignore_ipolicy,
> +
> instance_communication=instance_communication)
>
>
>> >
>> > SubmitOrSend(op, opts)
>> > return 0
>> > diff --git a/lib/client/gnt_instance.py b/lib/client/gnt_instance.py
>> > index e761a9a..f6d270b 100644
>> > --- a/lib/client/gnt_instance.py
>> > +++ b/lib/client/gnt_instance.py
>> > @@ -1,7 +1,7 @@
>> > #
>> > #
>> >
>> > -# Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012 Google Inc.
>> > +# Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012, 2014 Google Inc.
>> > #
>> > # This program is free software; you can redistribute it and/or modify
>> > # it under the terms of the GNU General Public License as published by
>> > @@ -1479,6 +1479,7 @@ add_opts = [
>> > FORCE_VARIANT_OPT,
>> > NO_INSTALL_OPT,
>> > IGNORE_IPOLICY_OPT,
>> > + INSTANCE_COMMUNICATION_OPT,
>> > ]
>> >
>> > commands = {
>> > diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs
>> > index 533ec6a..9e008fe 100644
>> > --- a/src/Ganeti/Constants.hs
>> > +++ b/src/Ganeti/Constants.hs
>> > @@ -16,7 +16,7 @@ imported.
>> >
>> > {-
>> >
>> > -Copyright (C) 2013 Google Inc.
>> > +Copyright (C) 2013, 2014 Google Inc.
>>
>> Thanks for updating the copyright dates in the files you touched. :-)
>>
>> >
>> > This program is free software; you can redistribute it and/or modify
>> > it under the terms of the GNU General Public License as published by
>> > @@ -4764,6 +4764,10 @@ glusterPortDefault = 24007
>> >
>> > -- * Instance communication
>> >
>> > +instanceCommunicationDoc :: String
>> > +instanceCommunicationDoc =
>> > + "Enable or disable the communication mechanism for an instance"
>> > +
>> > instanceCommunicationNetwork :: String
>> > instanceCommunicationNetwork = "ganeti:network:communication"
>> >
>> > diff --git a/src/Ganeti/OpCodes.hs b/src/Ganeti/OpCodes.hs
>> > index 9c9d736..bee6449 100644
>> > --- a/src/Ganeti/OpCodes.hs
>> > +++ b/src/Ganeti/OpCodes.hs
>> > @@ -7,7 +7,7 @@
>> >
>> > {-
>> >
>> > -Copyright (C) 2009, 2010, 2011, 2012, 2013 Google Inc.
>> > +Copyright (C) 2009, 2010, 2011, 2012, 2013, 2014 Google Inc.
>> >
>> > This program is free software; you can redistribute it and/or modify
>> > it under the terms of the GNU General Public License as published by
>> > @@ -452,6 +452,7 @@ $(genOpCode "OpCode"
>> > , pBackupCompress
>> > , pStartInstance
>> > , pInstTags
>> > + , pInstanceCommunication
>> > ],
>> > "instance_name")
>> > , ("OpInstanceMultiAlloc",
>> > diff --git a/src/Ganeti/OpParams.hs b/src/Ganeti/OpParams.hs
>> > index 11d62f6..127c5d6 100644
>> > --- a/src/Ganeti/OpParams.hs
>> > +++ b/src/Ganeti/OpParams.hs
>> > @@ -12,7 +12,7 @@ module.
>> >
>> > {-
>> >
>> > -Copyright (C) 2012 Google Inc.
>> > +Copyright (C) 2012, 2014 Google Inc.
>> >
>> > This program is free software; you can redistribute it and/or modify
>> > it under the terms of the GNU General Public License as published by
>> > @@ -44,6 +44,7 @@ module Ganeti.OpParams
>> > , SetParamsMods(..)
>> > , ExportTarget(..)
>> > , pInstanceName
>> > + , pInstanceCommunication
>> > , pInstanceUuid
>> > , pInstances
>> > , pName
>> > @@ -984,6 +985,11 @@ pInstanceName =
>> > withDoc "A required instance name (for single-instance LUs)" $
>> > simpleField "instance_name" [t| String |]
>> >
>> > +pInstanceCommunication :: Field
>> > +pInstanceCommunication =
>> > + withDoc C.instanceCommunicationDoc $
>> > + defaultFalse "instance_communication"
>> > +
>> > pForceVariant :: Field
>> > pForceVariant =
>> > withDoc "Whether to force an unknown OS variant" $
>> > diff --git a/test/hs/Test/Ganeti/OpCodes.hs
>> > b/test/hs/Test/Ganeti/OpCodes.hs
>> > index 5157801..90bc091 100644
>> > --- a/test/hs/Test/Ganeti/OpCodes.hs
>> > +++ b/test/hs/Test/Ganeti/OpCodes.hs
>> > @@ -241,7 +241,8 @@ instance Arbitrary OpCodes.OpCode where
>> > genMaybe genNodeNameNE <*> return Nothing <*> genMaybe (pure
>> > []) <*>
>> > genMaybe genNodeNameNE <*> arbitrary <*> genMaybe genNodeNameNE
>> > <*>
>> > return Nothing <*> genMaybe genNodeNameNE <*> genMaybe
>> > genNameNE <*>
>> > - arbitrary <*> arbitrary <*> (genTags >>= mapM mkNonEmpty)
>> > + arbitrary <*> arbitrary <*> (genTags >>= mapM mkNonEmpty) <*>
>> > + arbitrary
>> > "OP_INSTANCE_MULTI_ALLOC" ->
>> > OpCodes.OpInstanceMultiAlloc <$> arbitrary <*> genMaybe genNameNE
>> > <*>
>> > pure []
>> > --
>> > 1.8.5.3
>> >
>>
>> LGTM, thanks.
>> Michele
>>
>> --
>> 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
>
> --
> 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
--
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
