Add private OS parameters to cluster and instance conf

This updates objects, constructors and mocks for Instance and Cluster
objects in Python and Haskell.
Signed-off-by: Santi Raffa <rsanti@google.com>
Reviewed-by: Jose A. Lopes <jabolopes@google.com>

lib/bootstrap.py

@@ -786,6 +786,8 @@ def InitCluster(cluster_name, mac_prefix, # pylint: disable=R0913, R0914
     disk_state_static=disk_state,
     enabled_disk_templates=enabled_disk_templates,
     candidate_certs=candidate_certs,
+    osparams={},
+    osparams_private_cluster={}
     )
   master_node_config = objects.Node(name=hostname.name,
                                     primary_ip=hostname.ip,

lib/luxi.py

@@ -95,7 +95,9 @@ class Client(cl.AbstractClient):
     return self.CallMethod(REQ_PICKUP_JOB, (job,))
 
   def SubmitJob(self, ops):
-    ops_state = map(lambda op: op.__getstate__(), ops)
+    ops_state = map(lambda op: op.__getstate__()
+                               if not isinstance(op, objects.ConfigObject)
+                               else op.ToDict(_with_private=True), ops)
     return self.CallMethod(REQ_SUBMIT_JOB, (ops_state, ))
 
   def SubmitJobToDrainedQueue(self, ops):

lib/objects.py

@@ -47,6 +47,7 @@ from ganeti import constants
 from ganeti import netutils
 from ganeti import outils
 from ganeti import utils
+from ganeti import serializer
 
 from socket import AF_INET
 
@@ -213,7 +214,7 @@ class ConfigObject(outils.ValidatedSlots):
 
     """
 
-  def ToDict(self):
+  def ToDict(self, _with_private=False):
     """Convert to a dict holding only standard python types.
 
     The generic routine just dumps all of this object's attributes in
@@ -222,6 +223,15 @@ class ConfigObject(outils.ValidatedSlots):
     which case the object should subclass the function in order to
     make sure all objects returned are only standard python types.
 
+    Private fields can be included or not with the _with_private switch.
+    The actual implementation of this switch is left for those subclassses
+    with private fields to implement.
+
+    @type _with_private: bool
+    @param _with_private: if True, the object will leak its private fields in
+                          the dictionary representation. If False, the values
+                          will be replaced with None.
+
     """
     result = {}
     for name in self.GetAllSlots():
@@ -333,13 +343,13 @@ class TaggableObject(ConfigObject):
     except KeyError:
       raise errors.TagError("Tag not found")
 
-  def ToDict(self):
+  def ToDict(self, _with_private=False):
     """Taggable-object-specific conversion to standard python types.
 
     This replaces the tags set with a list.
 
     """
-    bo = super(TaggableObject, self).ToDict()
+    bo = super(TaggableObject, self).ToDict(_with_private=_with_private)
 
     tags = bo.get("tags", None)
     if isinstance(tags, set):
@@ -388,14 +398,14 @@ class ConfigData(ConfigObject):
     "serial_no",
     ] + _TIMESTAMPS
 
-  def ToDict(self):
+  def ToDict(self, _with_private=False):
     """Custom function for top-level config data.
 
     This just replaces the list of instances, nodes and the cluster
     with standard python types.
 
     """
-    mydict = super(ConfigData, self).ToDict()
+    mydict = super(ConfigData, self).ToDict(_with_private=_with_private)
     mydict["cluster"] = mydict["cluster"].ToDict()
     for key in "nodes", "instances", "nodegroups", "networks":
       mydict[key] = outils.ContainerToDicts(mydict[key])
@@ -746,7 +756,8 @@ class Disk(ConfigObject):
     self.dynamic_params = dyn_disk_params
 
   # pylint: disable=W0221
-  def ToDict(self, include_dynamic_params=False):
+  def ToDict(self, include_dynamic_params=False,
+             _with_private=False):
     """Disk-specific conversion to standard python types.
 
     This replaces the children lists of objects with lists of
@@ -1063,6 +1074,7 @@ class Instance(TaggableObject):
     "hvparams",
     "beparams",
     "osparams",
+    "osparams_private",
     "admin_state",
     "nics",
     "disks",
@@ -1187,14 +1199,17 @@ class Instance(TaggableObject):
                                  " 0 to %d" % (idx, len(self.disks) - 1),
                                  errors.ECODE_INVAL)
 
-  def ToDict(self):
+  def ToDict(self, _with_private=False):
     """Instance-specific conversion to standard python types.
 
     This replaces the children lists of objects with lists of standard
     python types.
 
     """
-    bo = super(Instance, self).ToDict()
+    bo = super(Instance, self).ToDict(_with_private=_with_private)
+
+    if _with_private:
+      bo["osparams_private"] = self.osparams_private.Unprivate()
 
     for attr in "nics", "disks":
       alist = bo.get(attr, None)
@@ -1238,6 +1253,8 @@ class Instance(TaggableObject):
           pass
     if self.osparams is None:
       self.osparams = {}
+    if self.osparams_private is None:
+      self.osparams_private = serializer.PrivateDict()
     UpgradeBeParams(self.beparams)
     if self.disks_active is None:
       self.disks_active = self.admin_state == constants.ADMINST_UP
@@ -1407,11 +1424,11 @@ class Node(TaggableObject):
     if self.powered is None:
       self.powered = True
 
-  def ToDict(self):
+  def ToDict(self, _with_private=False):
     """Custom function for serializing.
 
     """
-    data = super(Node, self).ToDict()
+    data = super(Node, self).ToDict(_with_private=_with_private)
 
     hv_state = data.get("hv_state", None)
     if hv_state is not None:
@@ -1459,14 +1476,14 @@ class NodeGroup(TaggableObject):
     "networks",
     ] + _TIMESTAMPS + _UUID
 
-  def ToDict(self):
+  def ToDict(self, _with_private=False):
     """Custom function for nodegroup.
 
     This discards the members object, which gets recalculated and is only kept
     in memory.
 
     """
-    mydict = super(NodeGroup, self).ToDict()
+    mydict = super(NodeGroup, self).ToDict(_with_private=_with_private)
     del mydict["members"]
     return mydict
 
@@ -1559,6 +1576,7 @@ class Cluster(TaggableObject):
     "os_hvp",
     "beparams",
     "osparams",
+    "osparams_private_cluster",
     "nicparams",
     "ndparams",
     "diskparams",
@@ -1600,9 +1618,11 @@ class Cluster(TaggableObject):
     if self.os_hvp is None:
       self.os_hvp = {}
 
-    # osparams added before 2.2
     if self.osparams is None:
       self.osparams = {}
+    # osparams_private_cluster added in 2.12
+    if self.osparams_private_cluster is None:
+      self.osparams_private_cluster = {}
 
     self.ndparams = UpgradeNDParams(self.ndparams)
 
@@ -1719,11 +1739,17 @@ class Cluster(TaggableObject):
     """
     return self.enabled_hypervisors[0]
 
-  def ToDict(self):
+  def ToDict(self, _with_private=False):
     """Custom function for cluster.
 
     """
-    mydict = super(Cluster, self).ToDict()
+    mydict = super(Cluster, self).ToDict(_with_private=_with_private)
+
+    # Explicitly save private parameters.
+    if _with_private:
+      for os in mydict["osparams_private_cluster"]:
+        mydict["osparams_private_cluster"][os] = \
+          self.osparams_private_cluster[os].Unprivate()
 
     if self.tcpudp_port_pool is None:
       tcpudp_port_pool = []
@@ -1855,25 +1881,89 @@ class Cluster(TaggableObject):
     """
     return FillDict(self.nicparams.get(constants.PP_DEFAULT, {}), nicparams)
 
-  def SimpleFillOS(self, os_name, os_params):
+  def SimpleFillOS(self, os_name,
+                    os_params_public,
+                    os_params_private=None,
+                    os_params_secret=None):
     """Fill an instance's osparams dict with cluster defaults.
 
     @type os_name: string
     @param os_name: the OS name to use
-    @type os_params: dict
-    @param os_params: the dict to fill with default values
+    @type os_params_public: dict
+    @param os_params_public: the dict to fill with default values
+    @type os_params_private: dict
+    @param os_params_private: the dict with private fields to fill
+                              with default values. Not passing this field
+                              results in no private fields being added to the
+                              return value. Private fields will be wrapped in
+                              L{Private} objects.
+    @type os_params_secret: dict
+    @param os_params_secret: the dict with secret fields to fill
+                             with default values. Not passing this field
+                             results in no secret fields being added to the
+                             return value. Private fields will be wrapped in
+                             L{Private} objects.
     @rtype: dict
     @return: a copy of the instance's osparams with missing keys filled from
-        the cluster defaults
+        the cluster defaults. Private and secret parameters are not included
+        unless the respective optional parameters are supplied.
 
     """
     name_only = os_name.split("+", 1)[0]
-    # base OS
-    result = self.osparams.get(name_only, {})
-    # OS with variant
-    result = FillDict(result, self.osparams.get(os_name, {}))
-    # specified params
-    return FillDict(result, os_params)
+
+    defaults_base_public = self.osparams.get(name_only, {})
+    defaults_public = FillDict(defaults_base_public,
+                               self.osparams.get(os_name, {}))
+    params_public = FillDict(defaults_public, os_params_public)
+
+    if os_params_private is not None:
+      defaults_base_private = self.osparams_private_cluster.get(name_only, {})
+      defaults_private = FillDict(defaults_base_private,
+                                  self.osparams_private_cluster.get(os_name,
+                                                                    {}))
+      params_private = FillDict(defaults_private, os_params_private)
+    else:
+      params_private = {}
+
+    if os_params_secret is not None:
+      # There can't be default secret settings, so there's nothing to be done.
+      params_secret = os_params_secret
+    else:
+      params_secret = {}
+
+    # Enforce that the set of keys be distinct:
+    duplicate_keys = utils.GetRepeatedKeys(params_public,
+                                           params_private,
+                                           params_secret)
+    if not duplicate_keys:
+
+      # Actually update them:
+      params_public.update(params_private)
+      params_public.update(params_secret)
+
+      return params_public
+
+    else:
+
+      def formatter(keys):
+        return utils.CommaJoin(sorted(map(repr, keys))) if keys else "(none)"
+
+      #Lose the values.
+      params_public = set(params_public)
+      params_private = set(params_private)
+      params_secret = set(params_secret)
+
+      msg = """Cannot assign multiple values to OS parameters.
+
+      Conflicting OS parameters that would have been set by this operation:
+      - at public visibility:  {public}
+      - at private visibility: {private}
+      - at secret visibility:  {secret}
+      """.format(dupes=formatter(duplicate_keys),
+                 public=formatter(params_public & duplicate_keys),
+                 private=formatter(params_private & duplicate_keys),
+                 secret=formatter(params_secret & duplicate_keys))
+      raise errors.OpPrereqError(msg)
 
   @staticmethod
   def SimpleFillHvState(hv_state):
@@ -2061,7 +2151,7 @@ class _QueryResponseBase(ConfigObject):
     "fields",
     ]
 
-  def ToDict(self):
+  def ToDict(self, _with_private=False):
     """Custom function for serializing.
 
     """

src/Ganeti/Config.hs

@@ -276,7 +276,7 @@ getFilledInstBeParams cfg inst = do
   return $ fillBeParams parentParams (instBeparams inst)
 
 -- | Retrieves the instance os params, missing values filled with cluster
--- defaults.
+-- defaults. This does NOT include private and secret parameters.
 getFilledInstOsParams :: ConfigData -> Instance -> OsParams
 getFilledInstOsParams cfg inst =
   let osLookupName = takeWhile (/= '+') (instOs inst)

src/Ganeti/HTools/Program/Harep.hs

@@ -307,6 +307,8 @@ detectBroken nl inst =
                                  , opInstanceUuid = Nothing
                                  , opOsType = Nothing
                                  , opTempOsParams = Nothing
+                                 , opOsparamsPrivate = Nothing
+                                 , opOsparamsSecret = Nothing
                                  , opForceVariant = False
                                  }
            ])
@@ -359,6 +361,8 @@ detectBroken nl inst =
                                  , opInstanceUuid = Nothing
                                  , opOsType = Nothing
                                  , opTempOsParams = Nothing
+                                 , opOsparamsPrivate = Nothing
+                                 , opOsparamsSecret = Nothing
                                  , opForceVariant = False
                                  }
            ])

src/Ganeti/Objects.hs

@@ -31,6 +31,7 @@ Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
 module Ganeti.Objects
   ( HvParams
   , OsParams
+  , OsParamsPrivate
   , PartialNicParams(..)
   , FilledNicParams(..)
   , fillNicParams
@@ -123,6 +124,7 @@ type HvParams = Container JSValue
 -- container, since the keys are dynamically declared by the OSes, and
 -- the values are always strings.
 type OsParams = Container String
+type OsParamsPrivate = Container (Private String)
 
 -- | Class of objects that have timestamps.
 class TimeStampObject a where
@@ -440,18 +442,19 @@ $(buildParam "Be" "bep"
   ])
 
 $(buildObject "Instance" "inst" $
-  [ simpleField "name"           [t| String             |]
-  , simpleField "primary_node"   [t| String             |]
-  , simpleField "os"             [t| String             |]
-  , simpleField "hypervisor"     [t| Hypervisor         |]
-  , simpleField "hvparams"       [t| HvParams           |]
-  , simpleField "beparams"       [t| PartialBeParams    |]
-  , simpleField "osparams"       [t| OsParams           |]
-  , simpleField "admin_state"    [t| AdminState         |]
-  , simpleField "nics"           [t| [PartialNic]       |]
-  , simpleField "disks"          [t| [Disk]             |]
-  , simpleField "disk_template"  [t| DiskTemplate       |]
-  , simpleField "disks_active"   [t| Bool               |]
+  [ simpleField "name"             [t| String             |]
+  , simpleField "primary_node"     [t| String             |]
+  , simpleField "os"               [t| String             |]
+  , simpleField "hypervisor"       [t| Hypervisor         |]
+  , simpleField "hvparams"         [t| HvParams           |]
+  , simpleField "beparams"         [t| PartialBeParams    |]
+  , simpleField "osparams"         [t| OsParams           |]
+  , simpleField "osparams_private" [t| OsParamsPrivate    |]
+  , simpleField "admin_state"      [t| AdminState         |]
+  , simpleField "nics"             [t| [PartialNic]       |]
+  , simpleField "disks"            [t| [Disk]             |]
+  , simpleField "disk_template"    [t| DiskTemplate       |]
+  , simpleField "disks_active"     [t| Bool               |]
   , optionalField $ simpleField "network_port" [t| Int  |]
   ]
   ++ timeStampFields
@@ -650,6 +653,7 @@ type ClusterBeParams = Container FilledBeParams
 
 -- | Cluster OsParams.
 type ClusterOsParams = Container OsParams
+type ClusterOsParamsPrivate = Container (Private OsParams)
 
 -- | Cluster NicParams.
 type ClusterNicParams = Container FilledNicParams
@@ -665,49 +669,50 @@ type CandidateCertificates = Container String
 
 -- * Cluster definitions
 $(buildObject "Cluster" "cluster" $
-  [ simpleField "rsahostkeypub"             [t| String           |]
+  [ simpleField "rsahostkeypub"             [t| String                 |]
   , optionalField $
-    simpleField "dsahostkeypub"             [t| String           |]
-  , simpleField "highest_used_port"         [t| Int              |]
-  , simpleField "tcpudp_port_pool"          [t| [Int]            |]
-  , simpleField "mac_prefix"                [t| String           |]
+    simpleField "dsahostkeypub"             [t| String                 |]
+  , simpleField "highest_used_port"         [t| Int                    |]
+  , simpleField "tcpudp_port_pool"          [t| [Int]                  |]
+  , simpleField "mac_prefix"                [t| String                 |]
   , optionalField $
-    simpleField "volume_group_name"         [t| String           |]
-  , simpleField "reserved_lvs"              [t| [String]         |]
+    simpleField "volume_group_name"         [t| String                 |]
+  , simpleField "reserved_lvs"              [t| [String]               |]
   , optionalField $
-    simpleField "drbd_usermode_helper"      [t| String           |]
-  , simpleField "master_node"               [t| String           |]
-  , simpleField "master_ip"                 [t| String           |]
-  , simpleField "master_netdev"             [t| String           |]
-  , simpleField "master_netmask"            [t| Int              |]
-  , simpleField "use_external_mip_script"   [t| Bool             |]
-  , simpleField "cluster_name"              [t| String           |]
-  , simpleField "file_storage_dir"          [t| String           |]
-  , simpleField "shared_file_storage_dir"   [t| String           |]
-  , simpleField "gluster_storage_dir"       [t| String           |]
-  , simpleField "enabled_hypervisors"       [t| [Hypervisor]     |]
-  , simpleField "hvparams"                  [t| ClusterHvParams  |]
-  , simpleField "os_hvp"                    [t| OsHvParams       |]
-  , simpleField "beparams"                  [t| ClusterBeParams  |]
-  , simpleField "osparams"                  [t| ClusterOsParams  |]
-  , simpleField "nicparams"                 [t| ClusterNicParams |]
-  , simpleField "ndparams"                  [t| FilledNDParams   |]
-  , simpleField "diskparams"                [t| DiskParams       |]
-  , simpleField "candidate_pool_size"       [t| Int              |]
-  , simpleField "modify_etc_hosts"          [t| Bool             |]
-  , simpleField "modify_ssh_setup"          [t| Bool             |]
-  , simpleField "maintain_node_health"      [t| Bool             |]
-  , simpleField "uid_pool"                  [t| UidPool          |]
-  , simpleField "default_iallocator"        [t| String           |]
-  , simpleField "default_iallocator_params" [t| IAllocatorParams |]
-  , simpleField "hidden_os"                 [t| [String]         |]
-  , simpleField "blacklisted_os"            [t| [String]         |]
-  , simpleField "primary_ip_family"         [t| IpFamily         |]
-  , simpleField "prealloc_wipe_disks"       [t| Bool             |]
-  , simpleField "ipolicy"                   [t| FilledIPolicy    |]
-  , simpleField "enabled_disk_templates"    [t| [DiskTemplate]   |]
-  , simpleField "candidate_certs"           [t| CandidateCertificates |]
-  , simpleField "max_running_jobs"          [t| Int              |]
+    simpleField "drbd_usermode_helper"      [t| String                 |]
+  , simpleField "master_node"               [t| String                 |]
+  , simpleField "master_ip"                 [t| String                 |]
+  , simpleField "master_netdev"             [t| String                 |]
+  , simpleField "master_netmask"            [t| Int                    |]
+  , simpleField "use_external_mip_script"   [t| Bool                   |]
+  , simpleField "cluster_name"              [t| String                 |]
+  , simpleField "file_storage_dir"          [t| String                 |]
+  , simpleField "shared_file_storage_dir"   [t| String                 |]
+  , simpleField "gluster_storage_dir"       [t| String                 |]
+  , simpleField "enabled_hypervisors"       [t| [Hypervisor]           |]
+  , simpleField "hvparams"                  [t| ClusterHvParams        |]
+  , simpleField "os_hvp"                    [t| OsHvParams             |]
+  , simpleField "beparams"                  [t| ClusterBeParams        |]
+  , simpleField "osparams"                  [t| ClusterOsParams        |]
+  , simpleField "osparams_private_cluster"  [t| ClusterOsParamsPrivate |]
+  , simpleField "nicparams"                 [t| ClusterNicParams       |]
+  , simpleField "ndparams"                  [t| FilledNDParams         |]
+  , simpleField "diskparams"                [t| DiskParams             |]
+  , simpleField "candidate_pool_size"       [t| Int                    |]
+  , simpleField "modify_etc_hosts"          [t| Bool                   |]
+  , simpleField "modify_ssh_setup"          [t| Bool                   |]
+  , simpleField "maintain_node_health"      [t| Bool                   |]
+  , simpleField "uid_pool"                  [t| UidPool                |]
+  , simpleField "default_iallocator"        [t| String                 |]
+  , simpleField "default_iallocator_params" [t| IAllocatorParams       |]
+  , simpleField "hidden_os"                 [t| [String]               |]
+  , simpleField "blacklisted_os"            [t| [String]               |]
+  , simpleField "primary_ip_family"         [t| IpFamily               |]
+  , simpleField "prealloc_wipe_disks"       [t| Bool                   |]
+  , simpleField "ipolicy"                   [t| FilledIPolicy          |]
+  , simpleField "enabled_disk_templates"    [t| [DiskTemplate]         |]
+  , simpleField "candidate_certs"           [t| CandidateCertificates  |]
+  , simpleField "max_running_jobs"          [t| Int                    |]
  ]
  ++ timeStampFields
  ++ uuidFields

test/data/instance-prim-sec.txt

@@ -72,6 +72,7 @@
    ],
    "os": "busybox",
    "osparams": {},
+   "osparams_private": {},
    "primary_node": "60e687a0-21fc-4577-997f-ccd08925fa65",
    "serial_no": 2,
    "uuid": "aec390cb-5eae-44e6-bcc2-ec14d31347f0"

test/hs/Test/Ganeti/Objects.hs

@@ -123,6 +123,8 @@ instance Arbitrary Instance where
       <*> arbitrary
       -- osparams
       <*> pure (GenericContainer Map.empty)
+      -- osparams_private
+      <*> pure (GenericContainer Map.empty)
       -- admin_state
       <*> arbitrary
       -- nics

test/hs/Test/Ganeti/Query/Instance.hs

@@ -52,7 +52,7 @@ createInstance name pnodeUuid adminState =
   Instance name pnodeUuid "" Kvm
     (GenericContainer Map.empty)
     (PartialBeParams Nothing Nothing Nothing Nothing Nothing Nothing)
-    (GenericContainer Map.empty)
+    (GenericContainer Map.empty) (GenericContainer Map.empty)
     adminState [] [] DTDrbd8 False Nothing epochTime epochTime "" 0 Set.empty
   where epochTime = TOD 0 0
 

test/py/cmdlib/testsupport/config_mock.py

@@ -181,6 +181,7 @@ class ConfigMock(config.ConfigWriter):
                      hvparams=None,
                      beparams=None,
                      osparams=None,
+                     osparams_private=None,
                      admin_state=None,
                      nics=None,
                      disks=None,
@@ -217,6 +218,8 @@ class ConfigMock(config.ConfigWriter):
       beparams = {}
     if osparams is None:
       osparams = {}
+    if osparams_private is None:
+      osparams_private = {}
     if admin_state is None:
       admin_state = constants.ADMINST_DOWN
     if nics is None:
@@ -247,6 +250,7 @@ class ConfigMock(config.ConfigWriter):
                             hvparams=hvparams,
                             beparams=beparams,
                             osparams=osparams,
+                            osparams_private=osparams_private,
                             admin_state=admin_state,
                             nics=nics,
                             disks=disks,
@@ -571,6 +575,7 @@ class ConfigMock(config.ConfigWriter):
       os_hvp={self.GetDefaultOs().name: constants.HVC_DEFAULTS.copy()},
       beparams=None,
       osparams=None,
+      osparams_private_cluster=None,
       nicparams={constants.PP_DEFAULT: constants.NICC_DEFAULTS},
       ndparams=None,
       diskparams=None,

test/py/ganeti.config_unittest.py

@@ -35,6 +35,7 @@ from ganeti import objects
 from ganeti import utils
 from ganeti import netutils
 from ganeti import compat
+from ganeti import serializer
 from ganeti.cmdlib import instance
 
 from ganeti.config import TemporaryReservationManager
@@ -109,7 +110,8 @@ class TestConfigRunner(unittest.TestCase):
                             uuid="test-uuid",
                             disks=[], nics=[],
                             disk_template=constants.DT_DISKLESS,
-                            primary_node=self._get_object().GetMasterNode())
+                            primary_node=self._get_object().GetMasterNode(),
+                            osparams_private=serializer.PrivateDict())
     return inst
 
   def testEmpty(self):

test/py/ganeti.ovf_unittest.py

@@ -173,6 +173,7 @@ ARGS_VBOX = dict(ARGS_EXPORT_DIR, **{
   "os": "lenny-image",
   "hypervisor": ("xen-pvm", {}),
   "osparams":{},
+  "osparams_private":{},
   "disks": [],
 })
 ARGS_COMPLETE = dict(ARGS_VBOX, **{
@@ -188,6 +189,7 @@ ARGS_BROKEN = dict(ARGS_EXPORT_DIR , **{
   "name": "test-instance",
   "os": "lenny-image",
   "osparams": {},
+  "osparams_private":{},
 })
 
 EXP_ARGS_COMPRESSED = dict(ARGS_EXPORT_DIR, **{