From 8b05721832e29eb47f40a393527e1eab7c1e7e21 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A9=20Nussbaumer?= <rn@google.com>
Date: Thu, 19 Jul 2012 10:26:13 +0200
Subject: [PATCH] Fix setting ipolicy on node groups
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 lib/cmdlib.py  |  4 ++--
 lib/config.py  |  9 +++++----
 lib/objects.py | 19 ++++++++++++++-----
 3 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index d937d48b7..92a6cd6a3 100644
--- 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 33d1b1e7f..d29a6121e 100644
--- 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 40c76ae72..74526d338 100644
--- 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)
 
-- 
GitLab