Fix setting ipolicy on node groups

On node groups we don't have the std field. However, the InstancePolicy
object always verifies that the std value is within a given range. As we
fill it up with defaults if not set (as it happens to be on node groups)
and the min value is higher than the default std value (taken from
constants.py) we fail.

We overcome this situation by simply let the function know if we want to
verify the std value at all. If we don't want to verify std, we just set
it to a compliant value (min_v) and continue.

We also slightly adapt the error message provided, as we don't have std
values on groups.

Signed-off-by: René Nussbaumer <rn@google.com>
Reviewed-by: Iustin Pop <iustin@google.com>

diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index d937d48..92a6cd6 100644 (file)
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -815,7 +815,7 @@ def _GetUpdatedIPolicy(old_ipolicy, new_ipolicy, group_policy=False):
           # in a nicer way
           ipolicy[key] = list(value)
   try:
-    objects.InstancePolicy.CheckParameterSyntax(ipolicy)
+    objects.InstancePolicy.CheckParameterSyntax(ipolicy, not group_policy)
   except errors.ConfigurationError, err:
     raise errors.OpPrereqError("Invalid instance policy: %s" % err,
                                errors.ECODE_INVAL)
@@ -13627,7 +13627,7 @@ class LUGroupAdd(LogicalUnit):
       cluster = self.cfg.GetClusterInfo()
       full_ipolicy = cluster.SimpleFillIPolicy(self.op.ipolicy)
       try:
-        objects.InstancePolicy.CheckParameterSyntax(full_ipolicy)
+        objects.InstancePolicy.CheckParameterSyntax(full_ipolicy, False)
       except errors.ConfigurationError, err:
         raise errors.OpPrereqError("Invalid instance policy: %s" % err,
                                    errors.ECODE_INVAL)

diff --git a/lib/config.py b/lib/config.py
index 33d1b1e..d29a612 100644 (file)
--- a/lib/config.py
+++ b/lib/config.py
@@ -480,9 +480,9 @@ class ConfigWriter:
       except errors.ConfigurationError, err:
         result.append("%s has invalid nicparams: %s" % (owner, err))
 
-    def _helper_ipolicy(owner, params):
+    def _helper_ipolicy(owner, params, check_std):
       try:
-        objects.InstancePolicy.CheckParameterSyntax(params)
+        objects.InstancePolicy.CheckParameterSyntax(params, check_std)
       except errors.ConfigurationError, err:
         result.append("%s has invalid instance policy: %s" % (owner, err))
 
@@ -510,7 +510,7 @@ class ConfigWriter:
     _helper_nic("cluster", cluster.SimpleFillNIC({}))
     _helper("cluster", "ndparams", cluster.SimpleFillND({}),
             constants.NDS_PARAMETER_TYPES)
-    _helper_ipolicy("cluster", cluster.SimpleFillIPolicy({}))
+    _helper_ipolicy("cluster", cluster.SimpleFillIPolicy({}), True)
     _helper_ispecs("cluster", cluster.SimpleFillIPolicy({}))
 
     # per-instance checks
@@ -636,7 +636,8 @@ class ConfigWriter:
       else:
         nodegroups_names.add(nodegroup.name)
       group_name = "group %s" % nodegroup.name
-      _helper_ipolicy(group_name, cluster.SimpleFillIPolicy(nodegroup.ipolicy))
+      _helper_ipolicy(group_name, cluster.SimpleFillIPolicy(nodegroup.ipolicy),
+                      False)
       _helper_ispecs(group_name, cluster.SimpleFillIPolicy(nodegroup.ipolicy))
       if nodegroup.ndparams:
         _helper(group_name, "ndparams",

diff --git a/lib/objects.py b/lib/objects.py
index 40c76ae..74526d3 100644 (file)
--- a/lib/objects.py
+++ b/lib/objects.py
@@ -946,12 +946,12 @@ class InstancePolicy(ConfigObject):
 
   """
   @classmethod
-  def CheckParameterSyntax(cls, ipolicy):
+  def CheckParameterSyntax(cls, ipolicy, check_std):
     """ Check the instance policy for validity.
 
     """
     for param in constants.ISPECS_PARAMETERS:
-      InstancePolicy.CheckISpecSyntax(ipolicy, param)
+      InstancePolicy.CheckISpecSyntax(ipolicy, param, check_std)
     if constants.IPOLICY_DTS in ipolicy:
       InstancePolicy.CheckDiskTemplates(ipolicy[constants.IPOLICY_DTS])
     for key in constants.IPOLICY_PARAMETERS:
@@ -963,7 +963,7 @@ class InstancePolicy(ConfigObject):
                                       utils.CommaJoin(wrong_keys))
 
   @classmethod
-  def CheckISpecSyntax(cls, ipolicy, name):
+  def CheckISpecSyntax(cls, ipolicy, name, check_std):
     """Check the instance policy for validity on a given key.
 
     We check if the instance policy makes sense for a given key, that is
@@ -973,17 +973,26 @@ class InstancePolicy(ConfigObject):
     @param ipolicy: dictionary with min, max, std specs
     @type name: string
     @param name: what are the limits for
+    @type check_std: bool
+    @param check_std: Whether to check std value or just assume compliance
     @raise errors.ConfigureError: when specs for given name are not valid
 
     """
     min_v = ipolicy[constants.ISPECS_MIN].get(name, 0)
-    std_v = ipolicy[constants.ISPECS_STD].get(name, min_v)
+
+    if check_std:
+      std_v = ipolicy[constants.ISPECS_STD].get(name, min_v)
+      std_msg = std_v
+    else:
+      std_v = min_v
+      std_msg = "-"
+
     max_v = ipolicy[constants.ISPECS_MAX].get(name, std_v)
     err = ("Invalid specification of min/max/std values for %s: %s/%s/%s" %
            (name,
             ipolicy[constants.ISPECS_MIN].get(name, "-"),
             ipolicy[constants.ISPECS_MAX].get(name, "-"),
-            ipolicy[constants.ISPECS_STD].get(name, "-")))
+            std_msg))
     if min_v > std_v or std_v > max_v:
       raise errors.ConfigurationError(err)