Limit specs in ipolicy command lines are always complete
authorBernardo Dal Seno <bdalseno@google.com>
Mon, 8 Apr 2013 18:40:40 +0000 (20:40 +0200)
committerBernardo Dal Seno <bdalseno@google.com>
Wed, 24 Apr 2013 10:02:45 +0000 (12:02 +0200)
Command line options are brought in line with the specs change of previous
patch. Old options are still allowed in gnt-cluster init, where the
semantic will remain non-ambiguous even after introducing multiple specs.

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

lib/cli.py
lib/client/gnt_cluster.py
lib/client/gnt_group.py
man/gnt-cluster.rst
man/gnt-group.rst
test/py/ganeti.cli_unittest.py

index 6152ed7..7615c73 100644 (file)
@@ -189,6 +189,7 @@ __all__ = [
   "SPECS_DISK_SIZE_OPT",
   "SPECS_MEM_SIZE_OPT",
   "SPECS_NIC_COUNT_OPT",
+  "SPLIT_ISPECS_OPTS",
   "IPOLICY_STD_SPECS_OPT",
   "IPOLICY_DISK_TEMPLATES",
   "IPOLICY_VCPU_RATIO",
@@ -1657,15 +1658,19 @@ COMMON_CREATE_OPTS = [
 
 # common instance policy options
 INSTANCE_POLICY_OPTS = [
+  IPOLICY_BOUNDS_SPECS_OPT,
+  IPOLICY_DISK_TEMPLATES,
+  IPOLICY_VCPU_RATIO,
+  IPOLICY_SPINDLE_RATIO,
+  ]
+
+# instance policy split specs options
+SPLIT_ISPECS_OPTS = [
   SPECS_CPU_COUNT_OPT,
   SPECS_DISK_COUNT_OPT,
   SPECS_DISK_SIZE_OPT,
   SPECS_MEM_SIZE_OPT,
   SPECS_NIC_COUNT_OPT,
-  IPOLICY_BOUNDS_SPECS_OPT,
-  IPOLICY_DISK_TEMPLATES,
-  IPOLICY_VCPU_RATIO,
-  IPOLICY_SPINDLE_RATIO,
   ]
 
 
@@ -3846,7 +3851,7 @@ def _MaybeParseUnit(elements):
 
 def _InitISpecsFromSplitOpts(ipolicy, ispecs_mem_size, ispecs_cpu_count,
                              ispecs_disk_count, ispecs_disk_size,
-                             ispecs_nic_count, group_ipolicy, allowed_values):
+                             ispecs_nic_count, group_ipolicy, fill_all):
   try:
     if ispecs_mem_size:
       ispecs_mem_size = _MaybeParseUnit(ispecs_mem_size)
@@ -3874,7 +3879,7 @@ def _InitISpecsFromSplitOpts(ipolicy, ispecs_mem_size, ispecs_cpu_count,
     forced_type = TISPECS_CLUSTER_TYPES
   for specs in ispecs_transposed.values():
     assert type(specs) is dict
-    utils.ForceDictType(specs, forced_type, allowed_values=allowed_values)
+    utils.ForceDictType(specs, forced_type)
 
   # then transpose
   ispecs = {
@@ -3887,15 +3892,25 @@ def _InitISpecsFromSplitOpts(ipolicy, ispecs_mem_size, ispecs_cpu_count,
     for key, val in specs.items(): # {min: .. ,max: .., std: ..}
       assert key in ispecs
       ispecs[key][name] = val
+  ipolicy[constants.ISPECS_MINMAX] = {}
   for key in constants.ISPECS_MINMAX_KEYS:
-    ipolicy[constants.ISPECS_MINMAX][key] = ispecs[key]
-  ipolicy[constants.ISPECS_STD] = ispecs[constants.ISPECS_STD]
+    if fill_all:
+      ipolicy[constants.ISPECS_MINMAX][key] = \
+        objects.FillDict(constants.ISPECS_MINMAX_DEFAULTS[key], ispecs[key])
+    else:
+      ipolicy[constants.ISPECS_MINMAX][key] = ispecs[key]
+  if fill_all:
+    ipolicy[constants.ISPECS_STD] = \
+        objects.FillDict(constants.IPOLICY_DEFAULTS[constants.ISPECS_STD],
+                         ispecs[constants.ISPECS_STD])
+  else:
+    ipolicy[constants.ISPECS_STD] = ispecs[constants.ISPECS_STD]
 
 
 def _ParseSpecUnit(spec, keyname):
   ret = spec.copy()
   for k in [constants.ISPEC_DISK_SIZE, constants.ISPEC_MEM_SIZE]:
-    if k in ret and ret[k] != constants.VALUE_DEFAULT:
+    if k in ret:
       try:
         ret[k] = utils.ParseUnit(ret[k])
       except (TypeError, ValueError, errors.UnitParseError), err:
@@ -3905,27 +3920,43 @@ def _ParseSpecUnit(spec, keyname):
   return ret
 
 
-def _ParseISpec(spec, keyname, allowed_values):
+def _ParseISpec(spec, keyname, required):
   ret = _ParseSpecUnit(spec, keyname)
-  utils.ForceDictType(ret, constants.ISPECS_PARAMETER_TYPES,
-                      allowed_values=allowed_values)
+  utils.ForceDictType(ret, constants.ISPECS_PARAMETER_TYPES)
+  missing = constants.ISPECS_PARAMETERS - frozenset(ret.keys())
+  if required and missing:
+    raise errors.OpPrereqError("Missing parameters in ipolicy spec %s: %s" %
+                               (keyname, utils.CommaJoin(missing)),
+                               errors.ECODE_INVAL)
+  return ret
+
+
+def _GetISpecsInAllowedValues(minmax_ispecs, allowed_values):
+  ret = None
+  if minmax_ispecs and allowed_values and len(minmax_ispecs) == 1:
+    for (key, spec) in minmax_ispecs.items():
+      # This loop is executed exactly once
+      if key in allowed_values and not spec:
+        ret = key
   return ret
 
 
 def _InitISpecsFromFullOpts(ipolicy_out, minmax_ispecs, std_ispecs,
                             group_ipolicy, allowed_values):
-  if minmax_ispecs is not None:
+  found_allowed = _GetISpecsInAllowedValues(minmax_ispecs, allowed_values)
+  if found_allowed is not None:
+    ipolicy_out[constants.ISPECS_MINMAX] = found_allowed
+  elif minmax_ispecs is not None:
     minmax_out = {}
     for (key, spec) in minmax_ispecs.items():
       if key not in constants.ISPECS_MINMAX_KEYS:
         msg = "Invalid key in bounds instance specifications: %s" % key
         raise errors.OpPrereqError(msg, errors.ECODE_INVAL)
-      minmax_out[key] = _ParseISpec(spec, key, allowed_values)
+      minmax_out[key] = _ParseISpec(spec, key, True)
     ipolicy_out[constants.ISPECS_MINMAX] = minmax_out
   if std_ispecs is not None:
     assert not group_ipolicy # This is not an option for gnt-group
-    ipolicy_out[constants.ISPECS_STD] = _ParseISpec(std_ispecs, "std",
-                                                    allowed_values)
+    ipolicy_out[constants.ISPECS_STD] = _ParseISpec(std_ispecs, "std", False)
 
 
 def CreateIPolicyFromOpts(ispecs_mem_size=None,
@@ -3946,21 +3977,23 @@ def CreateIPolicyFromOpts(ispecs_mem_size=None,
   @param fill_all: whether for cluster policies we should ensure that
     all values are filled
 
-
   """
-  if ((ispecs_mem_size or ispecs_cpu_count or ispecs_disk_count or
-       ispecs_disk_size or ispecs_nic_count) and
-      (minmax_ispecs is not None or std_ispecs is not None)):
+  assert not (fill_all and allowed_values)
+
+  split_specs = (ispecs_mem_size or ispecs_cpu_count or ispecs_disk_count or
+                 ispecs_disk_size or ispecs_nic_count)
+  if (split_specs and (minmax_ispecs is not None or std_ispecs is not None)):
     raise errors.OpPrereqError("A --specs-xxx option cannot be specified"
                                " together with any --ipolicy-xxx-specs option",
                                errors.ECODE_INVAL)
 
   ipolicy_out = objects.MakeEmptyIPolicy()
-  if minmax_ispecs is None and std_ispecs is None:
+  if split_specs:
+    assert fill_all
     _InitISpecsFromSplitOpts(ipolicy_out, ispecs_mem_size, ispecs_cpu_count,
                              ispecs_disk_count, ispecs_disk_size,
-                             ispecs_nic_count, group_ipolicy, allowed_values)
-  else:
+                             ispecs_nic_count, group_ipolicy, fill_all)
+  elif (minmax_ispecs is not None or std_ispecs is not None):
     _InitISpecsFromFullOpts(ipolicy_out, minmax_ispecs, std_ispecs,
                             group_ipolicy, allowed_values)
 
index 750d655..c5ee9a7 100644 (file)
@@ -967,11 +967,6 @@ def SetClusterParams(opts, args):
           opts.hv_state or
           opts.enabled_disk_templates or
           opts.disk_state or
-          opts.ispecs_mem_size or
-          opts.ispecs_cpu_count or
-          opts.ispecs_disk_count or
-          opts.ispecs_disk_size or
-          opts.ispecs_nic_count or
           opts.ipolicy_bounds_specs is not None or
           opts.ipolicy_std_specs is not None or
           opts.ipolicy_disk_templates is not None or
@@ -1025,11 +1020,6 @@ def SetClusterParams(opts, args):
     utils.ForceDictType(ndparams, constants.NDS_PARAMETER_TYPES)
 
   ipolicy = 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,
     minmax_ispecs=opts.ipolicy_bounds_specs,
     std_ispecs=opts.ipolicy_std_specs,
     ipolicy_disk_templates=opts.ipolicy_disk_templates,
@@ -1523,7 +1513,7 @@ commands = {
      DEFAULT_IALLOCATOR_OPT, PRIMARY_IP_VERSION_OPT, PREALLOC_WIPE_DISKS_OPT,
      NODE_PARAMS_OPT, GLOBAL_SHARED_FILEDIR_OPT, USE_EXTERNAL_MIP_SCRIPT,
      DISK_PARAMS_OPT, HV_STATE_OPT, DISK_STATE_OPT, ENABLED_DISK_TEMPLATES_OPT,
-     IPOLICY_STD_SPECS_OPT] + INSTANCE_POLICY_OPTS,
+     IPOLICY_STD_SPECS_OPT] + INSTANCE_POLICY_OPTS + SPLIT_ISPECS_OPTS,
     "[opts...] <cluster_name>", "Initialises a new cluster configuration"),
   "destroy": (
     DestroyCluster, ARGS_NONE, [YES_DOIT_OPT],
index 32e0e3a..5bef440 100644 (file)
@@ -50,11 +50,6 @@ def AddGroup(opts, args):
 
   """
   ipolicy = 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,
     minmax_ispecs=opts.ipolicy_bounds_specs,
     ipolicy_vcpu_ratio=opts.ipolicy_vcpu_ratio,
     ipolicy_spindle_ratio=opts.ipolicy_spindle_ratio,
@@ -162,9 +157,7 @@ def SetGroupParams(opts, args):
 
   """
   allmods = [opts.ndparams, opts.alloc_policy, opts.diskparams, opts.hv_state,
-             opts.disk_state, opts.ispecs_mem_size, opts.ispecs_cpu_count,
-             opts.ispecs_disk_count, opts.ispecs_disk_size,
-             opts.ispecs_nic_count, opts.ipolicy_bounds_specs,
+             opts.disk_state, opts.ipolicy_bounds_specs,
              opts.ipolicy_vcpu_ratio, opts.ipolicy_spindle_ratio,
              opts.diskparams]
   if allmods.count(None) == len(allmods):
@@ -180,26 +173,8 @@ def SetGroupParams(opts, args):
 
   diskparams = dict(opts.diskparams)
 
-  # set the default values
-  to_ipolicy = [
-    opts.ispecs_mem_size,
-    opts.ispecs_cpu_count,
-    opts.ispecs_disk_count,
-    opts.ispecs_disk_size,
-    opts.ispecs_nic_count,
-    ]
-  for ispec in to_ipolicy:
-    for param in ispec:
-      if isinstance(ispec[param], basestring):
-        if ispec[param].lower() == "default":
-          ispec[param] = constants.VALUE_DEFAULT
   # create ipolicy object
   ipolicy = 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,
     minmax_ispecs=opts.ipolicy_bounds_specs,
     ipolicy_disk_templates=opts.ipolicy_disk_templates,
     ipolicy_vcpu_ratio=opts.ipolicy_vcpu_ratio,
index 1e837ae..b04d1db 100644 (file)
@@ -536,6 +536,10 @@ comma-separated list of disk templates.
 - ``--ipolicy-spindle-ratio`` limits the instances-spindles ratio
 - ``--ipolicy-vcpu-ratio`` limits the vcpu-cpu ratio
 
+All the instance policy elements can be overridden at group level. Group
+level overrides can be removed by specifying ``default`` as the value of
+an item.
+
 For details about how to use ``--hypervisor-state`` and ``--disk-state``
 have a look at **ganeti**\(7).
 
@@ -607,11 +611,6 @@ MODIFY
 | [\--use-external-mip-script {yes \| no}]
 | [\--hypervisor-state *hvstate*]
 | [\--disk-state *diskstate*]
-| [\--specs-cpu-count *spec-param*=*value* [,*spec-param*=*value*...]]
-| [\--specs-disk-count *spec-param*=*value* [,*spec-param*=*value*...]]
-| [\--specs-disk-size *spec-param*=*value* [,*spec-param*=*value*...]]
-| [\--specs-mem-size *spec-param*=*value* [,*spec-param*=*value*...]]
-| [\--specs-nic-count *spec-param*=*value* [,*spec-param*=*value*...]]
 | [\--ipolicy-std-specs *spec*=*value* [,*spec*=*value*...]]
 | [\--ipolicy-bounds-specs *bounds_ispecs*]
 | [\--ipolicy-disk-templates *template* [,*template*...]]
@@ -654,8 +653,7 @@ The ``-I (--default-iallocator)`` is described in the **init**
 command. To clear the default iallocator, just pass an empty string
 ('').
 
-The ``--specs-...`` and ``--ipolicy-...`` options are described in the
-**init** command.
+The ``--ipolicy-...`` options are described in the **init** command.
 
 See **ganeti**\(7) for a description of ``--submit`` and other common
 options.
index 6786e69..12b4c84 100644 (file)
@@ -27,11 +27,6 @@ ADD
 | [\--node-parameters=*NDPARAMS*]
 | [\--alloc-policy=*POLICY*]
 | [{-D|\--disk-parameters} *disk-template*:*disk-param*=*value*[,*disk-param*=*value*...]]
-| [\--specs-cpu-count *spec-param*=*value* [,*spec-param*=*value*...]]
-| [\--specs-disk-count *spec-param*=*value* [,*spec-param*=*value*...]]
-| [\--specs-disk-size *spec-param*=*value* [,*spec-param*=*value*...]]
-| [\--specs-mem-size *spec-param*=*value* [,*spec-param*=*value*...]]
-| [\--specs-nic-count *spec-param*=*value* [,*spec-param*=*value*...]]
 | [\--ipolicy-bounds-specs *bound_ispecs*]
 | [\--ipolicy-disk-templates *template* [,*template*...]]
 | [\--ipolicy-spindle-ratio *ratio*]
@@ -70,9 +65,8 @@ parameters for the node group; please see the section about
 **gnt-cluster add** in **gnt-cluster**\(8) for more information about
 disk parameters
 
-The ``--specs-...`` and ``--ipolicy-...`` options specify instance
-policies on the node group, and are documented in the
-**gnt-cluster**\(8) man page.
+The ``--ipolicy-...`` options specify instance policies on the node
+group, and are documented in the **gnt-cluster**\(8) man page.
 
 See **ganeti**\(7) for a description of ``--submit`` and other common
 options.
@@ -105,11 +99,6 @@ MODIFY
 | [\--hypervisor-state *hvstate*]
 | [{-D|\--disk-parameters} *disk-template*:*disk-param*=*value*[,*disk-param*=*value*...]]
 | [\--disk-state *diskstate*]
-| [\--specs-cpu-count *spec-param*=*value* [,*spec-param*=*value*...]]
-| [\--specs-disk-count *spec-param*=*value* [,*spec-param*=*value*...]]
-| [\--specs-disk-size *spec-param*=*value* [,*spec-param*=*value*...]]
-| [\--specs-mem-size *spec-param*=*value* [,*spec-param*=*value*...]]
-| [\--specs-nic-count *spec-param*=*value* [,*spec-param*=*value*...]]
 | [\--ipolicy-bounds-specs *bound_ispecs*]
 | [\--ipolicy-disk-templates *template* [,*template*...]]
 | [\--ipolicy-spindle-ratio *ratio*]
@@ -126,9 +115,8 @@ The ``--node-parameters``, ``--alloc-policy``, ``-D
 (--disk-parameters)`` options are documented in the **add** command
 above.
 
-The ``--specs-...`` and ``--ipolicy-...`` options specify instance
-policies on the node group, and are documented in the
-**gnt-cluster**\(8) man page.
+The ``--ipolicy-...`` options specify instance policies on the node
+group, and are documented in the **gnt-cluster**\(8) man page.
 
 See **ganeti**\(7) for a description of ``--submit`` and other common
 options.
index a90dd4a..6ad63fc 100755 (executable)
 
 """Script for unittesting the cli module"""
 
-import unittest
+import copy
+import testutils
 import time
+import unittest
 from cStringIO import StringIO
 
-import ganeti
-import testutils
-
 from ganeti import constants
 from ganeti import cli
 from ganeti import errors
@@ -1181,6 +1180,10 @@ class TestSerializeGenericInfo(unittest.TestCase):
 
 class TestCreateIPolicyFromOpts(unittest.TestCase):
   """Test case for cli.CreateIPolicyFromOpts."""
+  def setUp(self):
+    # Policies are big, and we want to see the difference in case of an error
+    self.maxDiff = None
+
   def _RecursiveCheckMergedDicts(self, default_pol, diff_pol, merged_pol):
     self.assertTrue(type(default_pol) is dict)
     self.assertTrue(type(diff_pol) is dict)
@@ -1197,13 +1200,19 @@ class TestCreateIPolicyFromOpts(unittest.TestCase):
         self.assertEqual(val, default_pol[key])
 
   def testClusterPolicy(self):
-    exp_pol0 = {
-      constants.ISPECS_MINMAX: {
-        constants.ISPECS_MIN: {},
-        constants.ISPECS_MAX: {},
-        },
-      constants.ISPECS_STD: {},
-      }
+    pol0 = cli.CreateIPolicyFromOpts(
+      ispecs_mem_size={},
+      ispecs_cpu_count={},
+      ispecs_disk_count={},
+      ispecs_disk_size={},
+      ispecs_nic_count={},
+      ipolicy_disk_templates=None,
+      ipolicy_vcpu_ratio=None,
+      ipolicy_spindle_ratio=None,
+      fill_all=True
+      )
+    self.assertEqual(pol0, constants.IPOLICY_DEFAULTS)
+
     exp_pol1 = {
       constants.ISPECS_MINMAX: {
         constants.ISPECS_MIN: {
@@ -1221,6 +1230,20 @@ class TestCreateIPolicyFromOpts(unittest.TestCase):
         },
       constants.IPOLICY_VCPU_RATIO: 3.1,
       }
+    pol1 = cli.CreateIPolicyFromOpts(
+      ispecs_mem_size={"max": "12g"},
+      ispecs_cpu_count={"min": 2, "std": 2},
+      ispecs_disk_count={"min": 1, "max": 2, "std": 2},
+      ispecs_disk_size={},
+      ispecs_nic_count={},
+      ipolicy_disk_templates=None,
+      ipolicy_vcpu_ratio=3.1,
+      ipolicy_spindle_ratio=None,
+      fill_all=True
+      )
+    self._RecursiveCheckMergedDicts(constants.IPOLICY_DEFAULTS,
+                                    exp_pol1, pol1)
+
     exp_pol2 = {
       constants.ISPECS_MINMAX: {
         constants.ISPECS_MIN: {
@@ -1238,94 +1261,154 @@ class TestCreateIPolicyFromOpts(unittest.TestCase):
       constants.IPOLICY_SPINDLE_RATIO: 1.3,
       constants.IPOLICY_DTS: ["templates"],
       }
-    for fillall in [False, True]:
-      pol0 = cli.CreateIPolicyFromOpts(
-        ispecs_mem_size={},
-        ispecs_cpu_count={},
-        ispecs_disk_count={},
-        ispecs_disk_size={},
-        ispecs_nic_count={},
+    pol2 = cli.CreateIPolicyFromOpts(
+      ispecs_mem_size={},
+      ispecs_cpu_count={"std": 2},
+      ispecs_disk_count={},
+      ispecs_disk_size={"min": "0.5g"},
+      ispecs_nic_count={"min": 2, "max": 3, "std": 3},
+      ipolicy_disk_templates=["templates"],
+      ipolicy_vcpu_ratio=None,
+      ipolicy_spindle_ratio=1.3,
+      fill_all=True
+      )
+    self._RecursiveCheckMergedDicts(constants.IPOLICY_DEFAULTS,
+                                      exp_pol2, pol2)
+
+    for fill_all in [False, True]:
+      exp_pol3 = {
+        constants.ISPECS_STD: {
+          constants.ISPEC_CPU_COUNT: 2,
+          constants.ISPEC_NIC_COUNT: 3,
+          },
+        }
+      pol3 = cli.CreateIPolicyFromOpts(
+        std_ispecs={
+          constants.ISPEC_CPU_COUNT: "2",
+          constants.ISPEC_NIC_COUNT: "3",
+          },
         ipolicy_disk_templates=None,
         ipolicy_vcpu_ratio=None,
         ipolicy_spindle_ratio=None,
-        fill_all=fillall
-        )
-      if fillall:
-        self.assertEqual(pol0, constants.IPOLICY_DEFAULTS)
-      else:
-        self.assertEqual(pol0, exp_pol0)
-      pol1 = cli.CreateIPolicyFromOpts(
-        ispecs_mem_size={"max": "12g"},
-        ispecs_cpu_count={"min": 2, "std": 2},
-        ispecs_disk_count={"min": 1, "max": 2, "std": 2},
-        ispecs_disk_size={},
-        ispecs_nic_count={},
-        ipolicy_disk_templates=None,
-        ipolicy_vcpu_ratio=3.1,
-        ipolicy_spindle_ratio=None,
-        fill_all=fillall
+        fill_all=fill_all
         )
-      if fillall:
+      if fill_all:
         self._RecursiveCheckMergedDicts(constants.IPOLICY_DEFAULTS,
-                                        exp_pol1, pol1)
+                                        exp_pol3, pol3)
       else:
-        self.assertEqual(pol1, exp_pol1)
-      pol2 = cli.CreateIPolicyFromOpts(
-        ispecs_mem_size={},
-        ispecs_cpu_count={"std": 2},
-        ispecs_disk_count={},
-        ispecs_disk_size={"min": "0.5g"},
-        ispecs_nic_count={"min": 2, "max": 3, "std": 3},
-        ipolicy_disk_templates=["templates"],
-        ipolicy_vcpu_ratio=None,
-        ipolicy_spindle_ratio=1.3,
-        fill_all=fillall
-        )
-      if fillall:
-        self._RecursiveCheckMergedDicts(constants.IPOLICY_DEFAULTS,
-                                        exp_pol2, pol2)
+        self.assertEqual(pol3, exp_pol3)
+
+  def testPartialPolicy(self):
+    exp_pol0 = objects.MakeEmptyIPolicy()
+    pol0 = cli.CreateIPolicyFromOpts(
+      minmax_ispecs=None,
+      std_ispecs=None,
+      ipolicy_disk_templates=None,
+      ipolicy_vcpu_ratio=None,
+      ipolicy_spindle_ratio=None,
+      fill_all=False
+      )
+    self.assertEqual(pol0, exp_pol0)
+
+    exp_pol1 = {
+      constants.IPOLICY_VCPU_RATIO: 3.1,
+      }
+    pol1 = cli.CreateIPolicyFromOpts(
+      minmax_ispecs=None,
+      std_ispecs=None,
+      ipolicy_disk_templates=None,
+      ipolicy_vcpu_ratio=3.1,
+      ipolicy_spindle_ratio=None,
+      fill_all=False
+      )
+    self.assertEqual(pol1, exp_pol1)
+
+    exp_pol2 = {
+      constants.IPOLICY_SPINDLE_RATIO: 1.3,
+      constants.IPOLICY_DTS: ["templates"],
+      }
+    pol2 = cli.CreateIPolicyFromOpts(
+      minmax_ispecs=None,
+      std_ispecs=None,
+      ipolicy_disk_templates=["templates"],
+      ipolicy_vcpu_ratio=None,
+      ipolicy_spindle_ratio=1.3,
+      fill_all=False
+      )
+    self.assertEqual(pol2, exp_pol2)
+
+  def _TestInvalidISpecs(self, minmax_ispecs, std_ispecs, fail=True):
+    for fill_all in [False, True]:
+      if fail:
+        self.assertRaises((errors.OpPrereqError,
+                           errors.UnitParseError,
+                           errors.TypeEnforcementError),
+                          cli.CreateIPolicyFromOpts,
+                          minmax_ispecs=minmax_ispecs,
+                          std_ispecs=std_ispecs,
+                          fill_all=fill_all)
       else:
-        self.assertEqual(pol2, exp_pol2)
+        cli.CreateIPolicyFromOpts(minmax_ispecs=minmax_ispecs,
+                                  std_ispecs=std_ispecs,
+                                  fill_all=fill_all)
 
   def testInvalidPolicies(self):
-    self.assertRaises(errors.TypeEnforcementError, cli.CreateIPolicyFromOpts,
-                      ispecs_mem_size={}, ispecs_cpu_count={},
-                      ispecs_disk_count={}, ispecs_disk_size={"std": 1},
-                      ispecs_nic_count={}, ipolicy_disk_templates=None,
-                      ipolicy_vcpu_ratio=None, ipolicy_spindle_ratio=None,
-                      group_ipolicy=True)
+    self.assertRaises(AssertionError, cli.CreateIPolicyFromOpts,
+                      std_ispecs={constants.ISPEC_MEM_SIZE: 1024},
+                      ipolicy_disk_templates=None, ipolicy_vcpu_ratio=None,
+                      ipolicy_spindle_ratio=None, group_ipolicy=True)
     self.assertRaises(errors.OpPrereqError, cli.CreateIPolicyFromOpts,
                       ispecs_mem_size={"wrong": "x"}, ispecs_cpu_count={},
                       ispecs_disk_count={}, ispecs_disk_size={},
                       ispecs_nic_count={}, ipolicy_disk_templates=None,
-                      ipolicy_vcpu_ratio=None, ipolicy_spindle_ratio=None)
+                      ipolicy_vcpu_ratio=None, ipolicy_spindle_ratio=None,
+                      fill_all=True)
     self.assertRaises(errors.TypeEnforcementError, cli.CreateIPolicyFromOpts,
                       ispecs_mem_size={}, ispecs_cpu_count={"min": "default"},
                       ispecs_disk_count={}, ispecs_disk_size={},
                       ispecs_nic_count={}, ipolicy_disk_templates=None,
-                      ipolicy_vcpu_ratio=None, ipolicy_spindle_ratio=None)
+                      ipolicy_vcpu_ratio=None, ipolicy_spindle_ratio=None,
+                      fill_all=True)
+
+    good_mmspecs = constants.ISPECS_MINMAX_DEFAULTS
+    self._TestInvalidISpecs(good_mmspecs, None, fail=False)
+    broken_mmspecs = copy.deepcopy(good_mmspecs)
+    for key in constants.ISPECS_MINMAX_KEYS:
+      for par in constants.ISPECS_PARAMETERS:
+        old = broken_mmspecs[key][par]
+        del broken_mmspecs[key][par]
+        self._TestInvalidISpecs(broken_mmspecs, None)
+        broken_mmspecs[key][par] = "invalid"
+        self._TestInvalidISpecs(broken_mmspecs, None)
+        broken_mmspecs[key][par] = old
+      broken_mmspecs[key]["invalid_key"] = None
+      self._TestInvalidISpecs(broken_mmspecs, None)
+      del broken_mmspecs[key]["invalid_key"]
+    assert broken_mmspecs == good_mmspecs
+
+    good_stdspecs = constants.IPOLICY_DEFAULTS[constants.ISPECS_STD]
+    self._TestInvalidISpecs(None, good_stdspecs, fail=False)
+    broken_stdspecs = copy.deepcopy(good_stdspecs)
+    for par in constants.ISPECS_PARAMETERS:
+      old = broken_stdspecs[par]
+      broken_stdspecs[par] = "invalid"
+      self._TestInvalidISpecs(None, broken_stdspecs)
+      broken_stdspecs[par] = old
+    broken_stdspecs["invalid_key"] = None
+    self._TestInvalidISpecs(None, broken_stdspecs)
+    del broken_stdspecs["invalid_key"]
+    assert broken_stdspecs == good_stdspecs
 
   def testAllowedValues(self):
     allowedv = "blah"
     exp_pol1 = {
-      constants.ISPECS_MINMAX: {
-        constants.ISPECS_MIN: {
-          constants.ISPEC_CPU_COUNT: allowedv,
-          },
-        constants.ISPECS_MAX: {
-          },
-        },
-      constants.ISPECS_STD: {
-        },
+      constants.ISPECS_MINMAX: allowedv,
       constants.IPOLICY_DTS: allowedv,
       constants.IPOLICY_VCPU_RATIO: allowedv,
       constants.IPOLICY_SPINDLE_RATIO: allowedv,
       }
-    pol1 = cli.CreateIPolicyFromOpts(ispecs_mem_size={},
-                                     ispecs_cpu_count={"min": allowedv},
-                                     ispecs_disk_count={},
-                                     ispecs_disk_size={},
-                                     ispecs_nic_count={},
+    pol1 = cli.CreateIPolicyFromOpts(minmax_ispecs={allowedv: {}},
+                                     std_ispecs=None,
                                      ipolicy_disk_templates=allowedv,
                                      ipolicy_vcpu_ratio=allowedv,
                                      ipolicy_spindle_ratio=allowedv,
@@ -1357,16 +1440,11 @@ class TestCreateIPolicyFromOpts(unittest.TestCase):
       exp_ipol[constants.ISPECS_MINMAX] = exp_minmax
     else:
       minmax_ispecs = None
-      if constants.ISPECS_MINMAX not in exp_ipol:
-        empty_minmax = dict((k, {}) for k in constants.ISPECS_MINMAX_KEYS)
-        exp_ipol[constants.ISPECS_MINMAX] = empty_minmax
     if exp_std is not None:
       std_ispecs = self._ConvertSpecToStrings(exp_std)
       exp_ipol[constants.ISPECS_STD] = exp_std
     else:
       std_ispecs = None
-      if constants.ISPECS_STD not in exp_ipol:
-        exp_ipol[constants.ISPECS_STD] = {}
 
     self._CheckNewStyleSpecsCall(exp_ipol, minmax_ispecs, std_ispecs,
                                  group_ipolicy, fill_all)