From d2d3935a7127a06c791eaaf316ca43a7f7dfea62 Mon Sep 17 00:00:00 2001
From: Bernardo Dal Seno <bdalseno@google.com>
Date: Mon, 11 Mar 2013 15:12:21 +0100
Subject: [PATCH] Add --ipolicy-xxx-specs options

These options allow to specify whole instance policy specs. This is needed
for the upcoming changes that tend to threat specs as monolithic objects.

Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Helga Velroyen <helgav@google.com>
---
 lib/cli.py                     | 75 +++++++++++++++++++++++++---
 lib/client/gnt_cluster.py      | 14 ++++--
 lib/client/gnt_group.py        |  7 ++-
 man/gnt-cluster.rst            | 37 ++++++++++----
 man/gnt-group.rst              |  2 +
 test/py/ganeti.cli_unittest.py | 90 ++++++++++++++++++++++++++++++++++
 6 files changed, 203 insertions(+), 22 deletions(-)

diff --git a/lib/cli.py b/lib/cli.py
index 3b06442f5..9c83c419d 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -188,6 +188,7 @@ __all__ = [
   "SPECS_DISK_SIZE_OPT",
   "SPECS_MEM_SIZE_OPT",
   "SPECS_NIC_COUNT_OPT",
+  "IPOLICY_STD_SPECS_OPT",
   "IPOLICY_DISK_TEMPLATES",
   "IPOLICY_VCPU_RATIO",
   "SPICE_CACERT_OPT",
@@ -957,6 +958,18 @@ SPECS_NIC_COUNT_OPT = cli_option("--specs-nic-count", dest="ispecs_nic_count",
                                  help="NIC count specs: list of key=value,"
                                  " where key is one of min, max, std")
 
+IPOLICY_BOUNDS_SPECS_STR = "--ipolicy-bounds-specs"
+IPOLICY_BOUNDS_SPECS_OPT = cli_option(IPOLICY_BOUNDS_SPECS_STR,
+                                      dest="ipolicy_bounds_specs",
+                                      type="listidentkeyval", default=None,
+                                      help="Complete instance specs limits")
+
+IPOLICY_STD_SPECS_STR = "--ipolicy-std-specs"
+IPOLICY_STD_SPECS_OPT = cli_option(IPOLICY_STD_SPECS_STR,
+                                   dest="ipolicy_std_specs",
+                                   type="keyval", default=None,
+                                   help="Complte standard instance specs")
+
 IPOLICY_DISK_TEMPLATES = cli_option("--ipolicy-disk-templates",
                                     dest="ipolicy_disk_templates",
                                     type="list", default=None,
@@ -1643,6 +1656,7 @@ INSTANCE_POLICY_OPTS = [
   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,
@@ -3789,9 +3803,9 @@ def _MaybeParseUnit(elements):
   return parsed
 
 
-def _InitIspecsFromOpts(ipolicy, ispecs_mem_size, ispecs_cpu_count,
-                        ispecs_disk_count, ispecs_disk_size, ispecs_nic_count,
-                        group_ipolicy, allowed_values):
+def _InitISpecsFromSplitOpts(ipolicy, ispecs_mem_size, ispecs_cpu_count,
+                             ispecs_disk_count, ispecs_disk_size,
+                             ispecs_nic_count, group_ipolicy, allowed_values):
   try:
     if ispecs_mem_size:
       ispecs_mem_size = _MaybeParseUnit(ispecs_mem_size)
@@ -3818,6 +3832,7 @@ def _InitIspecsFromOpts(ipolicy, ispecs_mem_size, ispecs_cpu_count,
   else:
     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)
 
   # then transpose
@@ -3836,11 +3851,49 @@ def _InitIspecsFromOpts(ipolicy, ispecs_mem_size, ispecs_cpu_count,
   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:
+      try:
+        ret[k] = utils.ParseUnit(ret[k])
+      except (TypeError, ValueError, errors.UnitParseError), err:
+        raise errors.OpPrereqError(("Invalid parameter %s (%s) in %s instance"
+                                    " specs: %s" % (k, ret[k], keyname, err)),
+                                   errors.ECODE_INVAL)
+  return ret
+
+
+def _ParseISpec(spec, keyname, allowed_values):
+  ret = _ParseSpecUnit(spec, keyname)
+  utils.ForceDictType(ret, constants.ISPECS_PARAMETER_TYPES,
+                      allowed_values=allowed_values)
+  return ret
+
+
+def _InitISpecsFromFullOpts(ipolicy_out, minmax_ispecs, std_ispecs,
+                            group_ipolicy, allowed_values):
+  if 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)
+    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)
+
+
 def CreateIPolicyFromOpts(ispecs_mem_size=None,
                           ispecs_cpu_count=None,
                           ispecs_disk_count=None,
                           ispecs_disk_size=None,
                           ispecs_nic_count=None,
+                          minmax_ispecs=None,
+                          std_ispecs=None,
                           ipolicy_disk_templates=None,
                           ipolicy_vcpu_ratio=None,
                           ipolicy_spindle_ratio=None,
@@ -3854,11 +3907,21 @@ def CreateIPolicyFromOpts(ispecs_mem_size=None,
 
 
   """
+  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)):
+    raise errors.OpPrereqError("A --specs-xxx option cannot be specified"
+                               " together with any --ipolicy-xxx-specs option",
+                               errors.ECODE_INVAL)
 
   ipolicy_out = objects.MakeEmptyIPolicy()
-  _InitIspecsFromOpts(ipolicy_out, ispecs_mem_size, ispecs_cpu_count,
-                      ispecs_disk_count, ispecs_disk_size, ispecs_nic_count,
-                      group_ipolicy, allowed_values)
+  if minmax_ispecs is None and std_ispecs is None:
+    _InitISpecsFromSplitOpts(ipolicy_out, ispecs_mem_size, ispecs_cpu_count,
+                             ispecs_disk_count, ispecs_disk_size,
+                             ispecs_nic_count, group_ipolicy, allowed_values)
+  else:
+    _InitISpecsFromFullOpts(ipolicy_out, minmax_ispecs, std_ispecs,
+                            group_ipolicy, allowed_values)
 
   if ipolicy_disk_templates is not None:
     if allowed_values and ipolicy_disk_templates in allowed_values:
diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py
index 4183b9b45..e0e2d7fc2 100644
--- a/lib/client/gnt_cluster.py
+++ b/lib/client/gnt_cluster.py
@@ -150,6 +150,8 @@ def InitCluster(opts, args):
     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,
     ipolicy_vcpu_ratio=opts.ipolicy_vcpu_ratio,
     ipolicy_spindle_ratio=opts.ipolicy_spindle_ratio,
@@ -969,6 +971,8 @@ def SetClusterParams(opts, args):
           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
           opts.ipolicy_vcpu_ratio is not None or
           opts.ipolicy_spindle_ratio is not None):
@@ -1025,6 +1029,8 @@ def SetClusterParams(opts, args):
     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,
     ipolicy_vcpu_ratio=opts.ipolicy_vcpu_ratio,
     ipolicy_spindle_ratio=opts.ipolicy_spindle_ratio,
@@ -1495,8 +1501,8 @@ commands = {
      MAINTAIN_NODE_HEALTH_OPT, UIDPOOL_OPT, DRBD_HELPER_OPT, NODRBD_STORAGE_OPT,
      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]
-     + INSTANCE_POLICY_OPTS,
+     DISK_PARAMS_OPT, HV_STATE_OPT, DISK_STATE_OPT, ENABLED_DISK_TEMPLATES_OPT,
+     IPOLICY_STD_SPECS_OPT] + INSTANCE_POLICY_OPTS,
     "[opts...] <cluster_name>", "Initialises a new cluster configuration"),
   "destroy": (
     DestroyCluster, ARGS_NONE, [YES_DOIT_OPT],
@@ -1574,8 +1580,8 @@ commands = {
      DRBD_HELPER_OPT, NODRBD_STORAGE_OPT, DEFAULT_IALLOCATOR_OPT,
      RESERVED_LVS_OPT, DRY_RUN_OPT, PRIORITY_OPT, PREALLOC_WIPE_DISKS_OPT,
      NODE_PARAMS_OPT, USE_EXTERNAL_MIP_SCRIPT, DISK_PARAMS_OPT, HV_STATE_OPT,
-     DISK_STATE_OPT, SUBMIT_OPT, ENABLED_DISK_TEMPLATES_OPT] +
-    INSTANCE_POLICY_OPTS,
+     DISK_STATE_OPT, SUBMIT_OPT, ENABLED_DISK_TEMPLATES_OPT,
+     IPOLICY_STD_SPECS_OPT] + INSTANCE_POLICY_OPTS,
     "[opts...]",
     "Alters the parameters of the cluster"),
   "renew-crypto": (
diff --git a/lib/client/gnt_group.py b/lib/client/gnt_group.py
index 2d11618a2..2f8dcc9dc 100644
--- a/lib/client/gnt_group.py
+++ b/lib/client/gnt_group.py
@@ -53,6 +53,7 @@ def AddGroup(opts, args):
     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,
     group_ipolicy=True)
@@ -161,8 +162,9 @@ 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_vcpu_ratio,
-             opts.ipolicy_spindle_ratio, opts.diskparams]
+             opts.ispecs_nic_count, opts.ipolicy_bounds_specs,
+             opts.ipolicy_vcpu_ratio, opts.ipolicy_spindle_ratio,
+             opts.diskparams]
   if allmods.count(None) == len(allmods):
     ToStderr("Please give at least one of the parameters.")
     return 1
@@ -196,6 +198,7 @@ def SetGroupParams(opts, args):
     ispecs_disk_count=opts.ispecs_disk_count,
     ispecs_disk_size=opts.ispecs_disk_size,
     ispecs_nic_count=opts.ispecs_nic_count,
+    minmax_ispecs=opts.ipolicy_bounds_specs,
     ipolicy_disk_templates=opts.ipolicy_disk_templates,
     ipolicy_vcpu_ratio=opts.ipolicy_vcpu_ratio,
     ipolicy_spindle_ratio=opts.ipolicy_spindle_ratio,
diff --git a/man/gnt-cluster.rst b/man/gnt-cluster.rst
index f1aae99d7..c42c5fc5e 100644
--- a/man/gnt-cluster.rst
+++ b/man/gnt-cluster.rst
@@ -183,6 +183,8 @@ INIT
 | [\--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*...]]
 | [\--ipolicy-spindle-ratio *ratio*]
 | [\--ipolicy-vcpu-ratio *ratio*]
@@ -492,16 +494,24 @@ that the master will try to keep as master\_candidates. For more
 details about this role and other node roles, see the **ganeti**\(7).
 
 The ``--specs-...`` and ``--ipolicy-...`` options specify the instance
-policy on the cluster. For the ``--specs-...`` options, each option can
-have three values: ``min``, ``max`` and ``std``, which can also be
-modified on group level (except for ``std``, which is defined once for
-the entire cluster). Please note, that ``std`` values are not the same
-as defaults set by ``--beparams``, but they are used for the capacity
-calculations.
-
-The ``--ipolicy-disk-templates`` and ``--ipolicy-spindle-ratio`` options
-take a decimal number. The ``--ipolicy-disk-templates`` option takes a
-comma-separated list of disk templates.
+policy on the cluster. The ``--ipolicy-bounds-specs`` option sets the
+minimum and maximum specifications for instances. The format is:
+min:*param*=*value*,.../max:*param*=*value*,... The
+``--ipolicy-std-specs`` option takes a list of parameter/value pairs.
+For both options, *param* can be:
+
+- ``cpu-count``: number of VCPUs for an instance
+- ``disk-count``: number of disk for an instance
+- ``disk-size``: size of each disk
+- ``memory-size``: instance memory
+- ``nic-count``: number of network interface
+- ``spindle-use``: spindle usage for an instance
+
+For the ``--specs-...`` options, each option can have three values:
+``min``, ``max`` and ``std``, which can also be modified on group level
+(except for ``std``, which is defined once for the entire cluster).
+Please note, that ``std`` values are not the same as defaults set by
+``--beparams``, but they are used for the capacity calculations.
 
 - ``--specs-cpu-count`` limits the number of VCPUs that can be used by an
   instance.
@@ -509,6 +519,11 @@ comma-separated list of disk templates.
 - ``--specs-disk-size`` limits the disk size for every disk used
 - ``--specs-mem-size`` limits the amount of memory available
 - ``--specs-nic-count`` sets limits on the number of NICs used
+
+The ``--ipolicy-disk-templates`` and ``--ipolicy-spindle-ratio`` options
+take a decimal number. The ``--ipolicy-disk-templates`` option takes a
+comma-separated list of disk templates.
+
 - ``--ipolicy-disk-templates`` limits the allowed disk templates
 - ``--ipolicy-spindle-ratio`` limits the instances-spindles ratio
 - ``--ipolicy-vcpu-ratio`` limits the vcpu-cpu ratio
@@ -589,6 +604,8 @@ MODIFY
 | [\--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*...]]
 | [\--ipolicy-spindle-ratio *ratio*]
 | [\--ipolicy-vcpu-ratio *ratio*]
diff --git a/man/gnt-group.rst b/man/gnt-group.rst
index 2cf0a055f..a633dc98c 100644
--- a/man/gnt-group.rst
+++ b/man/gnt-group.rst
@@ -32,6 +32,7 @@ ADD
 | [\--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*]
 | [\--ipolicy-vcpu-ratio *ratio*]
@@ -109,6 +110,7 @@ MODIFY
 | [\--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*]
 | [\--ipolicy-vcpu-ratio *ratio*]
diff --git a/test/py/ganeti.cli_unittest.py b/test/py/ganeti.cli_unittest.py
index 151104339..96f84d60d 100755
--- a/test/py/ganeti.cli_unittest.py
+++ b/test/py/ganeti.cli_unittest.py
@@ -1332,6 +1332,96 @@ class TestCreateIPolicyFromOpts(unittest.TestCase):
                                      allowed_values=[allowedv])
     self.assertEqual(pol1, exp_pol1)
 
+  @staticmethod
+  def _ConvertSpecToStrings(spec):
+    ret = {}
+    for (par, val) in spec.items():
+      ret[par] = str(val)
+    return ret
+
+  def _CheckNewStyleSpecsCall(self, exp_ipolicy, minmax_ispecs, std_ispecs,
+                              group_ipolicy, fill_all):
+    ipolicy = cli.CreateIPolicyFromOpts(minmax_ispecs=minmax_ispecs,
+                                        std_ispecs=std_ispecs,
+                                        group_ipolicy=group_ipolicy,
+                                        fill_all=fill_all)
+    self.assertEqual(ipolicy, exp_ipolicy)
+
+  def _TestFullISpecsInner(self, skel_exp_ipol, exp_minmax, exp_std,
+                           group_ipolicy, fill_all):
+    exp_ipol = skel_exp_ipol.copy()
+    if exp_minmax is not None:
+      minmax_ispecs = {}
+      for (key, spec) in exp_minmax.items():
+        minmax_ispecs[key] = self._ConvertSpecToStrings(spec)
+      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)
+    if minmax_ispecs:
+      for (key, spec) in minmax_ispecs.items():
+        for par in [constants.ISPEC_MEM_SIZE, constants.ISPEC_DISK_SIZE]:
+          if par in spec:
+            spec[par] += "m"
+            self._CheckNewStyleSpecsCall(exp_ipol, minmax_ispecs, std_ispecs,
+                                         group_ipolicy, fill_all)
+    if std_ispecs:
+      for par in [constants.ISPEC_MEM_SIZE, constants.ISPEC_DISK_SIZE]:
+        if par in std_ispecs:
+          std_ispecs[par] += "m"
+          self._CheckNewStyleSpecsCall(exp_ipol, minmax_ispecs, std_ispecs,
+                                       group_ipolicy, fill_all)
+
+  def testFullISpecs(self):
+    exp_minmax1 = {
+      constants.ISPECS_MIN: {
+        constants.ISPEC_MEM_SIZE: 512,
+        constants.ISPEC_CPU_COUNT: 2,
+        constants.ISPEC_DISK_COUNT: 2,
+        constants.ISPEC_DISK_SIZE: 512,
+        constants.ISPEC_NIC_COUNT: 2,
+        constants.ISPEC_SPINDLE_USE: 2,
+        },
+      constants.ISPECS_MAX: {
+        constants.ISPEC_MEM_SIZE: 768*1024,
+        constants.ISPEC_CPU_COUNT: 7,
+        constants.ISPEC_DISK_COUNT: 6,
+        constants.ISPEC_DISK_SIZE: 2048*1024,
+        constants.ISPEC_NIC_COUNT: 3,
+        constants.ISPEC_SPINDLE_USE: 1,
+        },
+      }
+    exp_std1 = {
+      constants.ISPEC_MEM_SIZE: 768*1024,
+      constants.ISPEC_CPU_COUNT: 7,
+      constants.ISPEC_DISK_COUNT: 6,
+      constants.ISPEC_DISK_SIZE: 2048*1024,
+      constants.ISPEC_NIC_COUNT: 3,
+      constants.ISPEC_SPINDLE_USE: 1,
+      }
+    for fill_all in [False, True]:
+      if fill_all:
+        skel_ipolicy = constants.IPOLICY_DEFAULTS
+      else:
+        skel_ipolicy = {}
+      self._TestFullISpecsInner(skel_ipolicy, exp_minmax1, exp_std1,
+                                False, fill_all)
+      self._TestFullISpecsInner(skel_ipolicy, None, exp_std1,
+                                False, fill_all)
+      self._TestFullISpecsInner(skel_ipolicy, exp_minmax1, None,
+                                False, fill_all)
 
 if __name__ == "__main__":
   testutils.GanetiTestProgram()
-- 
GitLab