commit ec99611768f462a225722dac64e4d25c53389571
Author: Bernardo Dal Seno <bdalseno@google.com>
Date:   Tue Apr 23 01:21:01 2013 +0200

    QA: Transpose instance specs
    
    The format used to store instance specs in QA is changed to better handle
    multiple instance specs, a feature contained in the following patches.
    
    Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
    Reviewed-by: Helga Velroyen <helgav@google.com>

diff --git a/qa/ganeti-qa.py b/qa/ganeti-qa.py
index 5a09a8c..b4d8d2a 100755
--- a/qa/ganeti-qa.py
+++ b/qa/ganeti-qa.py
@@ -523,7 +523,11 @@ def RunExclusiveStorageTests():
 
 
 def _BuildSpecDict(par, mn, st, mx):
-  return {par: {"min": mn, "std": st, "max": mx}}
+  return {
+    "min": {par: mn},
+    "max": {par: mx},
+    "std": {par: st},
+    }
 
 
 def TestIPolicyPlainInstance():
@@ -534,10 +538,11 @@ def TestIPolicyPlainInstance():
     return
 
   # This test assumes that the group policy is empty
-  (_, old_specs) = qa_cluster.TestClusterSetISpecs({})
+  (_, old_specs) = qa_cluster.TestClusterSetISpecs()
   node = qa_config.AcquireNode()
   try:
-    # Log of policy changes, list of tuples: (change, policy_violated)
+    # Log of policy changes, list of tuples:
+    # (full_change, incremental_change, policy_violated)
     history = []
     instance = qa_instance.TestInstanceAddWithPlainDisk([node])
     try:
@@ -547,8 +552,8 @@ def TestIPolicyPlainInstance():
         (iminval, imaxval) = qa_instance.GetInstanceSpec(instance.name, par)
         # Some specs must be multiple of 4
         new_spec = _BuildSpecDict(par, imaxval + 4, imaxval + 4, imaxval + 4)
-        history.append((new_spec, True))
-        qa_cluster.TestClusterSetISpecs(new_spec)
+        history.append((None, new_spec, True))
+        qa_cluster.TestClusterSetISpecs(diff_specs=new_spec)
         qa_cluster.AssertClusterVerify(warnings=policyerror)
         if iminval > 0:
           # Some specs must be multiple of 4
@@ -557,19 +562,20 @@ def TestIPolicyPlainInstance():
           else:
             upper = iminval - 1
           new_spec = _BuildSpecDict(par, 0, upper, upper)
-          history.append((new_spec, True))
-          qa_cluster.TestClusterSetISpecs(new_spec)
+          history.append((None, new_spec, True))
+          qa_cluster.TestClusterSetISpecs(diff_specs=new_spec)
           qa_cluster.AssertClusterVerify(warnings=policyerror)
-        qa_cluster.TestClusterSetISpecs(old_specs)
-        history.append((old_specs, False))
+        qa_cluster.TestClusterSetISpecs(new_specs=old_specs)
+        history.append((old_specs, None, False))
       qa_instance.TestInstanceRemove(instance)
     finally:
       instance.Release()
 
     # Now we replay the same policy changes, and we expect that the instance
     # cannot be created for the cases where we had a policy violation above
-    for (change, failed) in history:
-      qa_cluster.TestClusterSetISpecs(change)
+    for (new_specs, diff_specs, failed) in history:
+      qa_cluster.TestClusterSetISpecs(new_specs=new_specs,
+                                      diff_specs=diff_specs)
       if failed:
         qa_instance.TestInstanceAddWithPlainDisk([node], fail=True)
       # Instance creation with no policy violation has been tested already
diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py
index f532826..a139e5f 100644
--- a/qa/qa_cluster.py
+++ b/qa/qa_cluster.py
@@ -532,7 +532,7 @@ def _GetClusterIPolicy():
   @rtype: tuple
   @return: (policy, specs), where:
       - policy is a dictionary of the policy values, instance specs excluded
-      - specs is dict of dict, specs[par][key] is a spec value, where key is
+      - specs is dict of dict, specs[key][par] is a spec value, where key is
         "min", "max", or "std"
 
   """
@@ -541,10 +541,7 @@ def _GetClusterIPolicy():
   (ret_policy, ret_specs) = qa_utils.ParseIPolicy(policy)
 
   # Sanity checks
-  assert len(ret_specs) > 0
-  good = all("min" in d and "std" in d and "max" in d
-             for d in ret_specs.values())
-  assert good, "Missing item in specs: %s" % ret_specs
+  assert "min" in ret_specs and "std" in ret_specs and "max" in ret_specs
   assert len(ret_policy) > 0
   return (ret_policy, ret_specs)
 
@@ -606,24 +603,31 @@ def TestClusterModifyIPolicy():
       AssertEqual(eff_policy[p], old_policy[p])
 
 
-def TestClusterSetISpecs(new_specs, fail=False, old_values=None):
+def TestClusterSetISpecs(new_specs=None, diff_specs=None, fail=False,
+                         old_values=None):
   """Change instance specs.
 
-  @type new_specs: dict of dict
-  @param new_specs: new_specs[par][key], where key is "min", "max", "std". It
-      can be an empty dictionary.
+  At most one of new_specs or diff_specs can be specified.
+
+  @type new_specs: dict
+  @param new_specs: new complete specs, in the same format returned by
+      L{_GetClusterIPolicy}
+  @type diff_specs: dict
+  @param diff_specs: diff_specs[key][par], where key is "min", "max", "std". It
+      can be an incomplete specifications or an empty dictionary.
   @type fail: bool
   @param fail: if the change is expected to fail
   @type old_values: tuple
   @param old_values: (old_policy, old_specs), as returned by
-     L{_GetClusterIPolicy}
+      L{_GetClusterIPolicy}
   @return: same as L{_GetClusterIPolicy}
 
   """
   build_cmd = lambda opts: ["gnt-cluster", "modify"] + opts
-  return qa_utils.TestSetISpecs(new_specs, get_policy_fn=_GetClusterIPolicy,
-                                build_cmd_fn=build_cmd, fail=fail,
-                                old_values=old_values)
+  return qa_utils.TestSetISpecs(
+    new_specs=new_specs, diff_specs=diff_specs,
+    get_policy_fn=_GetClusterIPolicy, build_cmd_fn=build_cmd,
+    fail=fail, old_values=old_values)
 
 
 def TestClusterModifyISpecs():
@@ -646,14 +650,18 @@ def TestClusterModifyISpecs():
       (False, 0, 4, "a"),
       # This is to restore the old values
       (True,
-       cur_specs[par]["min"], cur_specs[par]["std"], cur_specs[par]["max"])
+       cur_specs["min"][par], cur_specs["std"][par], cur_specs["max"][par])
       ]
     for (good, mn, st, mx) in test_values:
-      new_vals = {par: {"min": str(mn), "std": str(st), "max": str(mx)}}
+      new_vals = {
+        "min": {par: mn},
+        "std": {par: st},
+        "max": {par: mx}
+        }
       cur_state = (cur_policy, cur_specs)
       # We update cur_specs, as we've copied the values to restore already
-      (cur_policy, cur_specs) = TestClusterSetISpecs(new_vals, fail=not good,
-                                                     old_values=cur_state)
+      (cur_policy, cur_specs) = TestClusterSetISpecs(
+        diff_specs=new_vals, fail=not good, old_values=cur_state)
 
     # Get the ipolicy command
     mnode = qa_config.GetMasterNode()
diff --git a/qa/qa_group.py b/qa/qa_group.py
index 536515f..202bde8 100644
--- a/qa/qa_group.py
+++ b/qa/qa_group.py
@@ -87,7 +87,7 @@ def _GetGroupIPolicy(groupname):
   @rtype: tuple
   @return: (policy, specs), where:
       - policy is a dictionary of the policy values, instance specs excluded
-      - specs is dict of dict, specs[par][key] is a spec value, where key is
+      - specs is dict of dict, specs[key][par] is a spec value, where key is
         "min" or "max"
 
   """
@@ -98,51 +98,57 @@ def _GetGroupIPolicy(groupname):
   (ret_policy, ret_specs) = qa_utils.ParseIPolicy(policy)
 
   # Sanity checks
-  assert len(ret_specs) > 0
-  good = all("min" in d and "max" in d
-             for d in ret_specs.values())
-  assert good, "Missing item in specs: %s" % ret_specs
+  assert "min" in ret_specs and "max" in ret_specs
   assert len(ret_policy) > 0
   return (ret_policy, ret_specs)
 
 
-def _TestGroupSetISpecs(groupname, new_specs, fail=False, old_values=None):
+def _TestGroupSetISpecs(groupname, new_specs=None, diff_specs=None,
+                        fail=False, old_values=None):
   """Change instance specs on a group.
 
+  At most one of new_specs or diff_specs can be specified.
+
   @type groupname: string
   @param groupname: group name
-  @type new_specs: dict of dict
-  @param new_specs: new_specs[par][key], where key is "min", "max", "std". It
-      can be an empty dictionary.
+  @type new_specs: dict
+  @param new_specs: new complete specs, in the same format returned by
+      L{_GetGroupIPolicy}
+  @type diff_specs: dict
+  @param diff_specs: diff_specs[key][par], where key is "min", "max". It
+      can be an incomplete specifications or an empty dictionary.
   @type fail: bool
   @param fail: if the change is expected to fail
   @type old_values: tuple
   @param old_values: (old_policy, old_specs), as returned by
-     L{_GetGroupIPolicy}
+      L{_GetGroupIPolicy}
   @return: same as L{_GetGroupIPolicy}
 
   """
   build_cmd = lambda opts: ["gnt-group", "modify"] + opts + [groupname]
   get_policy = lambda: _GetGroupIPolicy(groupname)
-  return qa_utils.TestSetISpecs(new_specs, get_policy_fn=get_policy,
-                                build_cmd_fn=build_cmd, fail=fail,
-                                old_values=old_values)
+  return qa_utils.TestSetISpecs(
+    new_specs=new_specs, diff_specs=diff_specs,
+    get_policy_fn=get_policy, build_cmd_fn=build_cmd,
+    fail=fail, old_values=old_values)
 
 
 def _TestGroupModifyISpecs(groupname):
   # This test is built on the assumption that the default ipolicy holds for
   # the node group under test
   old_values = _GetGroupIPolicy(groupname)
-  mod_values = _TestGroupSetISpecs(groupname,
-                                   dict((p, {"min": 4, "max": 4})
-                                        for p in constants.ISPECS_PARAMETERS),
+  samevals = dict((p, 4) for p in constants.ISPECS_PARAMETERS)
+  base_specs = {"min": samevals, "max": samevals}
+  mod_values = _TestGroupSetISpecs(groupname, new_specs=base_specs,
                                    old_values=old_values)
   for par in constants.ISPECS_PARAMETERS:
     # First make sure that the test works with good values
-    mod_values = _TestGroupSetISpecs(groupname, {par: {"min": 8, "max": 8}},
+    good_specs = {"min": {par: 8}, "max": {par: 8}}
+    mod_values = _TestGroupSetISpecs(groupname, diff_specs=good_specs,
                                      old_values=mod_values)
-    _TestGroupSetISpecs(groupname, {par: {"min": 8, "max": 4}},
-                        fail=True, old_values=mod_values)
+    bad_specs = {"min": {par: 8}, "max": {par: 4}}
+    _TestGroupSetISpecs(groupname, diff_specs=bad_specs, fail=True,
+                        old_values=mod_values)
   AssertCommand(["gnt-group", "modify", "--ipolicy-bounds-specs", "default",
                  groupname])
   AssertEqual(_GetGroupIPolicy(groupname), old_values)
diff --git a/qa/qa_utils.py b/qa/qa_utils.py
index 9cea4b2..412f913 100644
--- a/qa/qa_utils.py
+++ b/qa/qa_utils.py
@@ -23,6 +23,7 @@
 
 """
 
+import copy
 import operator
 import os
 import random
@@ -779,31 +780,28 @@ def MakeNodePath(node, path):
     return path
 
 
-def _GetParameterOptions(key, specs, old_specs):
+def _GetParameterOptions(specs):
   """Helper to build policy options."""
-  values = ["%s=%s" % (par, keyvals[key])
-            for (par, keyvals) in specs.items()
-            if key in keyvals]
-  if old_specs:
-    present_pars = frozenset(par
-                             for (par, keyvals) in specs.items()
-                             if key in keyvals)
-    values.extend("%s=%s" % (par, keyvals[key])
-                  for (par, keyvals) in old_specs.items()
-                  if key in keyvals and par not in present_pars)
+  values = ["%s=%s" % (par, val)
+            for (par, val) in specs.items()]
   return ",".join(values)
 
 
-def TestSetISpecs(new_specs, get_policy_fn=None, build_cmd_fn=None,
-                  fail=False, old_values=None):
+def TestSetISpecs(new_specs=None, diff_specs=None, get_policy_fn=None,
+                  build_cmd_fn=None, fail=False, old_values=None):
   """Change instance specs for an object.
 
-  @type new_specs: dict of dict
-  @param new_specs: new_specs[par][key], where key is "min", "max", "std". It
-      can be an empty dictionary.
+  At most one of new_specs or diff_specs can be specified.
+
+  @type new_specs: dict
+  @param new_specs: new complete specs, in the same format returned by
+      L{ParseIPolicy}.
+  @type diff_specs: dict
+  @param diff_specs: diff_specs[key][par], where key is "min", "max", "std". It
+      can be an incomplete specifications or an empty dictionary.
   @type get_policy_fn: function
   @param get_policy_fn: function that returns the current policy as in
-      L{qa_cluster._GetClusterIPolicy}
+      L{ParseIPolicy}
   @type build_cmd_fn: function
   @param build_cmd_fn: function that return the full command line from the
       options alone
@@ -811,45 +809,57 @@ def TestSetISpecs(new_specs, get_policy_fn=None, build_cmd_fn=None,
   @param fail: if the change is expected to fail
   @type old_values: tuple
   @param old_values: (old_policy, old_specs), as returned by
-     L{qa_cluster._GetClusterIPolicy}
-  @return: same as L{qa_cluster._GetClusterIPolicy}
+     L{ParseIPolicy}
+  @return: same as L{ParseIPolicy}
 
   """
   assert get_policy_fn is not None
   assert build_cmd_fn is not None
+  assert new_specs is None or diff_specs is None
 
   if old_values:
     (old_policy, old_specs) = old_values
   else:
     (old_policy, old_specs) = get_policy_fn()
+
+  if diff_specs:
+    new_specs = copy.deepcopy(old_specs)
+    for (key, parvals) in diff_specs.items():
+      for (par, val) in parvals.items():
+        new_specs[key][par] = val
+
   if new_specs:
     cmd = []
-    if any(("min" in val or "max" in val) for val in new_specs.values()):
+    if (diff_specs is None or
+        ("min" in diff_specs or "max" in diff_specs)):
       minmax_opt_items = []
       for key in ["min", "max"]:
-        keyopt = _GetParameterOptions(key, new_specs, old_specs)
+        keyopt = _GetParameterOptions(new_specs[key])
         minmax_opt_items.append("%s:%s" % (key, keyopt))
       cmd.extend([
         "--ipolicy-bounds-specs",
         "/".join(minmax_opt_items)
         ])
-    std_opt = _GetParameterOptions("std", new_specs, {})
+    if diff_specs:
+      std_source = diff_specs
+    else:
+      std_source = new_specs
+    std_opt = _GetParameterOptions(std_source.get("std", {}))
     if std_opt:
       cmd.extend(["--ipolicy-std-specs", std_opt])
     AssertCommand(build_cmd_fn(cmd), fail=fail)
 
-  # Check the new state
-  (eff_policy, eff_specs) = get_policy_fn()
-  AssertEqual(eff_policy, old_policy)
-  if fail:
-    AssertEqual(eff_specs, old_specs)
+    # Check the new state
+    (eff_policy, eff_specs) = get_policy_fn()
+    AssertEqual(eff_policy, old_policy)
+    if fail:
+      AssertEqual(eff_specs, old_specs)
+    else:
+      AssertEqual(eff_specs, new_specs)
+
   else:
-    for par in eff_specs:
-      for key in eff_specs[par]:
-        if par in new_specs and key in new_specs[par]:
-          AssertEqual(int(eff_specs[par][key]), int(new_specs[par][key]))
-        else:
-          AssertEqual(int(eff_specs[par][key]), int(old_specs[par][key]))
+    (eff_policy, eff_specs) = (old_policy, old_specs)
+
   return (eff_policy, eff_specs)
 
 
@@ -861,7 +871,7 @@ def ParseIPolicy(policy):
   @rtype: tuple
   @return: (policy, specs), where:
       - policy is a dictionary of the policy values, instance specs excluded
-      - specs is dict of dict, specs[par][key] is a spec value, where key is
+      - specs is dict of dict, specs[key][par] is a spec value, where key is
         "min", "max", or "std"
 
   """
@@ -870,9 +880,7 @@ def ParseIPolicy(policy):
   ispec_keys = constants.ISPECS_MINMAX_KEYS | frozenset([constants.ISPECS_STD])
   for (key, val) in policy.items():
     if key in ispec_keys:
-      for (par, pval) in val.items():
-        d = ret_specs.setdefault(par, {})
-        d[key] = pval
+      ret_specs[key] = val
     else:
       ret_policy[key] = val
   return (ret_policy, ret_specs)
