From 2cc673a3ece0f388ce59ea5bf40d1b98a25d77fb Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Thu, 29 Dec 2011 16:33:32 +0100
Subject: [PATCH] Add new disk_templates parameter to instance policy
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This is a bit more complex patch, as it requires changing the
assumption that all keys in the policy dict points to values that are
themselves dicts. Right now we introduce an assumption that any
non-dicts are lists, we'll see in the future if this holds or whether
we need more complex type checking (manual, yay Python).

The patch also does some trivial style changes.

Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: RenΓ© Nussbaumer <rn@google.com>
---
 lib/bootstrap.py          |  3 +-
 lib/client/gnt_cluster.py | 13 ++++++--
 lib/client/gnt_group.py   |  1 +
 lib/cmdlib.py             | 64 +++++++++++++++++++++++++--------------
 lib/config.py             | 13 ++++++--
 lib/constants.py          |  5 ++-
 lib/objects.py            | 58 ++++++++++++++++++++++++++---------
 7 files changed, 110 insertions(+), 47 deletions(-)

diff --git a/lib/bootstrap.py b/lib/bootstrap.py
index 3e42eaee4..66c166c9f 100644
--- a/lib/bootstrap.py
+++ b/lib/bootstrap.py
@@ -421,8 +421,7 @@ def InitCluster(cluster_name, mac_prefix, # pylint: disable=R0913, R0914
     utils.ForceDictType(val, constants.ISPECS_PARAMETER_TYPES)
 
   objects.NIC.CheckParameterSyntax(nicparams)
-  full_ipolicy = objects.FillDictOfDicts(constants.IPOLICY_DEFAULTS,
-                                         ipolicy)
+  full_ipolicy = objects.FillIPolicy(constants.IPOLICY_DEFAULTS, ipolicy)
   objects.InstancePolicy.CheckParameterSyntax(full_ipolicy)
 
   if ndparams is not None:
diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
index d1ecbd538..6cbd4da86 100644
--- a/lib/client/gnt_cluster.py
+++ b/lib/client/gnt_cluster.py
@@ -139,13 +139,16 @@ def InitCluster(opts, args):
     utils.ForceDictType(diskparams[templ], constants.DISK_DT_TYPES)
 
   # prepare ipolicy dict
+  ispecs_dts = opts.ispecs_disk_templates # hate long var names
   ipolicy_raw = \
     objects.CreateIPolicyFromOpts(ispecs_mem_size=opts.ispecs_mem_size,
                                   ispecs_cpu_count=opts.ispecs_cpu_count,
                                   ispecs_disk_count=opts.ispecs_disk_count,
                                   ispecs_disk_size=opts.ispecs_disk_size,
-                                  ispecs_nic_count=opts.ispecs_nic_count)
-  ipolicy = objects.FillDictOfDicts(constants.IPOLICY_DEFAULTS, ipolicy_raw)
+                                  ispecs_nic_count=opts.ispecs_nic_count,
+                                  ispecs_disk_templates=ispecs_dts,
+                                  fill_all=True)
+  ipolicy = objects.FillIPolicy(constants.IPOLICY_DEFAULTS, ipolicy_raw)
   for value in ipolicy.values():
     utils.ForceDictType(value, constants.ISPECS_PARAMETER_TYPES)
 
@@ -461,6 +464,8 @@ def ShowClusterConfig(opts, args):
   for key in constants.IPOLICY_PARAMETERS:
     ToStdout("  - %s", key)
     _PrintGroupedParams(result["ipolicy"][key], roman=opts.roman_integers)
+  ToStdout("  - enabled disk templates: %s",
+           utils.CommaJoin(result["ipolicy"][constants.ISPECS_DTS]))
 
   return 0
 
@@ -984,12 +989,14 @@ def SetClusterParams(opts, args):
   if ndparams is not None:
     utils.ForceDictType(ndparams, constants.NDS_PARAMETER_TYPES)
 
+  ispecs_dts = opts.ispecs_disk_templates
   ipolicy = \
     objects.CreateIPolicyFromOpts(ispecs_mem_size=opts.ispecs_mem_size,
                                   ispecs_cpu_count=opts.ispecs_cpu_count,
                                   ispecs_disk_count=opts.ispecs_disk_count,
                                   ispecs_disk_size=opts.ispecs_disk_size,
-                                  ispecs_nic_count=opts.ispecs_nic_count)
+                                  ispecs_nic_count=opts.ispecs_nic_count,
+                                  ispecs_disk_templates=ispecs_dts)
 
   mnh = opts.maintain_node_health
 
diff --git a/lib/client/gnt_group.py b/lib/client/gnt_group.py
index f39c6e5ef..b8d823646 100644
--- a/lib/client/gnt_group.py
+++ b/lib/client/gnt_group.py
@@ -192,6 +192,7 @@ def SetGroupParams(opts, args):
     ispecs_disk_count=opts.ispecs_disk_count,
     ispecs_disk_size=opts.ispecs_disk_size,
     ispecs_nic_count=opts.ispecs_nic_count,
+    ispecs_disk_templates=opts.ispecs_disk_templates,
     group_ipolicy=True,
     allowed_values=[constants.VALUE_DEFAULT])
 
diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index b66bfbabe..1b612f733 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -721,6 +721,42 @@ def _GetUpdatedParams(old_params, update_dict,
   return params_copy
 
 
+def _GetUpdatedIPolicy(old_ipolicy, new_ipolicy, group_policy=False):
+  """Return the new version of a instance policy.
+
+  @param group_policy: whether this policy applies to a group and thus
+    we should support removal of policy entries
+
+  """
+  use_none = use_default = group_policy
+  ipolicy = copy.deepcopy(old_ipolicy)
+  for key, value in new_ipolicy.items():
+    if key in constants.IPOLICY_PARAMETERS:
+      utils.ForceDictType(value, constants.ISPECS_PARAMETER_TYPES)
+      ipolicy[key] = _GetUpdatedParams(old_ipolicy.get(key, {}), value,
+                                       use_none=use_none,
+                                       use_default=use_default)
+    else:
+      # FIXME: we assume all others are lists; this should be redone
+      # in a nicer way
+      if not value or value == [constants.VALUE_DEFAULT]:
+        if group_policy:
+          del ipolicy[key]
+        else:
+          raise errors.OpPrereqError("Can't unset ipolicy attribute '%s'"
+                                     " on the cluster'" % key,
+                                     errors.ECODE_INVAL)
+      else:
+        logging.info("Setting %s to %s", key, value)
+        ipolicy[key] = list(value)
+  try:
+    objects.InstancePolicy.CheckParameterSyntax(ipolicy)
+  except errors.ConfigurationError, err:
+    raise errors.OpPrereqError("Invalid instance policy: %s" % err,
+                               errors.ECODE_INVAL)
+  return ipolicy
+
+
 def _UpdateAndVerifySubDict(base, updates, type_check):
   """Updates and verifies a dict with sub dicts of the same type.
 
@@ -3830,17 +3866,8 @@ class LUClusterSetParams(LogicalUnit):
              for storage, svalues in new_disk_state.items())
 
     if self.op.ipolicy:
-      ipolicy = {}
-      for key, value in self.op.ipolicy.items():
-        utils.ForceDictType(value, constants.ISPECS_PARAMETER_TYPES)
-        ipolicy[key] = _GetUpdatedParams(cluster.ipolicy.get(key, {}),
-                                          value)
-      try:
-        objects.InstancePolicy.CheckParameterSyntax(ipolicy)
-      except errors.ConfigurationError, err:
-        raise errors.OpPrereqError("Invalid instance policy: %s" % err,
-                                   errors.ECODE_INVAL)
-      self.new_ipolicy = ipolicy
+      self.new_ipolicy = _GetUpdatedIPolicy(cluster.ipolicy, self.op.ipolicy,
+                                            group_policy=False)
 
     if self.op.nicparams:
       utils.ForceDictType(self.op.nicparams, constants.NICS_PARAMETER_TYPES)
@@ -13306,18 +13333,9 @@ class LUGroupSetParams(LogicalUnit):
                                  self.group.disk_state_static)
 
     if self.op.ipolicy:
-      g_ipolicy = {}
-      for key, value in self.op.ipolicy.iteritems():
-        g_ipolicy[key] = _GetUpdatedParams(self.group.ipolicy.get(key, {}),
-                                           value,
-                                           use_none=True)
-        utils.ForceDictType(g_ipolicy[key], constants.ISPECS_PARAMETER_TYPES)
-      self.new_ipolicy = g_ipolicy
-      try:
-        objects.InstancePolicy.CheckParameterSyntax(self.new_ipolicy)
-      except errors.ConfigurationError, err:
-        raise errors.OpPrereqError("Invalid instance policy: %s" % err,
-                                   errors.ECODE_INVAL)
+      self.new_ipolicy = _GetUpdatedIPolicy(self.group.ipolicy,
+                                            self.op.ipolicy,
+                                            group_policy=True)
 
   def BuildHooksEnv(self):
     """Build hooks env.
diff --git a/lib/config.py b/lib/config.py
index 03a3d5aeb..7654ba1a7 100644
--- a/lib/config.py
+++ b/lib/config.py
@@ -433,9 +433,16 @@ class ConfigWriter:
         result.append("%s has invalid instance policy: %s" % (owner, err))
 
     def _helper_ispecs(owner, params):
-      for key, value in params.iteritems():
-        fullkey = "ipolicy/" + key
-        _helper(owner, fullkey, value, constants.ISPECS_PARAMETER_TYPES)
+      for key, value in params.items():
+        if key in constants.IPOLICY_PARAMETERS:
+          fullkey = "ipolicy/" + key
+          _helper(owner, fullkey, value, constants.ISPECS_PARAMETER_TYPES)
+        else:
+          # FIXME: assuming list type
+          if not isinstance(value, list):
+            result.append("%s has invalid instance policy: for %s,"
+                          " expecting list, got %s" %
+                          (owner, key, type(value)))
 
     # check cluster parameters
     _helper("cluster", "beparams", cluster.SimpleFillBE({}),
diff --git a/lib/constants.py b/lib/constants.py
index 16931ac6c..9216e0ab9 100644
--- a/lib/constants.py
+++ b/lib/constants.py
@@ -941,12 +941,14 @@ ISPECS_PARAMETERS = frozenset(ISPECS_PARAMETER_TYPES.keys())
 ISPECS_MIN = "min"
 ISPECS_MAX = "max"
 ISPECS_STD = "std"
+ISPECS_DTS = "disk_templates"
 
 IPOLICY_PARAMETERS = frozenset([
   ISPECS_MIN,
   ISPECS_MAX,
   ISPECS_STD,
   ])
+IPOLICY_ALL_KEYS = IPOLICY_PARAMETERS.union([ISPECS_DTS])
 
 # Node parameter names
 ND_OOB_PROGRAM = "oob_program"
@@ -1885,7 +1887,8 @@ IPOLICY_DEFAULTS = {
     ISPEC_DISK_COUNT: 1,
     ISPEC_DISK_SIZE: 1024,
     ISPEC_NIC_COUNT: 1,
-    }
+    },
+  ISPECS_DTS: DISK_TEMPLATES,
   }
 
 MASTER_POOL_SIZE_DEFAULT = 10
diff --git a/lib/objects.py b/lib/objects.py
index 47fe527df..3da52edd8 100644
--- a/lib/objects.py
+++ b/lib/objects.py
@@ -1,7 +1,7 @@
 #
 #
 
-# Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011 Google Inc.
+# Copyright (C) 2006, 2007, 2008, 2009, 2010, 2011, 2012 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
@@ -59,7 +59,7 @@ _UUID = ["uuid"]
 TISPECS_GROUP_TYPES = {
   constants.ISPECS_MIN: constants.VTYPE_INT,
   constants.ISPECS_MAX: constants.VTYPE_INT,
-}
+  }
 
 TISPECS_CLUSTER_TYPES = {
   constants.ISPECS_MIN: constants.VTYPE_INT,
@@ -92,15 +92,20 @@ def FillDict(defaults_dict, custom_dict, skip_keys=None):
   return ret_dict
 
 
-def FillDictOfDicts(defaults_dict, custom_dict, skip_keys=None):
-  """Run FillDict for each key in dictionary.
+def FillIPolicy(default_ipolicy, custom_ipolicy, skip_keys=None):
+  """Fills an instance policy with defaults.
 
   """
+  assert frozenset(default_ipolicy.keys()) == constants.IPOLICY_ALL_KEYS
   ret_dict = {}
-  for key in defaults_dict:
-    ret_dict[key] = FillDict(defaults_dict[key],
-                             custom_dict.get(key, {}),
+  for key in constants.IPOLICY_PARAMETERS:
+    ret_dict[key] = FillDict(default_ipolicy[key],
+                             custom_ipolicy.get(key, {}),
                              skip_keys=skip_keys)
+  # list items
+  for key in [constants.ISPECS_DTS]:
+    ret_dict[key] = list(custom_ipolicy.get(key, default_ipolicy[key]))
+
   return ret_dict
 
 
@@ -166,9 +171,9 @@ def MakeEmptyIPolicy():
 
   """
   return dict([
-    (constants.ISPECS_MIN, dict()),
-    (constants.ISPECS_MAX, dict()),
-    (constants.ISPECS_STD, dict()),
+    (constants.ISPECS_MIN, {}),
+    (constants.ISPECS_MAX, {}),
+    (constants.ISPECS_STD, {}),
     ])
 
 
@@ -177,9 +182,14 @@ def CreateIPolicyFromOpts(ispecs_mem_size=None,
                           ispecs_disk_count=None,
                           ispecs_disk_size=None,
                           ispecs_nic_count=None,
+                          ispecs_disk_templates=None,
                           group_ipolicy=False,
-                          allowed_values=None):
-  """Creation of instane policy based on command line options.
+                          allowed_values=None,
+                          fill_all=False):
+  """Creation of instance policy based on command line options.
+
+  @param fill_all: whether for cluster policies we should ensure that
+    all values are filled
 
 
   """
@@ -208,6 +218,12 @@ def CreateIPolicyFromOpts(ispecs_mem_size=None,
     for key, val in specs.items(): # {min: .. ,max: .., std: ..}
       ipolicy_out[key][name] = val
 
+  # no filldict for lists
+  if not group_ipolicy and fill_all and ispecs_disk_templates is None:
+    ispecs_disk_templates = constants.DISK_TEMPLATES
+  if ispecs_disk_templates is not None:
+    ipolicy_out[constants.ISPECS_DTS] = list(ispecs_disk_templates)
+
   return ipolicy_out
 
 
@@ -857,7 +873,7 @@ class Disk(ConfigObject):
 
 class InstancePolicy(ConfigObject):
   """Config object representing instance policy limits dictionary."""
-  __slots__ = ["min", "max", "std"]
+  __slots__ = ["min", "max", "std", "disk_templates"]
 
   @classmethod
   def CheckParameterSyntax(cls, ipolicy):
@@ -866,6 +882,8 @@ class InstancePolicy(ConfigObject):
     """
     for param in constants.ISPECS_PARAMETERS:
       InstancePolicy.CheckISpecSyntax(ipolicy, param)
+    if constants.ISPECS_DTS in ipolicy:
+      InstancePolicy.CheckDiskTemplates(ipolicy[constants.ISPECS_DTS])
 
   @classmethod
   def CheckISpecSyntax(cls, ipolicy, name):
@@ -892,6 +910,16 @@ class InstancePolicy(ConfigObject):
     if min_v > std_v or std_v > max_v:
       raise errors.ConfigurationError(err)
 
+  @classmethod
+  def CheckDiskTemplates(cls, disk_templates):
+    """Checks the disk templates for validity.
+
+    """
+    wrong = frozenset(disk_templates).difference(constants.DISK_TEMPLATES)
+    if wrong:
+      raise errors.ConfigurationError("Invalid disk template(s) %s" %
+                                      utils.CommaJoin(wrong))
+
 
 class Instance(TaggableObject):
   """Config object representing an instance."""
@@ -1473,7 +1501,7 @@ class Cluster(TaggableObject):
 
     # instance policy added before 2.6
     if self.ipolicy is None:
-      self.ipolicy = FillDictOfDicts(constants.IPOLICY_DEFAULTS, {})
+      self.ipolicy = FillIPolicy(constants.IPOLICY_DEFAULTS, {})
 
   @property
   def primary_hypervisor(self):
@@ -1668,7 +1696,7 @@ class Cluster(TaggableObject):
       the cluster defaults
 
     """
-    return FillDictOfDicts(self.ipolicy, ipolicy)
+    return FillIPolicy(self.ipolicy, ipolicy)
 
 
 class BlockDevStatus(ConfigObject):
-- 
GitLab