Refactor ispecs in ipolicy structures
authorBernardo Dal Seno <bdalseno@google.com>
Tue, 19 Feb 2013 21:14:13 +0000 (22:14 +0100)
committerBernardo Dal Seno <bdalseno@google.com>
Wed, 27 Mar 2013 10:46:22 +0000 (11:46 +0100)
Minimum and maximum instance specs are put together into a single element
of the instance policy. This is in preparation for introducing multiple
min/max specs.

Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Helga Velroyen <helgav@google.com>

25 files changed:
doc/rapi.rst
lib/cli.py
lib/client/gnt_cluster.py
lib/cmdlib.py
lib/config.py
lib/constants.py
lib/objects.py
qa/qa_cluster.py
src/Ganeti/HTools/Backend/Text.hs
src/Ganeti/HTools/Instance.hs
src/Ganeti/HTools/Program/Hspace.hs
src/Ganeti/HTools/Types.hs
src/Ganeti/Objects.hs
test/data/htools/hail-alloc-drbd.json
test/data/htools/hail-change-group.json
test/data/htools/hail-node-evac.json
test/data/htools/hail-reloc-drbd.json
test/data/htools/rapi/groups.json
test/data/htools/rapi/info.json
test/hs/Test/Ganeti/HTools/Types.hs
test/hs/Test/Ganeti/Objects.hs
test/hs/Test/Ganeti/TestHTools.hs
test/py/ganeti.cli_unittest.py
test/py/ganeti.cmdlib_unittest.py
test/py/ganeti.objects_unittest.py

index 4a7d708..10cd80e 100644 (file)
@@ -226,8 +226,7 @@ The instance policy specification is a dict with the following fields:
 
 .. pyassert::
 
-  constants.IPOLICY_ALL_KEYS == set([constants.ISPECS_MIN,
-                                     constants.ISPECS_MAX,
+  constants.IPOLICY_ALL_KEYS == set([constants.ISPECS_MINMAX,
                                      constants.ISPECS_STD,
                                      constants.IPOLICY_DTS,
                                      constants.IPOLICY_VCPU_RATIO,
@@ -249,24 +248,30 @@ The instance policy specification is a dict with the following fields:
 .. |ispec-std| replace:: :pyeval:`constants.ISPECS_STD`
 
 
-|ispec-min|, |ispec-max|, |ispec-std|
-  A sub- `dict` with the following fields, which sets the limit and standard
-  values of the instances:
-
-  :pyeval:`constants.ISPEC_MEM_SIZE`
-    The size in MiB of the memory used
-  :pyeval:`constants.ISPEC_DISK_SIZE`
-    The size in MiB of the disk used
-  :pyeval:`constants.ISPEC_DISK_COUNT`
-    The numbers of disks used
-  :pyeval:`constants.ISPEC_CPU_COUNT`
-    The numbers of cpus used
-  :pyeval:`constants.ISPEC_NIC_COUNT`
-    The numbers of nics used
-  :pyeval:`constants.ISPEC_SPINDLE_USE`
-    The numbers of virtual disk spindles used by this instance. They are
-    not real in the sense of actual HDD spindles, but useful for
-    accounting the spindle usage on the residing node
+:pyeval:`constants.ISPECS_MINMAX`
+  A dict with the following two fields:
+
+  |ispec-min|, |ispec-max|
+    A sub- `dict` with the following fields, which sets the limit of the
+    instances:
+
+    :pyeval:`constants.ISPEC_MEM_SIZE`
+      The size in MiB of the memory used
+    :pyeval:`constants.ISPEC_DISK_SIZE`
+      The size in MiB of the disk used
+    :pyeval:`constants.ISPEC_DISK_COUNT`
+      The numbers of disks used
+    :pyeval:`constants.ISPEC_CPU_COUNT`
+      The numbers of cpus used
+    :pyeval:`constants.ISPEC_NIC_COUNT`
+      The numbers of nics used
+    :pyeval:`constants.ISPEC_SPINDLE_USE`
+      The numbers of virtual disk spindles used by this instance. They
+      are not real in the sense of actual HDD spindles, but useful for
+      accounting the spindle usage on the residing node
+|ispec-std|
+  A sub- `dict` with the same fields as |ispec-min| and |ispec-max| above,
+  which sets the standard values of the instances.
 :pyeval:`constants.IPOLICY_DTS`
   A `list` of disk templates allowed for instances using this policy
 :pyeval:`constants.IPOLICY_VCPU_RATIO`
index 1fa0eda..7857e18 100644 (file)
@@ -3717,10 +3717,19 @@ def _InitIspecsFromOpts(ipolicy, ispecs_mem_size, ispecs_cpu_count,
     utils.ForceDictType(specs, forced_type, allowed_values=allowed_values)
 
   # then transpose
+  ispecs = {
+    constants.ISPECS_MIN: {},
+    constants.ISPECS_MAX: {},
+    constants.ISPECS_STD: {},
+    }
   for (name, specs) in ispecs_transposed.iteritems():
     assert name in constants.ISPECS_PARAMETERS
     for key, val in specs.items(): # {min: .. ,max: .., std: ..}
-      ipolicy[key][name] = val
+      assert key in ispecs
+      ispecs[key][name] = val
+  for key in constants.ISPECS_MINMAX_KEYS:
+    ipolicy[constants.ISPECS_MINMAX][key] = ispecs[key]
+  ipolicy[constants.ISPECS_STD] = ispecs[constants.ISPECS_STD]
 
 
 def CreateIPolicyFromOpts(ispecs_mem_size=None,
index 1ce13b4..bbee602 100644 (file)
@@ -476,10 +476,14 @@ def ShowClusterConfig(opts, args):
     ("Instance policy - limits for instances",
      [
        (key,
-        _FormatGroupedParams(result["ipolicy"][key], roman=opts.roman_integers))
-       for key in constants.IPOLICY_ISPECS
+        _FormatGroupedParams(result["ipolicy"][constants.ISPECS_MINMAX][key],
+                             roman=opts.roman_integers))
+       for key in constants.ISPECS_MINMAX_KEYS
        ] +
      [
+       (constants.ISPECS_STD,
+        _FormatGroupedParams(result["ipolicy"][constants.ISPECS_STD],
+                             roman=opts.roman_integers)),
        ("enabled disk templates",
         utils.CommaJoin(result["ipolicy"][constants.IPOLICY_DTS])),
        ] +
index 1c0ad21..55d313c 100644 (file)
@@ -813,8 +813,22 @@ def _GetUpdatedParams(old_params, update_dict,
   return params_copy
 
 
+def _UpdateMinMaxISpecs(ipolicy, new_minmax, group_policy):
+  use_none = use_default = group_policy
+  minmax = ipolicy.setdefault(constants.ISPECS_MINMAX, {})
+  for (key, value) in new_minmax.items():
+    if key not in constants.ISPECS_MINMAX_KEYS:
+      raise errors.OpPrereqError("Invalid key in new ipolicy/%s: %s" %
+                                 (constants.ISPECS_MINMAX, key),
+                                 errors.ECODE_INVAL)
+    old_spec = minmax.get(key, {})
+    minmax[key] = _GetUpdatedParams(old_spec, value, use_none=use_none,
+                                    use_default=use_default)
+    utils.ForceDictType(minmax[key], constants.ISPECS_PARAMETER_TYPES)
+
+
 def _GetUpdatedIPolicy(old_ipolicy, new_ipolicy, group_policy=False):
-  """Return the new version of a instance policy.
+  """Return the new version of an instance policy.
 
   @param group_policy: whether this policy applies to a group and thus
     we should support removal of policy entries
@@ -826,7 +840,9 @@ def _GetUpdatedIPolicy(old_ipolicy, new_ipolicy, group_policy=False):
     if key not in constants.IPOLICY_ALL_KEYS:
       raise errors.OpPrereqError("Invalid key in new ipolicy: %s" % key,
                                  errors.ECODE_INVAL)
-    if key in constants.IPOLICY_ISPECS:
+    if key == constants.ISPECS_MINMAX:
+      _UpdateMinMaxISpecs(ipolicy, value, group_policy)
+    elif key == constants.ISPECS_STD:
       ipolicy[key] = _GetUpdatedParams(old_ipolicy.get(key, {}), value,
                                        use_none=use_none,
                                        use_default=use_default)
@@ -1203,22 +1219,21 @@ def _CheckInstanceState(lu, instance, req_states, msg=None):
                      " is down")
 
 
-def _ComputeMinMaxSpec(name, qualifier, ipolicy, value):
+def _ComputeMinMaxSpec(name, qualifier, ispecs, value):
   """Computes if value is in the desired range.
 
   @param name: name of the parameter for which we perform the check
   @param qualifier: a qualifier used in the error message (e.g. 'disk/1',
       not just 'disk')
-  @param ipolicy: dictionary containing min, max and std values
+  @param ispecs: dictionary containing min and max values
   @param value: actual value that we want to use
-  @return: None or element not meeting the criteria
-
+  @return: None or an error string
 
   """
   if value in [None, constants.VALUE_AUTO]:
     return None
-  max_v = ipolicy[constants.ISPECS_MAX].get(name, value)
-  min_v = ipolicy[constants.ISPECS_MIN].get(name, value)
+  max_v = ispecs[constants.ISPECS_MAX].get(name, value)
+  min_v = ispecs[constants.ISPECS_MIN].get(name, value)
   if value > max_v or min_v > value:
     if qualifier:
       fqn = "%s/%s" % (name, qualifier)
@@ -1273,8 +1288,9 @@ def _ComputeIPolicySpecViolation(ipolicy, mem_size, cpu_count, disk_count,
     ret.append("Disk template %s is not allowed (allowed templates: %s)" %
                (disk_template, utils.CommaJoin(allowed_dts)))
 
+  minmax = ipolicy[constants.ISPECS_MINMAX]
   return ret + filter(None,
-                      (_compute_fn(name, qualifier, ipolicy, value)
+                      (_compute_fn(name, qualifier, minmax, value)
                        for (name, qualifier, value) in test_settings))
 
 
index 3870e97..58f6657 100644 (file)
@@ -600,17 +600,17 @@ class ConfigWriter:
       except errors.ConfigurationError, err:
         result.append("%s has invalid nicparams: %s" % (owner, err))
 
-    def _helper_ipolicy(owner, params, check_std):
+    def _helper_ipolicy(owner, ipolicy, iscluster):
       try:
-        objects.InstancePolicy.CheckParameterSyntax(params, check_std)
+        objects.InstancePolicy.CheckParameterSyntax(ipolicy, iscluster)
       except errors.ConfigurationError, err:
         result.append("%s has invalid instance policy: %s" % (owner, err))
-
-    def _helper_ispecs(owner, params):
-      for key, value in params.items():
-        if key in constants.IPOLICY_ISPECS:
-          fullkey = "ipolicy/" + key
-          _helper(owner, fullkey, value, constants.ISPECS_PARAMETER_TYPES)
+      for key, value in ipolicy.items():
+        if key == constants.ISPECS_MINMAX:
+          _helper_ispecs(owner, "ipolicy/" + key, value)
+        elif key == constants.ISPECS_STD:
+          _helper(owner, "ipolicy/" + key, value,
+                  constants.ISPECS_PARAMETER_TYPES)
         else:
           # FIXME: assuming list type
           if key in constants.IPOLICY_PARAMETERS:
@@ -622,6 +622,11 @@ class ConfigWriter:
                           " expecting %s, got %s" %
                           (owner, key, exp_type.__name__, type(value)))
 
+    def _helper_ispecs(owner, parentkey, params):
+      for (key, value) in params.items():
+        fullkey = "/".join([parentkey, key])
+        _helper(owner, fullkey, value, constants.ISPECS_PARAMETER_TYPES)
+
     # check cluster parameters
     _helper("cluster", "beparams", cluster.SimpleFillBE({}),
             constants.BES_PARAMETER_TYPES)
@@ -630,8 +635,7 @@ class ConfigWriter:
     _helper_nic("cluster", cluster.SimpleFillNIC({}))
     _helper("cluster", "ndparams", cluster.SimpleFillND({}),
             constants.NDS_PARAMETER_TYPES)
-    _helper_ipolicy("cluster", cluster.SimpleFillIPolicy({}), True)
-    _helper_ispecs("cluster", cluster.SimpleFillIPolicy({}))
+    _helper_ipolicy("cluster", cluster.ipolicy, True)
 
     # per-instance checks
     for instance_name in data.instances:
@@ -762,7 +766,6 @@ class ConfigWriter:
       group_name = "group %s" % nodegroup.name
       _helper_ipolicy(group_name, cluster.SimpleFillIPolicy(nodegroup.ipolicy),
                       False)
-      _helper_ispecs(group_name, cluster.SimpleFillIPolicy(nodegroup.ipolicy))
       if nodegroup.ndparams:
         _helper(group_name, "ndparams",
                 cluster.SimpleFillND(nodegroup.ndparams),
index 5a4e465..3926f02 100644 (file)
@@ -1139,6 +1139,7 @@ ISPECS_PARAMETER_TYPES = {
 
 ISPECS_PARAMETERS = frozenset(ISPECS_PARAMETER_TYPES.keys())
 
+ISPECS_MINMAX = "minmax"
 ISPECS_MIN = "min"
 ISPECS_MAX = "max"
 ISPECS_STD = "std"
@@ -1146,10 +1147,9 @@ IPOLICY_DTS = "disk-templates"
 IPOLICY_VCPU_RATIO = "vcpu-ratio"
 IPOLICY_SPINDLE_RATIO = "spindle-ratio"
 
-IPOLICY_ISPECS = compat.UniqueFrozenset([
+ISPECS_MINMAX_KEYS = compat.UniqueFrozenset([
   ISPECS_MIN,
   ISPECS_MAX,
-  ISPECS_STD,
   ])
 
 IPOLICY_PARAMETERS = compat.UniqueFrozenset([
@@ -1157,9 +1157,8 @@ IPOLICY_PARAMETERS = compat.UniqueFrozenset([
   IPOLICY_SPINDLE_RATIO,
   ])
 
-IPOLICY_ALL_KEYS = (IPOLICY_ISPECS |
-                    IPOLICY_PARAMETERS |
-                    frozenset([IPOLICY_DTS]))
+IPOLICY_ALL_KEYS = (IPOLICY_PARAMETERS |
+                    frozenset([ISPECS_MINMAX, ISPECS_STD, IPOLICY_DTS]))
 
 # Node parameter names
 ND_OOB_PROGRAM = "oob_program"
@@ -2182,7 +2181,7 @@ NICC_DEFAULTS = {
 
 # All of the following values are quite arbitrarily - there are no
 # "good" defaults, these must be customised per-site
-IPOLICY_DEFAULTS = {
+ISPECS_MINMAX_DEFAULTS = {
   ISPECS_MIN: {
     ISPEC_MEM_SIZE: 128,
     ISPEC_CPU_COUNT: 1,
@@ -2199,6 +2198,9 @@ IPOLICY_DEFAULTS = {
     ISPEC_NIC_COUNT: MAX_NICS,
     ISPEC_SPINDLE_USE: 12,
     },
+  }
+IPOLICY_DEFAULTS = {
+  ISPECS_MINMAX: ISPECS_MINMAX_DEFAULTS,
   ISPECS_STD: {
     ISPEC_MEM_SIZE: 128,
     ISPEC_CPU_COUNT: 1,
index 782dacc..d8dbca0 100644 (file)
@@ -82,16 +82,28 @@ def FillDict(defaults_dict, custom_dict, skip_keys=None):
   return ret_dict
 
 
-def FillIPolicy(default_ipolicy, custom_ipolicy, skip_keys=None):
+def _FillMinMaxISpecs(default_specs, custom_specs):
+  assert frozenset(default_specs.keys()) == constants.ISPECS_MINMAX_KEYS
+  ret_specs = {}
+  for key in constants.ISPECS_MINMAX_KEYS:
+    ret_specs[key] = FillDict(default_specs[key],
+                              custom_specs.get(key, {}))
+  return ret_specs
+
+
+def FillIPolicy(default_ipolicy, custom_ipolicy):
   """Fills an instance policy with defaults.
 
   """
   assert frozenset(default_ipolicy.keys()) == constants.IPOLICY_ALL_KEYS
   ret_dict = {}
-  for key in constants.IPOLICY_ISPECS:
-    ret_dict[key] = FillDict(default_ipolicy[key],
-                             custom_ipolicy.get(key, {}),
-                             skip_keys=skip_keys)
+  # Instance specs
+  new_mm = _FillMinMaxISpecs(default_ipolicy[constants.ISPECS_MINMAX],
+                             custom_ipolicy.get(constants.ISPECS_MINMAX, {}))
+  ret_dict[constants.ISPECS_MINMAX] = new_mm
+  new_std = FillDict(default_ipolicy[constants.ISPECS_STD],
+                     custom_ipolicy.get(constants.ISPECS_STD, {}))
+  ret_dict[constants.ISPECS_STD] = new_std
   # list items
   for key in [constants.IPOLICY_DTS]:
     ret_dict[key] = list(custom_ipolicy.get(key, default_ipolicy[key]))
@@ -186,11 +198,13 @@ def MakeEmptyIPolicy():
   """Create empty IPolicy dictionary.
 
   """
-  return dict([
-    (constants.ISPECS_MIN, {}),
-    (constants.ISPECS_MAX, {}),
-    (constants.ISPECS_STD, {}),
-    ])
+  return {
+    constants.ISPECS_MINMAX: {
+      constants.ISPECS_MIN: {},
+      constants.ISPECS_MAX: {},
+      },
+    constants.ISPECS_STD: {},
+    }
 
 
 class ConfigObject(outils.ValidatedSlots):
@@ -912,7 +926,6 @@ class Disk(ConfigObject):
 class InstancePolicy(ConfigObject):
   """Config object representing instance policy limits dictionary.
 
-
   Note that this object is not actually used in the config, it's just
   used as a placeholder for a few functions.
 
@@ -921,9 +934,21 @@ class InstancePolicy(ConfigObject):
   def CheckParameterSyntax(cls, ipolicy, check_std):
     """ Check the instance policy for validity.
 
+    @type ipolicy: dict
+    @param ipolicy: dictionary with min/max/std specs and policies
+    @type check_std: bool
+    @param check_std: Whether to check std value or just assume compliance
+    @raise errors.ConfigurationError: when the policy is not legal
+
     """
-    for param in constants.ISPECS_PARAMETERS:
-      InstancePolicy.CheckISpecSyntax(ipolicy, param, check_std)
+    if constants.ISPECS_MINMAX in ipolicy:
+      if check_std and constants.ISPECS_STD not in ipolicy:
+        msg = "Missing key in ipolicy: %s" % constants.ISPECS_STD
+        raise errors.ConfigurationError(msg)
+      minmaxspecs = ipolicy[constants.ISPECS_MINMAX]
+      stdspec = ipolicy.get(constants.ISPECS_STD)
+      for param in constants.ISPECS_PARAMETERS:
+        InstancePolicy.CheckISpecSyntax(minmaxspecs, stdspec, param, check_std)
     if constants.IPOLICY_DTS in ipolicy:
       InstancePolicy.CheckDiskTemplates(ipolicy[constants.IPOLICY_DTS])
     for key in constants.IPOLICY_PARAMETERS:
@@ -935,37 +960,47 @@ class InstancePolicy(ConfigObject):
                                       utils.CommaJoin(wrong_keys))
 
   @classmethod
-  def CheckISpecSyntax(cls, ipolicy, name, check_std):
-    """Check the instance policy for validity on a given key.
+  def CheckISpecSyntax(cls, minmaxspecs, stdspec, name, check_std):
+    """Check the instance policy specs for validity on a given key.
 
-    We check if the instance policy makes sense for a given key, that is
-    if ipolicy[min][name] <= ipolicy[std][name] <= ipolicy[max][name].
+    We check if the instance specs makes sense for a given key, that is
+    if minmaxspecs[min][name] <= stdspec[name] <= minmaxspec[max][name].
 
-    @type ipolicy: dict
-    @param ipolicy: dictionary with min, max, std specs
+    @type minmaxspecs: dict
+    @param minmaxspecs: dictionary with min and max instance spec
+    @type stdspec: dict
+    @param stdspec: dictionary with standard instance spec
     @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
+    @raise errors.ConfigurationError: when specs for the given name are not
+        valid
 
     """
-    min_v = ipolicy[constants.ISPECS_MIN].get(name, 0)
+    missing = constants.ISPECS_MINMAX_KEYS - frozenset(minmaxspecs.keys())
+    if missing:
+      msg = "Missing instance specification: %s" % utils.CommaJoin(missing)
+      raise errors.ConfigurationError(msg)
+
+    minspec = minmaxspecs[constants.ISPECS_MIN]
+    maxspec = minmaxspecs[constants.ISPECS_MAX]
+    min_v = minspec.get(name, 0)
 
     if check_std:
-      std_v = ipolicy[constants.ISPECS_STD].get(name, min_v)
+      std_v = stdspec.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, "-"),
-            std_msg))
+    max_v = maxspec.get(name, std_v)
     if min_v > std_v or std_v > max_v:
+      err = ("Invalid specification of min/max/std values for %s: %s/%s/%s" %
+             (name,
+              minspec.get(name, "-"),
+              maxspec.get(name, "-"),
+              std_msg))
       raise errors.ConfigurationError(err)
 
   @classmethod
index e608b8c..c7e02c5 100644 (file)
@@ -467,8 +467,9 @@ def _GetClusterIPolicy():
   policy = info["Instance policy - limits for instances"]
   ret_specs = {}
   ret_policy = {}
+  ispec_keys = constants.ISPECS_MINMAX_KEYS | frozenset([constants.ISPECS_STD])
   for (key, val) in policy.items():
-    if key in constants.IPOLICY_ISPECS:
+    if key in ispec_keys:
       for (par, pval) in val.items():
         if par == "memory-size":
           par = "mem-size"
index cb3719c..2ca928f 100644 (file)
@@ -7,7 +7,7 @@ files, as produced by @gnt-node@ and @gnt-instance@ @list@ command.
 
 {-
 
-Copyright (C) 2009, 2010, 2011, 2012 Google Inc.
+Copyright (C) 2009, 2010, 2011, 2012, 2013 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
@@ -132,7 +132,8 @@ serializeDiskTemplates = intercalate "," . map diskTemplateToRaw
 -- | Generate policy data from a given policy object.
 serializeIPolicy :: String -> IPolicy -> String
 serializeIPolicy owner ipol =
-  let IPolicy stdspec minspec maxspec dts vcpu_ratio spindle_ratio = ipol
+  let IPolicy minmax stdspec dts vcpu_ratio spindle_ratio = ipol
+      MinMaxISpecs minspec maxspec = minmax
       strings = [ owner
                 , serializeISpec stdspec
                 , serializeISpec minspec
@@ -263,7 +264,8 @@ loadIPolicy [owner, stdspec, minspec, maxspec, dtemplates,
   xvcpu_ratio <- tryRead (owner ++ "/vcpu_ratio") vcpu_ratio
   xspindle_ratio <- tryRead (owner ++ "/spindle_ratio") spindle_ratio
   return (owner,
-          IPolicy xstdspec xminspec xmaxspec xdts xvcpu_ratio xspindle_ratio)
+          IPolicy (MinMaxISpecs xminspec xmaxspec) xstdspec
+                xdts xvcpu_ratio xspindle_ratio)
 loadIPolicy s = fail $ "Invalid ipolicy data: '" ++ show s ++ "'"
 
 loadOnePolicy :: (IPolicy, Group.List) -> String
index 9c20af1..d227cf1 100644 (file)
@@ -7,7 +7,7 @@ intelligence is in the "Node" and "Cluster" modules.
 
 {-
 
-Copyright (C) 2009, 2010, 2011, 2012 Google Inc.
+Copyright (C) 2009, 2010, 2011, 2012, 2013 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
@@ -280,8 +280,9 @@ instAboveISpec inst ispec
 -- | Checks if an instance matches a policy.
 instMatchesPolicy :: Instance -> T.IPolicy -> T.OpResult ()
 instMatchesPolicy inst ipol = do
-  instAboveISpec inst (T.iPolicyMinSpec ipol)
-  instBelowISpec inst (T.iPolicyMaxSpec ipol)
+  let minmax = T.iPolicyMinMaxISpecs ipol
+  instAboveISpec inst (T.minMaxISpecsMinSpec minmax)
+  instBelowISpec inst (T.minMaxISpecsMaxSpec minmax)
   if diskTemplate inst `elem` T.iPolicyDiskTemplates ipol
     then Ok ()
     else Bad T.FailDisk
index 02c81bf..73d39a4 100644 (file)
@@ -443,7 +443,8 @@ main opts args = do
 
   -- Run the tiered allocation
 
-  let tspec = fromMaybe (rspecFromISpec (iPolicyMaxSpec ipol))
+  let minmax = iPolicyMinMaxISpecs ipol
+  let tspec = fromMaybe (rspecFromISpec (minMaxISpecsMaxSpec minmax))
               (optTieredSpec opts)
 
   (treason, trl_nl, _, spec_map) <-
index c394c6c..e579e36 100644 (file)
@@ -6,7 +6,7 @@
 
 {-
 
-Copyright (C) 2009, 2010, 2011, 2012 Google Inc.
+Copyright (C) 2009, 2010, 2011, 2012, 2013 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
@@ -69,6 +69,7 @@ module Ganeti.HTools.Types
   , opToResult
   , EvacMode(..)
   , ISpec(..)
+  , MinMaxISpecs(..)
   , IPolicy(..)
   , defIPolicy
   , rspecFromISpec
@@ -168,12 +169,12 @@ $(THH.buildObject "ISpec" "iSpec"
 
 -- | The default minimum ispec.
 defMinISpec :: ISpec
-defMinISpec = ISpec { iSpecMemorySize = C.ipolicyDefaultsMinMemorySize
-                    , iSpecCpuCount   = C.ipolicyDefaultsMinCpuCount
-                    , iSpecDiskSize   = C.ipolicyDefaultsMinDiskSize
-                    , iSpecDiskCount  = C.ipolicyDefaultsMinDiskCount
-                    , iSpecNicCount   = C.ipolicyDefaultsMinNicCount
-                    , iSpecSpindleUse = C.ipolicyDefaultsMinSpindleUse
+defMinISpec = ISpec { iSpecMemorySize = C.ipolicyDefaultsMinmaxMinMemorySize
+                    , iSpecCpuCount   = C.ipolicyDefaultsMinmaxMinCpuCount
+                    , iSpecDiskSize   = C.ipolicyDefaultsMinmaxMinDiskSize
+                    , iSpecDiskCount  = C.ipolicyDefaultsMinmaxMinDiskCount
+                    , iSpecNicCount   = C.ipolicyDefaultsMinmaxMinNicCount
+                    , iSpecSpindleUse = C.ipolicyDefaultsMinmaxMinSpindleUse
                     }
 
 -- | The default standard ispec.
@@ -188,19 +189,31 @@ defStdISpec = ISpec { iSpecMemorySize = C.ipolicyDefaultsStdMemorySize
 
 -- | The default max ispec.
 defMaxISpec :: ISpec
-defMaxISpec = ISpec { iSpecMemorySize = C.ipolicyDefaultsMaxMemorySize
-                    , iSpecCpuCount   = C.ipolicyDefaultsMaxCpuCount
-                    , iSpecDiskSize   = C.ipolicyDefaultsMaxDiskSize
-                    , iSpecDiskCount  = C.ipolicyDefaultsMaxDiskCount
-                    , iSpecNicCount   = C.ipolicyDefaultsMaxNicCount
-                    , iSpecSpindleUse = C.ipolicyDefaultsMaxSpindleUse
+defMaxISpec = ISpec { iSpecMemorySize = C.ipolicyDefaultsMinmaxMaxMemorySize
+                    , iSpecCpuCount   = C.ipolicyDefaultsMinmaxMaxCpuCount
+                    , iSpecDiskSize   = C.ipolicyDefaultsMinmaxMaxDiskSize
+                    , iSpecDiskCount  = C.ipolicyDefaultsMinmaxMaxDiskCount
+                    , iSpecNicCount   = C.ipolicyDefaultsMinmaxMaxNicCount
+                    , iSpecSpindleUse = C.ipolicyDefaultsMinmaxMaxSpindleUse
                     }
 
+-- | Minimum and maximum instance specs type.
+$(THH.buildObject "MinMaxISpecs" "minMaxISpecs"
+  [ THH.renameField "MinSpec" $ THH.simpleField "min" [t| ISpec |]
+  , THH.renameField "MaxSpec" $ THH.simpleField "max" [t| ISpec |]
+  ])
+
+-- | Defult minimum and maximum instance specs.
+defMinMaxISpecs :: MinMaxISpecs
+defMinMaxISpecs = MinMaxISpecs { minMaxISpecsMinSpec = defMinISpec
+                               , minMaxISpecsMaxSpec = defMaxISpec
+                               }
+
 -- | Instance policy type.
 $(THH.buildObject "IPolicy" "iPolicy"
-  [ THH.renameField "StdSpec" $ THH.simpleField C.ispecsStd [t| ISpec |]
-  , THH.renameField "MinSpec" $ THH.simpleField C.ispecsMin [t| ISpec |]
-  , THH.renameField "MaxSpec" $ THH.simpleField C.ispecsMax [t| ISpec |]
+  [ THH.renameField "MinMaxISpecs" $
+      THH.simpleField C.ispecsMinmax [t| MinMaxISpecs |]
+  , THH.renameField "StdSpec" $ THH.simpleField C.ispecsStd [t| ISpec |]
   , THH.renameField "DiskTemplates" $
       THH.simpleField C.ipolicyDts [t| [DiskTemplate] |]
   , THH.renameField "VcpuRatio" $
@@ -218,9 +231,8 @@ rspecFromISpec ispec = RSpec { rspecCpu = iSpecCpuCount ispec
 
 -- | The default instance policy.
 defIPolicy :: IPolicy
-defIPolicy = IPolicy { iPolicyStdSpec = defStdISpec
-                     , iPolicyMinSpec = defMinISpec
-                     , iPolicyMaxSpec = defMaxISpec
+defIPolicy = IPolicy { iPolicyMinMaxISpecs = defMinMaxISpecs
+                     , iPolicyStdSpec = defStdISpec
                      -- hardcoding here since Constants.hs exports the
                      -- string values, not the actual type; and in
                      -- htools, we are mostly looking at DRBD
index 7e85cf1..bfbab6b 100644 (file)
@@ -66,6 +66,9 @@ module Ganeti.Objects
   , PartialISpecParams(..)
   , fillISpecParams
   , allISpecParamFields
+  , FilledMinMaxISpecs(..)
+  , PartialMinMaxISpecs(..)
+  , fillMinMaxISpecs
   , FilledIPolicy(..)
   , PartialIPolicy(..)
   , fillIPolicy
@@ -491,11 +494,25 @@ $(buildParam "ISpec" "ispec"
   , simpleField C.ispecSpindleUse  [t| Int |]
   ])
 
+-- | Partial min-max instance specs. These is not built via buildParam since
+-- it has a special 2-level inheritance mode.
+$(buildObject "PartialMinMaxISpecs" "mmis"
+  [ renameField "MinSpecP" $ simpleField "min" [t| PartialISpecParams |]
+  , renameField "MaxSpecP" $ simpleField "max" [t| PartialISpecParams |]
+  ])
+
+-- | Filled min-max instance specs. This is not built via buildParam since
+-- it has a special 2-level inheritance mode.
+$(buildObject "FilledMinMaxISpecs" "mmis"
+  [ renameField "MinSpec" $ simpleField "min" [t| FilledISpecParams |]
+  , renameField "MaxSpec" $ simpleField "max" [t| FilledISpecParams |]
+  ])
+
 -- | Custom partial ipolicy. This is not built via buildParam since it
 -- has a special 2-level inheritance mode.
 $(buildObject "PartialIPolicy" "ipolicy"
-  [ renameField "MinSpecP" $ simpleField "min" [t| PartialISpecParams |]
-  , renameField "MaxSpecP" $ simpleField "max" [t| PartialISpecParams |]
+  [ optionalField . renameField "MinMaxISpecsP"
+                    $ simpleField C.ispecsMinmax [t| PartialMinMaxISpecs |]
   , renameField "StdSpecP" $ simpleField "std" [t| PartialISpecParams |]
   , optionalField . renameField "SpindleRatioP"
                     $ simpleField "spindle-ratio"  [t| Double |]
@@ -508,30 +525,38 @@ $(buildObject "PartialIPolicy" "ipolicy"
 -- | Custom filled ipolicy. This is not built via buildParam since it
 -- has a special 2-level inheritance mode.
 $(buildObject "FilledIPolicy" "ipolicy"
-  [ renameField "MinSpec" $ simpleField "min" [t| FilledISpecParams |]
-  , renameField "MaxSpec" $ simpleField "max" [t| FilledISpecParams |]
+  [ renameField "MinMaxISpecs"
+    $ simpleField C.ispecsMinmax [t| FilledMinMaxISpecs |]
   , renameField "StdSpec" $ simpleField "std" [t| FilledISpecParams |]
   , simpleField "spindle-ratio"  [t| Double |]
   , simpleField "vcpu-ratio"     [t| Double |]
   , simpleField "disk-templates" [t| [DiskTemplate] |]
   ])
 
+-- | Custom filler for the min-max instance specs.
+fillMinMaxISpecs :: FilledMinMaxISpecs -> Maybe PartialMinMaxISpecs ->
+                    FilledMinMaxISpecs
+fillMinMaxISpecs fminmax Nothing = fminmax
+fillMinMaxISpecs (FilledMinMaxISpecs { mmisMinSpec = fmin
+                                     , mmisMaxSpec = fmax })
+                 (Just PartialMinMaxISpecs { mmisMinSpecP = pmin
+                                           , mmisMaxSpecP = pmax }) =
+  FilledMinMaxISpecs { mmisMinSpec = fillISpecParams fmin pmin
+                     , mmisMaxSpec = fillISpecParams fmax pmax }
+
 -- | Custom filler for the ipolicy types.
 fillIPolicy :: FilledIPolicy -> PartialIPolicy -> FilledIPolicy
-fillIPolicy (FilledIPolicy { ipolicyMinSpec       = fmin
-                           , ipolicyMaxSpec       = fmax
+fillIPolicy (FilledIPolicy { ipolicyMinMaxISpecs  = fminmax
                            , ipolicyStdSpec       = fstd
                            , ipolicySpindleRatio  = fspindleRatio
                            , ipolicyVcpuRatio     = fvcpuRatio
                            , ipolicyDiskTemplates = fdiskTemplates})
-            (PartialIPolicy { ipolicyMinSpecP       = pmin
-                            , ipolicyMaxSpecP       = pmax
+            (PartialIPolicy { ipolicyMinMaxISpecsP  = pminmax
                             , ipolicyStdSpecP       = pstd
                             , ipolicySpindleRatioP  = pspindleRatio
                             , ipolicyVcpuRatioP     = pvcpuRatio
                             , ipolicyDiskTemplatesP = pdiskTemplates}) =
-  FilledIPolicy { ipolicyMinSpec       = fillISpecParams fmin pmin
-                , ipolicyMaxSpec       = fillISpecParams fmax pmax
+  FilledIPolicy { ipolicyMinMaxISpecs  = fillMinMaxISpecs fminmax pminmax
                 , ipolicyStdSpec       = fillISpecParams fstd pstd
                 , ipolicySpindleRatio  = fromMaybe fspindleRatio pspindleRatio
                 , ipolicyVcpuRatio     = fromMaybe fvcpuRatio pvcpuRatio
index 1f5686a..f650fbe 100644 (file)
           "cpu-count": 1,
           "spindle-use": 1
         },
-        "min": {
-          "nic-count": 1,
-          "disk-size": 128,
-          "disk-count": 1,
-          "memory-size": 128,
-          "cpu-count": 1,
-          "spindle-use": 1
-        },
-        "max": {
-          "nic-count": 8,
-          "disk-size": 1048576,
-          "disk-count": 16,
-          "memory-size": 32768,
-          "cpu-count": 8,
-          "spindle-use": 8
+        "minmax": {
+         "min": {
+           "nic-count": 1,
+           "disk-size": 128,
+           "disk-count": 1,
+           "memory-size": 128,
+           "cpu-count": 1,
+           "spindle-use": 1
+         },
+         "max": {
+           "nic-count": 8,
+           "disk-size": 1048576,
+           "disk-count": 16,
+           "memory-size": 32768,
+           "cpu-count": 8,
+           "spindle-use": 8
+         }
         },
         "vcpu-ratio": 4.0,
         "disk-templates": [
index 08d8908..f698638 100644 (file)
           "cpu-count": 1,
           "spindle-use": 1
         },
-        "min": {
-          "nic-count": 1,
-          "disk-size": 128,
-          "disk-count": 1,
-          "memory-size": 128,
-          "cpu-count": 1,
-          "spindle-use": 1
-        },
-        "max": {
-          "nic-count": 8,
-          "disk-size": 1048576,
-          "disk-count": 16,
-          "memory-size": 32768,
-          "cpu-count": 8,
-          "spindle-use": 8
+        "minmax": {
+         "min": {
+           "nic-count": 1,
+           "disk-size": 128,
+           "disk-count": 1,
+           "memory-size": 128,
+           "cpu-count": 1,
+           "spindle-use": 1
+         },
+         "max": {
+           "nic-count": 8,
+           "disk-size": 1048576,
+           "disk-count": 16,
+           "memory-size": 32768,
+           "cpu-count": 8,
+           "spindle-use": 8
+         }
         },
         "vcpu-ratio": 4.0,
         "disk-templates": [
           "cpu-count": 1,
           "spindle-use": 1
         },
-        "min": {
-          "nic-count": 1,
-          "disk-size": 128,
-          "disk-count": 1,
-          "memory-size": 128,
-          "cpu-count": 1,
-          "spindle-use": 1
-        },
-        "max": {
-          "nic-count": 8,
-          "disk-size": 1048576,
-          "disk-count": 16,
-          "memory-size": 32768,
-          "cpu-count": 8,
-          "spindle-use": 8
+        "minmax": {
+         "min": {
+           "nic-count": 1,
+           "disk-size": 128,
+           "disk-count": 1,
+           "memory-size": 128,
+           "cpu-count": 1,
+           "spindle-use": 1
+         },
+         "max": {
+           "nic-count": 8,
+           "disk-size": 1048576,
+           "disk-count": 16,
+           "memory-size": 32768,
+           "cpu-count": 8,
+           "spindle-use": 8
+         }
         },
         "vcpu-ratio": 4.0,
         "disk-templates": [
       "disk-count": 1,
       "spindle-use": 1
     },
-    "min": {
-      "nic-count": 1,
-      "disk-size": 1024,
-      "memory-size": 128,
-      "cpu-count": 1,
-      "disk-count": 1,
-      "spindle-use": 1
-    },
-    "max": {
-      "nic-count": 8,
-      "disk-size": 1048576,
-      "memory-size": 32768,
-      "cpu-count": 8,
-      "disk-count": 16,
-      "spindle-use": 8
+    "minmax": {
+      "min": {
+       "nic-count": 1,
+       "disk-size": 1024,
+       "memory-size": 128,
+       "cpu-count": 1,
+       "disk-count": 1,
+       "spindle-use": 1
+      },
+      "max": {
+       "nic-count": 8,
+       "disk-size": 1048576,
+       "memory-size": 32768,
+       "cpu-count": 8,
+       "disk-count": 16,
+       "spindle-use": 8
+      }
     },
     "vcpu-ratio": 4.0,
     "disk-templates": [
index 941d41d..8b80e25 100644 (file)
           "cpu-count": 1,
           "spindle-use": 1
         },
-        "min": {
-          "nic-count": 1,
-          "disk-size": 128,
-          "disk-count": 1,
-          "memory-size": 128,
-          "cpu-count": 1,
-          "spindle-use": 1
-        },
-        "max": {
-          "nic-count": 8,
-          "disk-size": 1048576,
-          "disk-count": 16,
-          "memory-size": 32768,
-          "cpu-count": 8,
-          "spindle-use": 8
+        "minmax": {
+         "min": {
+           "nic-count": 1,
+           "disk-size": 128,
+           "disk-count": 1,
+           "memory-size": 128,
+           "cpu-count": 1,
+           "spindle-use": 1
+         },
+         "max": {
+           "nic-count": 8,
+           "disk-size": 1048576,
+           "disk-count": 16,
+           "memory-size": 32768,
+           "cpu-count": 8,
+           "spindle-use": 8
+         }
         },
         "vcpu-ratio": 4.0,
         "disk-templates": [
index 09ef2bb..ce66041 100644 (file)
           "cpu-count": 1,
           "spindle-use": 1
         },
-        "min": {
-          "nic-count": 1,
-          "disk-size": 128,
-          "disk-count": 1,
-          "memory-size": 128,
-          "cpu-count": 1,
-          "spindle-use": 1
-        },
-        "max": {
-          "nic-count": 8,
-          "disk-size": 1048576,
-          "disk-count": 16,
-          "memory-size": 32768,
-          "cpu-count": 8,
-          "spindle-use": 8
+        "minmax": {
+         "min": {
+           "nic-count": 1,
+           "disk-size": 128,
+           "disk-count": 1,
+           "memory-size": 128,
+           "cpu-count": 1,
+           "spindle-use": 1
+         },
+         "max": {
+           "nic-count": 8,
+           "disk-size": 1048576,
+           "disk-count": 16,
+           "memory-size": 32768,
+           "cpu-count": 8,
+           "spindle-use": 8
+         }
         },
         "vcpu-ratio": 4.0,
         "disk-templates": [
index 226eed7..dab6569 100644 (file)
         "disk-count": 1,
         "spindle-use": 1
       },
-      "min": {
-        "cpu-count": 1,
-        "nic-count": 1,
-        "disk-size": 1024,
-        "memory-size": 128,
-        "disk-count": 1,
-        "spindle-use": 1
-      },
-      "max": {
-        "cpu-count": 8,
-        "nic-count": 8,
-        "disk-size": 1048576,
-        "memory-size": 32768,
-        "disk-count": 16,
-        "spindle-use": 8
+      "minmax": {
+       "min": {
+         "cpu-count": 1,
+         "nic-count": 1,
+         "disk-size": 1024,
+         "memory-size": 128,
+         "disk-count": 1,
+         "spindle-use": 1
+       },
+       "max": {
+         "cpu-count": 8,
+         "nic-count": 8,
+         "disk-size": 1048576,
+         "memory-size": 32768,
+         "disk-count": 16,
+         "spindle-use": 8
+       }
       },
       "vcpu-ratio": 4.0,
       "disk-templates": [
index 821a320..a37d4e5 100644 (file)
       "cpu-count": 1,
       "spindle-use": 1
     },
-    "min": {
-      "nic-count": 1,
-      "disk-size": 128,
-      "disk-count": 1,
-      "memory-size": 128,
-      "cpu-count": 1,
-      "spindle-use": 1
-    },
-    "max": {
-      "nic-count": 8,
-      "disk-size": 1048576,
-      "disk-count": 16,
-      "memory-size": 32768,
-      "cpu-count": 8,
-      "spindle-use": 8
+    "minmax": {
+      "min": {
+       "nic-count": 1,
+       "disk-size": 128,
+       "disk-count": 1,
+       "memory-size": 128,
+       "cpu-count": 1,
+       "spindle-use": 1
+      },
+      "max": {
+       "nic-count": 8,
+       "disk-size": 1048576,
+       "disk-count": 16,
+       "memory-size": 32768,
+       "cpu-count": 8,
+       "spindle-use": 8
+      }
     },
     "vcpu-ratio": 4.0,
     "disk-templates": [
index da21725..3980edb 100644 (file)
@@ -110,9 +110,11 @@ instance Arbitrary Types.IPolicy where
     dts  <- genUniquesList num_tmpl arbitrary
     vcpu_ratio <- choose (1.0, maxVcpuRatio)
     spindle_ratio <- choose (1.0, maxSpindleRatio)
-    return Types.IPolicy { Types.iPolicyMinSpec = imin
+    return Types.IPolicy { Types.iPolicyMinMaxISpecs = Types.MinMaxISpecs
+                           { Types.minMaxISpecsMinSpec = imin
+                           , Types.minMaxISpecsMaxSpec = imax
+                           }
                          , Types.iPolicyStdSpec = istd
-                         , Types.iPolicyMaxSpec = imax
                          , Types.iPolicyDiskTemplates = dts
                          , Types.iPolicyVcpuRatio = vcpu_ratio
                          , Types.iPolicySpindleRatio = spindle_ratio
index 1655aaf..744d2b6 100644 (file)
@@ -139,12 +139,14 @@ genInstWithNets nets = do
 -- | FIXME: This generates completely random data, without normal
 -- validation rules.
 $(genArbitrary ''PartialISpecParams)
+$(genArbitrary ''PartialMinMaxISpecs)
 
 -- | FIXME: This generates completely random data, without normal
 -- validation rules.
 $(genArbitrary ''PartialIPolicy)
 
 $(genArbitrary ''FilledISpecParams)
+$(genArbitrary ''FilledMinMaxISpecs)
 $(genArbitrary ''FilledIPolicy)
 $(genArbitrary ''IpFamily)
 $(genArbitrary ''FilledNDParams)
index b27c34c..8758509 100644 (file)
@@ -52,20 +52,23 @@ import qualified Ganeti.HTools.Types as Types
 -- | Null iPolicy, and by null we mean very liberal.
 nullIPolicy :: Types.IPolicy
 nullIPolicy = Types.IPolicy
-  { Types.iPolicyMinSpec = Types.ISpec { Types.iSpecMemorySize = 0
-                                       , Types.iSpecCpuCount   = 0
-                                       , Types.iSpecDiskSize   = 0
-                                       , Types.iSpecDiskCount  = 0
-                                       , Types.iSpecNicCount   = 0
-                                       , Types.iSpecSpindleUse = 0
-                                       }
-  , Types.iPolicyMaxSpec = Types.ISpec { Types.iSpecMemorySize = maxBound
-                                       , Types.iSpecCpuCount   = maxBound
-                                       , Types.iSpecDiskSize   = maxBound
-                                       , Types.iSpecDiskCount  = C.maxDisks
-                                       , Types.iSpecNicCount   = C.maxNics
-                                       , Types.iSpecSpindleUse = maxBound
-                                       }
+  { Types.iPolicyMinMaxISpecs = Types.MinMaxISpecs
+    { Types.minMaxISpecsMinSpec = Types.ISpec { Types.iSpecMemorySize = 0
+                                              , Types.iSpecCpuCount   = 0
+                                              , Types.iSpecDiskSize   = 0
+                                              , Types.iSpecDiskCount  = 0
+                                              , Types.iSpecNicCount   = 0
+                                              , Types.iSpecSpindleUse = 0
+                                              }
+    , Types.minMaxISpecsMaxSpec = Types.ISpec
+      { Types.iSpecMemorySize = maxBound
+      , Types.iSpecCpuCount   = maxBound
+      , Types.iSpecDiskSize   = maxBound
+      , Types.iSpecDiskCount  = C.maxDisks
+      , Types.iSpecNicCount   = C.maxNics
+      , Types.iSpecSpindleUse = maxBound
+      }
+    }
   , Types.iPolicyStdSpec = Types.ISpec { Types.iSpecMemorySize = Types.unitMem
                                        , Types.iSpecCpuCount   = Types.unitCpu
                                        , Types.iSpecDiskSize   = Types.unitDsk
index 4a3072e..c38ad0e 100755 (executable)
@@ -1141,18 +1141,22 @@ class TestCreateIPolicyFromOpts(unittest.TestCase):
 
   def testClusterPolicy(self):
     exp_pol0 = {
-      constants.ISPECS_MIN: {},
-      constants.ISPECS_MAX: {},
+      constants.ISPECS_MINMAX: {
+        constants.ISPECS_MIN: {},
+        constants.ISPECS_MAX: {},
+        },
       constants.ISPECS_STD: {},
       }
     exp_pol1 = {
-      constants.ISPECS_MIN: {
-        constants.ISPEC_CPU_COUNT: 2,
-        constants.ISPEC_DISK_COUNT: 1,
-        },
-      constants.ISPECS_MAX: {
-        constants.ISPEC_MEM_SIZE: 12*1024,
-        constants.ISPEC_DISK_COUNT: 2,
+      constants.ISPECS_MINMAX: {
+        constants.ISPECS_MIN: {
+          constants.ISPEC_CPU_COUNT: 2,
+          constants.ISPEC_DISK_COUNT: 1,
+          },
+        constants.ISPECS_MAX: {
+          constants.ISPEC_MEM_SIZE: 12*1024,
+          constants.ISPEC_DISK_COUNT: 2,
+          },
         },
       constants.ISPECS_STD: {
         constants.ISPEC_CPU_COUNT: 2,
@@ -1161,12 +1165,14 @@ class TestCreateIPolicyFromOpts(unittest.TestCase):
       constants.IPOLICY_VCPU_RATIO: 3.1,
       }
     exp_pol2 = {
-      constants.ISPECS_MIN: {
-        constants.ISPEC_DISK_SIZE: 512,
-        constants.ISPEC_NIC_COUNT: 2,
-        },
-      constants.ISPECS_MAX: {
-        constants.ISPEC_NIC_COUNT: 3,
+      constants.ISPECS_MINMAX: {
+        constants.ISPECS_MIN: {
+          constants.ISPEC_DISK_SIZE: 512,
+          constants.ISPEC_NIC_COUNT: 2,
+          },
+        constants.ISPECS_MAX: {
+          constants.ISPEC_NIC_COUNT: 3,
+          },
         },
       constants.ISPECS_STD: {
         constants.ISPEC_CPU_COUNT: 2,
@@ -1245,10 +1251,12 @@ class TestCreateIPolicyFromOpts(unittest.TestCase):
   def testAllowedValues(self):
     allowedv = "blah"
     exp_pol1 = {
-      constants.ISPECS_MIN: {
-        constants.ISPEC_CPU_COUNT: allowedv,
-        },
-      constants.ISPECS_MAX: {
+      constants.ISPECS_MINMAX: {
+        constants.ISPECS_MIN: {
+          constants.ISPEC_CPU_COUNT: allowedv,
+          },
+        constants.ISPECS_MAX: {
+          },
         },
       constants.ISPECS_STD: {
         },
index 85f1969..5603e4a 100755 (executable)
@@ -1,7 +1,7 @@
 #!/usr/bin/python
 #
 
-# Copyright (C) 2008, 2011, 2012 Google Inc.
+# Copyright (C) 2008, 2011, 2012, 2013 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
@@ -589,7 +589,7 @@ class TestDiskStateHelper(unittest.TestCase):
 
 class TestComputeMinMaxSpec(unittest.TestCase):
   def setUp(self):
-    self.ipolicy = {
+    self.ispecs = {
       constants.ISPECS_MAX: {
         constants.ISPEC_MEM_SIZE: 512,
         constants.ISPEC_DISK_SIZE: 1024,
@@ -602,38 +602,38 @@ class TestComputeMinMaxSpec(unittest.TestCase):
 
   def testNoneValue(self):
     self.assertTrue(cmdlib._ComputeMinMaxSpec(constants.ISPEC_MEM_SIZE, None,
-                                              self.ipolicy, None) is None)
+                                              self.ispecs, None) is None)
 
   def testAutoValue(self):
     self.assertTrue(cmdlib._ComputeMinMaxSpec(constants.ISPEC_MEM_SIZE, None,
-                                              self.ipolicy,
+                                              self.ispecs,
                                               constants.VALUE_AUTO) is None)
 
   def testNotDefined(self):
     self.assertTrue(cmdlib._ComputeMinMaxSpec(constants.ISPEC_NIC_COUNT, None,
-                                              self.ipolicy, 3) is None)
+                                              self.ispecs, 3) is None)
 
   def testNoMinDefined(self):
     self.assertTrue(cmdlib._ComputeMinMaxSpec(constants.ISPEC_DISK_SIZE, None,
-                                              self.ipolicy, 128) is None)
+                                              self.ispecs, 128) is None)
 
   def testNoMaxDefined(self):
     self.assertTrue(cmdlib._ComputeMinMaxSpec(constants.ISPEC_DISK_COUNT, None,
-                                                self.ipolicy, 16) is None)
+                                                self.ispecs, 16) is None)
 
   def testOutOfRange(self):
     for (name, val) in ((constants.ISPEC_MEM_SIZE, 64),
                         (constants.ISPEC_MEM_SIZE, 768),
                         (constants.ISPEC_DISK_SIZE, 4096),
                         (constants.ISPEC_DISK_COUNT, 0)):
-      min_v = self.ipolicy[constants.ISPECS_MIN].get(name, val)
-      max_v = self.ipolicy[constants.ISPECS_MAX].get(name, val)
+      min_v = self.ispecs[constants.ISPECS_MIN].get(name, val)
+      max_v = self.ispecs[constants.ISPECS_MAX].get(name, val)
       self.assertEqual(cmdlib._ComputeMinMaxSpec(name, None,
-                                                 self.ipolicy, val),
+                                                 self.ispecs, val),
                        "%s value %s is not in range [%s, %s]" %
                        (name, val,min_v, max_v))
       self.assertEqual(cmdlib._ComputeMinMaxSpec(name, "1",
-                                                 self.ipolicy, val),
+                                                 self.ispecs, val),
                        "%s/1 value %s is not in range [%s, %s]" %
                        (name, val,min_v, max_v))
 
@@ -645,7 +645,7 @@ class TestComputeMinMaxSpec(unittest.TestCase):
                         (constants.ISPEC_DISK_SIZE, 0),
                         (constants.ISPEC_DISK_COUNT, 1),
                         (constants.ISPEC_DISK_COUNT, 5)):
-      self.assertTrue(cmdlib._ComputeMinMaxSpec(name, None, self.ipolicy, val)
+      self.assertTrue(cmdlib._ComputeMinMaxSpec(name, None, self.ispecs, val)
                       is None)
 
 
@@ -673,6 +673,7 @@ class TestComputeIPolicySpecViolation(unittest.TestCase):
   # Minimal policy accepted by _ComputeIPolicySpecViolation()
   _MICRO_IPOL = {
     constants.IPOLICY_DTS: [constants.DT_PLAIN, constants.DT_DISKLESS],
+    constants.ISPECS_MINMAX: NotImplemented,
     }
 
   def test(self):
@@ -1732,21 +1733,24 @@ class TestGetUpdatedIPolicy(unittest.TestCase):
   """Tests for cmdlib._GetUpdatedIPolicy()"""
   _OLD_CLUSTER_POLICY = {
     constants.IPOLICY_VCPU_RATIO: 1.5,
-    constants.ISPECS_MIN: {
-      constants.ISPEC_MEM_SIZE: 20,
-      constants.ISPEC_CPU_COUNT: 2,
+    constants.ISPECS_MINMAX: {
+      constants.ISPECS_MIN: {
+        constants.ISPEC_MEM_SIZE: 20,
+        constants.ISPEC_CPU_COUNT: 2,
+        },
+      constants.ISPECS_MAX: {},
       },
-    constants.ISPECS_MAX: {},
     constants.ISPECS_STD: {},
     }
-
   _OLD_GROUP_POLICY = {
     constants.IPOLICY_SPINDLE_RATIO: 2.5,
-    constants.ISPECS_MIN: {
-      constants.ISPEC_DISK_SIZE: 20,
-      constants.ISPEC_NIC_COUNT: 2,
+    constants.ISPECS_MINMAX: {
+      constants.ISPECS_MIN: {
+        constants.ISPEC_DISK_SIZE: 20,
+        constants.ISPEC_NIC_COUNT: 2,
+        },
+      constants.ISPECS_MAX: {},
       },
-    constants.ISPECS_MAX: {},
     }
 
   def _TestSetSpecs(self, old_policy, isgroup):
@@ -1756,11 +1760,21 @@ class TestGetUpdatedIPolicy(unittest.TestCase):
       constants.ISPEC_DISK_SIZE: 30,
       }
     diff_policy = {
-      ispec_key: diff_ispec
+      constants.ISPECS_MINMAX: {
+        ispec_key: diff_ispec,
+        },
       }
+    if not isgroup:
+      diff_std = {
+        constants.ISPEC_CPU_COUNT: 3,
+        constants.ISPEC_DISK_COUNT: 3,
+        }
+      diff_policy[constants.ISPECS_STD] = diff_std
     new_policy = cmdlib._GetUpdatedIPolicy(old_policy, diff_policy,
                                            group_policy=isgroup)
-    new_ispec = new_policy[ispec_key]
+
+    self.assertTrue(constants.ISPECS_MINMAX in new_policy)
+    new_ispec = new_policy[constants.ISPECS_MINMAX][ispec_key]
     for key in diff_ispec:
       self.assertTrue(key in new_ispec)
       self.assertEqual(new_ispec[key], diff_ispec[key])
@@ -1768,11 +1782,26 @@ class TestGetUpdatedIPolicy(unittest.TestCase):
       if not key in diff_policy:
         self.assertTrue(key in new_policy)
         self.assertEqual(new_policy[key], old_policy[key])
-    old_ispec = old_policy[ispec_key]
-    for key in old_ispec:
-      if not key in diff_ispec:
-        self.assertTrue(key in new_ispec)
-        self.assertEqual(new_ispec[key], old_ispec[key])
+
+    if constants.ISPECS_MINMAX in old_policy:
+      old_minmax = old_policy[constants.ISPECS_MINMAX]
+      for key in old_minmax:
+        if key != ispec_key:
+          self.assertTrue(key in new_policy[constants.ISPECS_MINMAX])
+          self.assertEqual(new_policy[constants.ISPECS_MINMAX][key],
+                           old_minmax[key])
+      old_ispec = old_policy[constants.ISPECS_MINMAX][ispec_key]
+      for key in old_ispec:
+        if not key in diff_ispec:
+          self.assertTrue(key in new_ispec)
+          self.assertEqual(new_ispec[key], old_ispec[key])
+
+    if not isgroup:
+      new_std = new_policy[constants.ISPECS_STD]
+      for key in diff_std:
+        self.assertTrue(key in new_std)
+        self.assertEqual(new_std[key], diff_std[key])
+
 
   def _TestSet(self, old_policy, isgroup):
     diff_policy = {
@@ -1816,9 +1845,16 @@ class TestGetUpdatedIPolicy(unittest.TestCase):
     invalid_policy = INVALID_DICT
     self.assertRaises(errors.OpPrereqError, cmdlib._GetUpdatedIPolicy,
                       old_policy, invalid_policy, group_policy=isgroup)
-    for key in constants.IPOLICY_ISPECS:
+    invalid_ispecs = {
+      constants.ISPECS_MINMAX: INVALID_DICT,
+      }
+    self.assertRaises(errors.OpPrereqError, cmdlib._GetUpdatedIPolicy,
+                      old_policy, invalid_ispecs, group_policy=isgroup)
+    for key in constants.ISPECS_MINMAX_KEYS:
       invalid_ispec = {
-        key: INVALID_DICT,
+        constants.ISPECS_MINMAX: {
+          key: INVALID_DICT,
+          },
         }
       self.assertRaises(errors.TypeEnforcementError, cmdlib._GetUpdatedIPolicy,
                         old_policy, invalid_ispec, group_policy=isgroup)
index a90afb2..720b22a 100755 (executable)
@@ -1,7 +1,7 @@
 #!/usr/bin/python
 #
 
-# Copyright (C) 2006, 2007, 2008, 2010, 2012 Google Inc.
+# Copyright (C) 2006, 2007, 2008, 2010, 2012, 2013 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
@@ -414,9 +414,13 @@ class TestInstancePolicy(unittest.TestCase):
 
   def _AssertIPolicyIsFull(self, policy):
     self.assertEqual(frozenset(policy.keys()), constants.IPOLICY_ALL_KEYS)
-    for key in constants.IPOLICY_ISPECS:
-      spec = policy[key]
-      self.assertEqual(frozenset(spec.keys()), constants.ISPECS_PARAMETERS)
+    minmax = policy[constants.ISPECS_MINMAX]
+    self.assertEqual(frozenset(minmax.keys()), constants.ISPECS_MINMAX_KEYS)
+    for key in constants.ISPECS_MINMAX_KEYS:
+      self.assertEqual(frozenset(minmax[key].keys()),
+                       constants.ISPECS_PARAMETERS)
+    self.assertEqual(frozenset(policy[constants.ISPECS_STD].keys()),
+                     constants.ISPECS_PARAMETERS)
 
   def testDefaultIPolicy(self):
     objects.InstancePolicy.CheckParameterSyntax(constants.IPOLICY_DEFAULTS,
@@ -426,28 +430,29 @@ class TestInstancePolicy(unittest.TestCase):
   def testCheckISpecSyntax(self):
     par = "my_parameter"
     for check_std in [True, False]:
-      if check_std:
-        allkeys = constants.IPOLICY_ISPECS
-      else:
-        allkeys = constants.IPOLICY_ISPECS - frozenset([constants.ISPECS_STD])
       # Only one policy limit
-      for key in allkeys:
-        policy = dict((k, {}) for k in allkeys)
-        policy[key][par] = 11
-        objects.InstancePolicy.CheckISpecSyntax(policy, par, check_std)
+      for key in constants.ISPECS_MINMAX_KEYS:
+        minmax = dict((k, {}) for k in constants.ISPECS_MINMAX_KEYS)
+        minmax[key][par] = 11
+        objects.InstancePolicy.CheckISpecSyntax(minmax, {}, par, check_std)
+      if check_std:
+        minmax = dict((k, {}) for k in constants.ISPECS_MINMAX_KEYS)
+        stdspec = {par: 11}
+        objects.InstancePolicy.CheckISpecSyntax(minmax, stdspec, par, check_std)
+
       # Min and max only
       good_values = [(11, 11), (11, 40), (0, 0)]
       for (mn, mx) in good_values:
-        policy = dict((k, {}) for k in allkeys)
-        policy[constants.ISPECS_MIN][par] = mn
-        policy[constants.ISPECS_MAX][par] = mx
-        objects.InstancePolicy.CheckISpecSyntax(policy, par, check_std)
-      policy = dict((k, {}) for k in allkeys)
-      policy[constants.ISPECS_MIN][par] = 11
-      policy[constants.ISPECS_MAX][par] = 5
+        minmax = dict((k, {}) for k in constants.ISPECS_MINMAX_KEYS)
+        minmax[constants.ISPECS_MIN][par] = mn
+        minmax[constants.ISPECS_MAX][par] = mx
+        objects.InstancePolicy.CheckISpecSyntax(minmax, {}, par, check_std)
+      minmax = dict((k, {}) for k in constants.ISPECS_MINMAX_KEYS)
+      minmax[constants.ISPECS_MIN][par] = 11
+      minmax[constants.ISPECS_MAX][par] = 5
       self.assertRaises(errors.ConfigurationError,
                         objects.InstancePolicy.CheckISpecSyntax,
-                        policy, par, check_std)
+                        minmax, {}, par, check_std)
     # Min, std, max
     good_values = [
       (11, 11, 11),
@@ -455,12 +460,12 @@ class TestInstancePolicy(unittest.TestCase):
       (11, 40, 40),
       ]
     for (mn, st, mx) in good_values:
-      policy = {
+      minmax = {
         constants.ISPECS_MIN: {par: mn},
-        constants.ISPECS_STD: {par: st},
         constants.ISPECS_MAX: {par: mx},
         }
-      objects.InstancePolicy.CheckISpecSyntax(policy, par, True)
+      stdspec = {par: st}
+      objects.InstancePolicy.CheckISpecSyntax(minmax, stdspec, par, True)
     bad_values = [
       (11, 11,  5),
       (40, 11, 11),
@@ -470,14 +475,14 @@ class TestInstancePolicy(unittest.TestCase):
       (40, 40, 11),
       ]
     for (mn, st, mx) in bad_values:
-      policy = {
+      minmax = {
         constants.ISPECS_MIN: {par: mn},
-        constants.ISPECS_STD: {par: st},
         constants.ISPECS_MAX: {par: mx},
         }
+      stdspec = {par: st}
       self.assertRaises(errors.ConfigurationError,
                         objects.InstancePolicy.CheckISpecSyntax,
-                        policy, par, True)
+                        minmax, stdspec, par, True)
 
   def testCheckDiskTemplates(self):
     invalid = "this_is_not_a_good_template"
@@ -499,18 +504,14 @@ class TestInstancePolicy(unittest.TestCase):
   def testCheckParameterSyntax(self):
     invalid = "this_key_shouldnt_be_here"
     for check_std in [True, False]:
-      self.assertRaises(KeyError,
-                        objects.InstancePolicy.CheckParameterSyntax,
-                        {}, check_std)
-      policy = objects.MakeEmptyIPolicy()
-      policy[invalid] = None
+      objects.InstancePolicy.CheckParameterSyntax({}, check_std)
+      policy = {invalid: None}
       self.assertRaises(errors.ConfigurationError,
                         objects.InstancePolicy.CheckParameterSyntax,
                         policy, check_std)
       for par in constants.IPOLICY_PARAMETERS:
-        policy = objects.MakeEmptyIPolicy()
         for val in ("blah", None, {}, [42]):
-          policy[par] = val
+          policy = {par: val}
           self.assertRaises(errors.ConfigurationError,
                             objects.InstancePolicy.CheckParameterSyntax,
                             policy, check_std)
@@ -530,7 +531,12 @@ class TestInstancePolicy(unittest.TestCase):
   def _AssertIPolicyMerged(self, default_pol, diff_pol, merged_pol):
     for (key, value) in merged_pol.items():
       if key in diff_pol:
-        if key in constants.IPOLICY_ISPECS:
+        if key == constants.ISPECS_MINMAX:
+          self.assertEqual(frozenset(value), constants.ISPECS_MINMAX_KEYS)
+          for k in constants.ISPECS_MINMAX_KEYS:
+            self._AssertISpecsMerged(default_pol[key][k], diff_pol[key][k],
+                                     value[k])
+        elif key == constants.ISPECS_STD:
           self._AssertISpecsMerged(default_pol[key], diff_pol[key], value)
         else:
           self.assertEqual(value, diff_pol[key])
@@ -550,17 +556,28 @@ class TestInstancePolicy(unittest.TestCase):
       self._AssertIPolicyMerged(constants.IPOLICY_DEFAULTS, diff_pol, policy)
 
   def testFillIPolicySpecs(self):
-    partial_policies = [
-      {constants.ISPECS_MIN: {constants.ISPEC_MEM_SIZE: 32},
-       constants.ISPECS_MAX: {constants.ISPEC_CPU_COUNT: 1024}},
-      {constants.ISPECS_STD: {constants.ISPEC_DISK_SIZE: 2048},
-       constants.ISPECS_MAX: {
-          constants.ISPEC_DISK_COUNT: constants.MAX_DISKS - 1,
-          constants.ISPEC_NIC_COUNT: constants.MAX_NICS - 1,
-          }},
-      {constants.ISPECS_STD: {constants.ISPEC_SPINDLE_USE: 3}},
+    partial_ipolicies = [
+      {
+        constants.ISPECS_MINMAX: {
+          constants.ISPECS_MIN: {constants.ISPEC_MEM_SIZE: 32},
+          constants.ISPECS_MAX: {constants.ISPEC_CPU_COUNT: 1024}
+          },
+        },
+      {
+        constants.ISPECS_MINMAX: {
+          constants.ISPECS_MAX: {
+            constants.ISPEC_DISK_COUNT: constants.MAX_DISKS - 1,
+            constants.ISPEC_NIC_COUNT: constants.MAX_NICS - 1,
+            },
+          constants.ISPECS_MIN: {},
+          },
+          constants.ISPECS_STD: {constants.ISPEC_DISK_SIZE: 2048},
+        },
+      {
+        constants.ISPECS_STD: {constants.ISPEC_SPINDLE_USE: 3},
+        },
       ]
-    for diff_pol in partial_policies:
+    for diff_pol in partial_ipolicies:
       policy = objects.FillIPolicy(constants.IPOLICY_DEFAULTS, diff_pol)
       objects.InstancePolicy.CheckParameterSyntax(policy, True)
       self._AssertIPolicyIsFull(policy)