LGTM. Thanks, Jose
On Thu, Feb 06, 2014 at 05:24:15PM +0100, Santi Raffa wrote: > Modify InstanceSetParams to accept and process private parameters. > > Signed-off-by: Santi Raffa <[email protected]> > --- > lib/backend.py | 3 ++- > lib/client/gnt_instance.py | 7 ++++--- > lib/cmdlib/instance.py | 31 ++++++++++++++++++++++++++----- > lib/rpc/node.py | 8 ++++++-- > man/gnt-instance.rst | 1 + > src/Ganeti/OpCodes.hs | 1 + > test/hs/Test/Ganeti/OpCodes.hs | 31 ++++++++++++++++++++++++------- > 7 files changed, 64 insertions(+), 18 deletions(-) > > diff --git a/lib/backend.py b/lib/backend.py > index 5547a39..3d51704 100644 > --- a/lib/backend.py > +++ b/lib/backend.py > @@ -3604,7 +3604,8 @@ def ValidateOS(required, osname, checks, osparams): > @type checks: list > @param checks: list of the checks to run (currently only 'parameters') > @type osparams: dict > - @param osparams: dictionary with OS parameters > + @param osparams: dictionary with OS parameters, some of which may be > + private. > @rtype: boolean > @return: True if the validation passed, or False if the OS was not > found and L{required} was false > diff --git a/lib/client/gnt_instance.py b/lib/client/gnt_instance.py > index 9292de7..61483ce 100644 > --- a/lib/client/gnt_instance.py > +++ b/lib/client/gnt_instance.py > @@ -1310,9 +1310,9 @@ def SetInstanceParams(opts, args): > @return: the desired exit code > > """ > - if not (opts.nics or opts.disks or opts.disk_template or > - opts.hvparams or opts.beparams or opts.os or opts.osparams or > - opts.offline_inst or opts.online_inst or opts.runtime_mem or > + if not (opts.nics or opts.disks or opts.disk_template or opts.hvparams or > + opts.beparams or opts.os or opts.osparams or opts.osparams_private > + or opts.offline_inst or opts.online_inst or opts.runtime_mem or > opts.new_primary_node): > ToStderr("Please give at least one of the parameters.") > return 1 > @@ -1372,6 +1372,7 @@ def SetInstanceParams(opts, args): > runtime_mem=opts.runtime_mem, > os_name=opts.os, > osparams=opts.osparams, > + osparams_private=opts.osparams_private, > force_variant=opts.force_variant, > force=opts.force, > wait_for_sync=opts.wait_for_sync, > diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py > index 03f4583..4784c52 100644 > --- a/lib/cmdlib/instance.py > +++ b/lib/cmdlib/instance.py > @@ -2475,7 +2475,7 @@ class LUInstanceSetParams(LogicalUnit): > if not (self.op.nics or self.op.disks or self.op.disk_template or > self.op.hvparams or self.op.beparams or self.op.os_name or > self.op.osparams or self.op.offline is not None or > - self.op.runtime_mem or self.op.pnode): > + self.op.runtime_mem or self.op.pnode or > self.op.osparams_private): > raise errors.OpPrereqError("No changes submitted", errors.ECODE_INVAL) > > if self.op.hvparams: > @@ -3008,12 +3008,27 @@ class LUInstanceSetParams(LogicalUnit): > hvspecs) > > # osparams processing > - if self.op.osparams: > - i_osdict = GetUpdatedParams(self.instance.osparams, self.op.osparams) > - CheckOSParams(self, True, node_uuids, instance_os, i_osdict) > - self.os_inst = i_osdict # the new dict (without defaults) > + if self.op.osparams or self.op.osparams_private_cluster: > + public_parms = self.op.osparams or {} > + private_parms = self.op.osparams_private_cluster or {} > + dupe_keys = utils.GetRepeatedKeys(public_parms, private_parms) > + > + if dupe_keys: > + raise errors.OpPrereqError("OS parameters repeated multiple times: > %s" % > + utils.CommaJoin(dupe_keys)) > + > + self.os_inst = GetUpdatedParams(self.instance.osparams, > + public_parms) > + self.os_inst_private = GetUpdatedParams(self.instance.osparams_private, > + private_parms) > + > + CheckOSParams(self, True, node_uuids, instance_os, > + objects.FillDict(self.os_inst, > + self.os_inst_private)) > + > else: > self.os_inst = {} > + self.os_inst_private = {} > > #TODO(dynmem): do the appropriate check involving MINMEM > if (constants.BE_MAXMEM in self.op.beparams and not self.op.force and > @@ -3610,6 +3625,12 @@ class LUInstanceSetParams(LogicalUnit): > for key, val in self.op.osparams.iteritems(): > result.append(("os/%s" % key, val)) > > + if self.op.osparams_private: > + self.instance.osparams_private = self.os_inst_private > + for key, val in self.op.osparams_private.iteritems(): > + # Show the Private(...) blurb. > + result.append(("os_private/%s" % key, repr(val))) > + > if self.op.offline is None: > # Ignore > pass > diff --git a/lib/rpc/node.py b/lib/rpc/node.py > index 845d73c..dd50817 100644 > --- a/lib/rpc/node.py > +++ b/lib/rpc/node.py > @@ -501,8 +501,12 @@ class _RpcClientBase: > # name to the prep_fn, and serialise its return value > encode_args_fn = lambda node: map(compat.partial(self._encoder, node), > zip(map(compat.snd, argdefs), args)) > - pnbody = dict((n, serializer.DumpJson(prep_fn(n, encode_args_fn(n)))) > - for n in node_list) > + pnbody = dict( > + (n, > + serializer.DumpJson(prep_fn(n, encode_args_fn(n)), > + > private_encoder=serializer.EncodeWithPrivateFields)) > + for n in node_list > + ) > > result = self._proc(node_list, procedure, pnbody, read_timeout, > req_resolver_opts) > diff --git a/man/gnt-instance.rst b/man/gnt-instance.rst > index 5e4838d..0b47334 100644 > --- a/man/gnt-instance.rst > +++ b/man/gnt-instance.rst > @@ -1173,6 +1173,7 @@ MODIFY > | [\--new-primary=*node*] > | [\--os-type=*OS* [\--force-variant]] > | [{-O|\--os-parameters} *param*=*value*... ] > +| [--os-parameters-private *param*=*value*... ] > | [\--offline \| \--online] > | [\--submit] [\--print-job-id] > | [\--ignore-ipolicy] > diff --git a/src/Ganeti/OpCodes.hs b/src/Ganeti/OpCodes.hs > index bee6449..b6e26df 100644 > --- a/src/Ganeti/OpCodes.hs > +++ b/src/Ganeti/OpCodes.hs > @@ -648,6 +648,7 @@ $(genOpCode "OpCode" > pRemoteNodeUuid > , pOsNameChange > , pInstOsParams > + , pInstOsParamsPrivate > , pWaitForSync > , withDoc "Whether to mark the instance as offline" pOffline > , pIpConflictsCheck > diff --git a/test/hs/Test/Ganeti/OpCodes.hs b/test/hs/Test/Ganeti/OpCodes.hs > index c9be067..ef480ba 100644 > --- a/test/hs/Test/Ganeti/OpCodes.hs > +++ b/test/hs/Test/Ganeti/OpCodes.hs > @@ -285,13 +285,30 @@ instance Arbitrary OpCodes.OpCode where > OpCodes.OpInstanceQueryData <$> arbitrary <*> > genNodeNamesNE <*> arbitrary > "OP_INSTANCE_SET_PARAMS" -> > - OpCodes.OpInstanceSetParams <$> genFQDN <*> return Nothing <*> > - arbitrary <*> arbitrary <*> arbitrary <*> arbitrary <*> > - arbitrary <*> pure emptyJSObject <*> arbitrary <*> > - pure emptyJSObject <*> arbitrary <*> genMaybe genNodeNameNE <*> > - return Nothing <*> genMaybe genNodeNameNE <*> return Nothing <*> > - genMaybe genNameNE <*> pure emptyJSObject <*> arbitrary <*> > - arbitrary <*> arbitrary <*> arbitrary <*> arbitrary > + OpCodes.OpInstanceSetParams > + <$> genFQDN -- instance_name > + <*> return Nothing -- instance_uuid > + <*> arbitrary -- force > + <*> arbitrary -- force_variant > + <*> arbitrary -- ignore_ipolicy > + <*> arbitrary -- nics > + <*> arbitrary -- disks > + <*> pure emptyJSObject -- beparams > + <*> arbitrary -- runtime_mem > + <*> pure emptyJSObject -- hvparams > + <*> arbitrary -- disk_template > + <*> genMaybe genNodeNameNE -- pnode > + <*> return Nothing -- pnode_uuid > + <*> genMaybe genNodeNameNE -- remote_node > + <*> return Nothing -- remote_node_uuid > + <*> genMaybe genNameNE -- os_name > + <*> pure emptyJSObject -- osparams > + <*> genMaybe arbitraryPrivateJSObj -- osparams_private > + <*> arbitrary -- wait_for_sync > + <*> arbitrary -- offline > + <*> arbitrary -- conflicts_check > + <*> arbitrary -- hotplug > + <*> arbitrary -- hotplug_if_possible > "OP_INSTANCE_GROW_DISK" -> > OpCodes.OpInstanceGrowDisk <$> genFQDN <*> return Nothing <*> > arbitrary <*> arbitrary <*> arbitrary <*> arbitrary > -- > 1.9.0.rc1.175.g0b1dcb5 > -- 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
