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.
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
