On Tue, Feb 04, 2014 at 05:00:30PM +0100, Santi Raffa wrote:
> Signed-off-by: Santi Raffa <[email protected]>
> ---
> lib/backend.py | 4 +++
> lib/cli.py | 3 +++
> lib/client/gnt_instance.py | 4 ++-
> lib/cmdlib/instance.py | 31 ++++++++++++++++++++---
> man/gnt-instance.rst | 14 +++++++++++
> src/Ganeti/Constants.hs | 3 +++
> src/Ganeti/OpCodes.hs | 2 ++
> src/Ganeti/OpParams.hs | 22 ++++++++++++++++
> test/hs/Test/Ganeti/OpCodes.hs | 50
> +++++++++++++++++++++++++++++--------
> test/py/cmdlib/instance_unittest.py | 1 +
> 10 files changed, 118 insertions(+), 16 deletions(-)
>
> diff --git a/lib/backend.py b/lib/backend.py
> index 3d51704..dfc8690 100644
> --- a/lib/backend.py
> +++ b/lib/backend.py
> @@ -3266,6 +3266,10 @@ def FinalizeExport(instance, snap_disks):
> for name, value in instance.osparams.items():
> config.set(constants.INISECT_OSP, name, str(value))
>
> + config.add_section(constants.INISECT_OSP_PRIVATE)
> + for name, value in instance.osparams_private.items():
> + config.set(constants.INISECT_OSP_PRIVATE, name, str(value.Get()))
> +
> utils.WriteFile(utils.PathJoin(destdir, constants.EXPORT_CONF_FILE),
> data=config.Dumps())
> shutil.rmtree(finaldestdir, ignore_errors=True)
> diff --git a/lib/cli.py b/lib/cli.py
> index 5a99da6..e7f1755 100644
> --- a/lib/cli.py
> +++ b/lib/cli.py
> @@ -2764,6 +2764,7 @@ def GenericInstanceCreate(mode, opts, args):
> no_install = None
> identify_defaults = opts.identify_defaults
> compress = opts.compress
> + opts.osparams_secret = None
Please follow the structure of the current code, namely,
if mode == constants.INSTANCE_CREATE:
...
osparams_secret = opts.osparams_secret
else:
...
osparams_secret = None
> else:
> raise errors.ProgrammerError("Invalid creation mode %s" % mode)
>
> @@ -2783,6 +2784,8 @@ def GenericInstanceCreate(mode, opts, args):
> hvparams=hvparams,
> beparams=opts.beparams,
> osparams=opts.osparams,
> + osparams_private=opts.osparams_private,
> + osparams_secret=opts.osparams_secret,
> mode=mode,
> start=start,
> os_type=os_type,
> diff --git a/lib/client/gnt_instance.py b/lib/client/gnt_instance.py
> index 980368f..b4deaa7 100644
> --- a/lib/client/gnt_instance.py
> +++ b/lib/client/gnt_instance.py
> @@ -1484,7 +1484,9 @@ add_opts = [
>
> commands = {
> "add": (
> - AddInstance, [ArgHost(min=1, max=1)], COMMON_CREATE_OPTS + add_opts,
> + AddInstance, [ArgHost(min=1, max=1)],
> + COMMON_CREATE_OPTS + add_opts + [OSPARAMS_NOLOG_OPT,
> + OSPARAMS_NOLOG_NOSAVE_OPT],
> "[...] -t disk-type -n node[:secondary-node] -o os-type <name>",
> "Creates and adds a new instance to the cluster"),
> "batch-create": (
> diff --git a/lib/cmdlib/instance.py b/lib/cmdlib/instance.py
> index 86247c5..2f38a01 100644
> --- a/lib/cmdlib/instance.py
> +++ b/lib/cmdlib/instance.py
> @@ -37,6 +37,7 @@ from ganeti import masterd
> from ganeti import netutils
> from ganeti import objects
> from ganeti import pathutils
> +from ganeti import serializer
> import ganeti.rpc.node as rpc
> from ganeti import utils
>
> @@ -836,6 +837,12 @@ class LUInstanceCreate(LogicalUnit):
> if name not in self.op.osparams:
> self.op.osparams[name] = value
>
> + if einfo.has_section(constants.INISECT_OSP_PRIVATE):
> + # use the parameters, without overriding
> + for name, value in einfo.items(constants.INISECT_OSP_PRIVATE):
> + if name not in self.op.osparams_private:
> + self.op.osparams_private[name] = serializer.Private(value,
> descr=name)
> +
> def _RevertToDefaults(self, cluster):
> """Revert the instance parameters to the default values.
>
> @@ -862,6 +869,11 @@ class LUInstanceCreate(LogicalUnit):
> if name in os_defs and os_defs[name] == self.op.osparams[name]:
> del self.op.osparams[name]
Missing comment, for consistency.
> + os_defs_ = cluster.SimpleFillOS(self.op.os_type, {}, {})
Aren't these keyword arguments (at the end of the call) ?
If so, there is another call just above.
And aren't secret params missing also?
> + for name in self.op.osparams_private.keys():
> + if name in os_defs_ and os_defs_[name] ==
> self.op.osparams_private[name]:
> + del self.op.osparams_private[name]
> +
> def _CalculateFileStorageDir(self):
> """Calculate final instance file storage dir.
>
> @@ -966,7 +978,15 @@ class LUInstanceCreate(LogicalUnit):
> self.be_full = _ComputeFullBeParams(self.op, cluster)
>
> # build os parameters
> - self.os_full = cluster.SimpleFillOS(self.op.os_type, self.op.osparams)
> + if self.op.osparams_private is None:
> + self.op.osparams_private = serializer.PrivateDict()
> + if self.op.osparams_secret is None:
> + self.op.osparams_secret = serializer.PrivateDict()
> +
> + self.os_full = cluster.SimpleFillOS(self.op.os_type,
> + self.op.osparams,
> + self.op.osparams_private,
> + self.op.osparams_secret)
Keyword args here too?
Thanks,
Jose
> # now that hvp/bep are in final format, let's reset to defaults,
> # if told to do so
> @@ -1321,6 +1341,7 @@ class LUInstanceCreate(LogicalUnit):
> hvparams=self.op.hvparams,
> hypervisor=self.op.hypervisor,
> osparams=self.op.osparams,
> + osparams_private=self.op.osparams_private,
> )
>
> if self.op.tags:
> @@ -1417,7 +1438,9 @@ class LUInstanceCreate(LogicalUnit):
> feedback_fn("* running the instance OS create scripts...")
> # FIXME: pass debug option from opcode to backend
> os_add_result = \
> - self.rpc.call_instance_os_add(self.pnode.uuid, (iobj, None),
> False,
> + self.rpc.call_instance_os_add(self.pnode.uuid,
> + (iobj, self.op.osparams_secret),
> + False,
> self.op.debug_level)
> if pause_sync:
> feedback_fn("* resuming disk sync")
> @@ -2997,9 +3020,9 @@ class LUInstanceSetParams(LogicalUnit):
> hvspecs)
>
> # osparams processing
> - if self.op.osparams or self.op.osparams_private_cluster:
> + if self.op.osparams or self.op.osparams_private:
> public_parms = self.op.osparams or {}
> - private_parms = self.op.osparams_private_cluster or {}
> + private_parms = self.op.osparams_private or {}
> dupe_keys = utils.GetRepeatedKeys(public_parms, private_parms)
>
> if dupe_keys:
> diff --git a/man/gnt-instance.rst b/man/gnt-instance.rst
> index cc23891..f41cfd7 100644
> --- a/man/gnt-instance.rst
> +++ b/man/gnt-instance.rst
> @@ -37,6 +37,8 @@ ADD
> | [{-B|\--backend-parameters} *BEPARAMS*]
> | [{-H|\--hypervisor-parameters} *HYPERVISOR* [: option=*value*... ]]
> | [{-O|\--os-parameters} *param*=*value*... ]
> +| [--os-parameters-private *param*=*value*... ]
> +| [--os-parameters-secret *param*=*value*... ]
> | [\--file-storage-dir *dir\_path*] [\--file-driver {loop \| blktap \|
> blktap2}]
> | {{-n|\--node} *node[:secondary-node]* \| {-I|\--iallocator} *name*}
> | {{-o|\--os-type} *os-type*}
> @@ -826,6 +828,18 @@ a hypothetical ``dhcp`` parameter to yes can be achieved
> by::
>
> gnt-instance add -O dhcp=yes ...
>
> +You can also specify OS parameters that should not be logged but reused
> +at the next reinstall with ``--os-parameters-private`` and OS parameters
> +that should not be logged or saved to configuration with
> +``--os-parameters-secret``. Bear in mind that:
> +
> + * Launching the daemons in debug mode will cause debug logging to
> + happen, which leaks private and secret parameters to the log files.
> + Do not use the debug mode in production. Deamons will emit a warning
> + on startup if they are in debug mode.
> + * You will have to pass again all ``--os-parameters-secret`` parameters
> + should you want to reinstall this instance.
> +
> The ``-I (--iallocator)`` option specifies the instance allocator plugin
> to use (``.`` means the default allocator). If you pass in this option
> the allocator will select nodes for this instance automatically, so you
> diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs
> index 642ae5b..e16b32c 100644
> --- a/src/Ganeti/Constants.hs
> +++ b/src/Ganeti/Constants.hs
> @@ -1089,6 +1089,9 @@ inisectIns = "instance"
> inisectOsp :: String
> inisectOsp = "os"
>
> +inisectOspPrivate :: String
> +inisectOspPrivate = "os_private"
> +
> -- * Dynamic device modification
>
> ddmAdd :: String
> diff --git a/src/Ganeti/OpCodes.hs b/src/Ganeti/OpCodes.hs
> index 8762198..362b2e8 100644
> --- a/src/Ganeti/OpCodes.hs
> +++ b/src/Ganeti/OpCodes.hs
> @@ -438,6 +438,8 @@ $(genOpCode "OpCode"
> , pInstNics
> , pNoInstall
> , pInstOsParams
> + , pInstOsParamsPrivate
> + , pInstOsParamsSecret
> , pInstOs
> , pPrimaryNode
> , pPrimaryNodeUuid
> diff --git a/src/Ganeti/OpParams.hs b/src/Ganeti/OpParams.hs
> index cda57e5..9dcb5e6 100644
> --- a/src/Ganeti/OpParams.hs
> +++ b/src/Ganeti/OpParams.hs
> @@ -119,6 +119,8 @@ module Ganeti.OpParams
> , pClusterOsParams
> , pClusterOsParamsPrivate
> , pInstOsParams
> + , pInstOsParamsPrivate
> + , pInstOsParamsSecret
> , pCandidatePoolSize
> , pMaxRunningJobs
> , pUidPool
> @@ -1096,6 +1098,26 @@ pInstOsParams =
> defaultField [| toJSObject [] |] $
> simpleField "osparams" [t| JSObject JSValue |]
>
> +pInstOsParamsPrivate :: Field
> +pInstOsParamsPrivate =
> + field { fieldDoc = "meh"
> + -- fieldShow = Just [| \(_,y) -> showPrivateJSObject y |]
> + }
> + where field =
> + withDoc "Private OS parameters for instance" .
> + optionalField $
> + simpleField "osparams_private" [t| JSObject (Private JSValue) |]
> +
> +pInstOsParamsSecret :: Field
> +pInstOsParamsSecret =
> + field { fieldDoc = "meh"
> + -- fieldShow = Just [| \(_,y) -> showPrivateJSObject y |]
> + }
> + where field =
> + withDoc "Secret OS parameters for instance" .
> + optionalField $
> + simpleField "osparams_secret" [t| JSObject (Private JSValue) |]
> +
> pPrimaryNode :: Field
> pPrimaryNode =
> withDoc "Primary node for an instance" $
> diff --git a/test/hs/Test/Ganeti/OpCodes.hs b/test/hs/Test/Ganeti/OpCodes.hs
> index eb9d940..56a76fc 100644
> --- a/test/hs/Test/Ganeti/OpCodes.hs
> +++ b/test/hs/Test/Ganeti/OpCodes.hs
> @@ -255,17 +255,45 @@ instance Arbitrary OpCodes.OpCode where
> return Nothing <*> genMaybe genNodeNameNE <*> return Nothing <*>
> genMaybe genNameNE <*> arbitrary
> "OP_INSTANCE_CREATE" ->
> - OpCodes.OpInstanceCreate <$> genFQDN <*> arbitrary <*>
> - arbitrary <*> arbitrary <*> arbitrary <*> arbitrary <*>
> - pure emptyJSObject <*> arbitrary <*> arbitrary <*> arbitrary <*>
> - genMaybe genNameNE <*> pure emptyJSObject <*> arbitrary <*>
> - genMaybe genNameNE <*> arbitrary <*> arbitrary <*> arbitrary <*>
> - arbitrary <*> arbitrary <*> arbitrary <*> pure emptyJSObject <*>
> - genMaybe genNameNE <*> genMaybe genNodeNameNE <*> return Nothing
> <*>
> - genMaybe genNodeNameNE <*> return Nothing <*> genMaybe (pure [])
> <*>
> - genMaybe genNodeNameNE <*> arbitrary <*> genMaybe genNodeNameNE <*>
> - return Nothing <*> genMaybe genNodeNameNE <*> genMaybe genNameNE
> <*>
> - arbitrary <*> arbitrary <*> (genTags >>= mapM mkNonEmpty)
> + OpCodes.OpInstanceCreate
> + <$> genFQDN -- instance_name
> + <*> arbitrary -- force_variant
> + <*> arbitrary -- wait_for_sync
> + <*> arbitrary -- name_check
> + <*> arbitrary -- ignore_ipolicy
> + <*> arbitrary -- opportunistic_locking
> + <*> pure emptyJSObject -- beparams
> + <*> arbitrary -- disks
> + <*> arbitrary -- disk_template
> + <*> arbitrary -- file_driver
> + <*> genMaybe genNameNE -- file_storage_dir
> + <*> pure emptyJSObject -- hvparams
> + <*> arbitrary -- hypervisor
> + <*> genMaybe genNameNE -- iallocator
> + <*> arbitrary -- identify_defaults
> + <*> arbitrary -- ip_check
> + <*> arbitrary -- conflicts_check
> + <*> arbitrary -- mode
> + <*> arbitrary -- nics
> + <*> arbitrary -- no_install
> + <*> pure emptyJSObject -- osparams
> + <*> genMaybe arbitraryPrivateJSObj -- osparams_private
> + <*> genMaybe arbitraryPrivateJSObj -- osparams_secret
> + <*> genMaybe genNameNE -- os_type
> + <*> genMaybe genNodeNameNE -- pnode
> + <*> return Nothing -- pnode_uuid
> + <*> genMaybe genNodeNameNE -- snode
> + <*> return Nothing -- snode_uuid
> + <*> genMaybe (pure []) -- source_handshake
> + <*> genMaybe genNodeNameNE -- source_instance_name
> + <*> arbitrary -- source_shutdown_timeout
> + <*> genMaybe genNodeNameNE -- source_x509_ca
> + <*> return Nothing -- src_node
> + <*> genMaybe genNodeNameNE -- src_node_uuid
> + <*> genMaybe genNameNE -- src_path
> + <*> arbitrary -- compress
> + <*> arbitrary -- start
> + <*> (genTags >>= mapM mkNonEmpty) -- tags
> "OP_INSTANCE_MULTI_ALLOC" ->
> OpCodes.OpInstanceMultiAlloc <$> arbitrary <*> genMaybe genNameNE <*>
> pure []
> diff --git a/test/py/cmdlib/instance_unittest.py
> b/test/py/cmdlib/instance_unittest.py
> index 23605d3..3fe3ee7 100644
> --- a/test/py/cmdlib/instance_unittest.py
> +++ b/test/py/cmdlib/instance_unittest.py
> @@ -465,6 +465,7 @@ class TestLUInstanceCreate(CmdlibTestCase):
> osparams={
> self.os_name_variant: {}
> },
> + osparams_private={},
> identify_defaults=True)
> self.ExecOpCode(op)
>
> --
> 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
