Hi!
On Wed, Apr 24, 2013 at 2:00 PM, Bernardo Dal Seno <bdals...@google.com>wrote:
> Existing tests are updated to cope with the new instance specs format.
>
> Signed-off-by: Bernardo Dal Seno <bdals...@google.com>
> ---
> qa/ganeti-qa.py | 10 +++++++---
> qa/qa_cluster.py | 26 +++++++++++++++++---------
> qa/qa_group.py | 34 ++++++++++++++++++++++++++--------
> qa/qa_utils.py | 54
> ++++++++++++++++++++++++++++++++++++------------------
> 4 files changed, 86 insertions(+), 38 deletions(-)
>
> diff --git a/qa/ganeti-qa.py b/qa/ganeti-qa.py
> index b4d8d2a..ca698d0 100755
> --- a/qa/ganeti-qa.py
> +++ b/qa/ganeti-qa.py
> @@ -524,9 +524,11 @@ def RunExclusiveStorageTests():
>
> def _BuildSpecDict(par, mn, st, mx):
> return {
> - "min": {par: mn},
> - "max": {par: mx},
> - "std": {par: st},
> + constants.ISPECS_MINMAX: [{
> + constants.ISPECS_MIN: {par: mn},
> + constants.ISPECS_MAX: {par: mx},
> + }],
> + constants.ISPECS_STD: {par: st},
> }
>
>
> @@ -539,6 +541,8 @@ def TestIPolicyPlainInstance():
>
> # This test assumes that the group policy is empty
> (_, old_specs) = qa_cluster.TestClusterSetISpecs()
> + # We also assume to have only one min/max bound
> + assert len(old_specs[constants.ISPECS_MINMAX]) == 1
> node = qa_config.AcquireNode()
> try:
> # Log of policy changes, list of tuples:
> diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py
> index a139e5f..e281178 100644
> --- a/qa/qa_cluster.py
> +++ b/qa/qa_cluster.py
> @@ -532,8 +532,8 @@ 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[key][par] is a spec value, where key
> is
> - "min", "max", or "std"
> + - specs is a dictionary containing only the specs, using the
> internal
> + format (see L{constants.IPOLICY_DEFAULTS} for an example)
>
> """
> info = qa_utils.GetObjectInfo(["gnt-cluster", "info"])
> @@ -541,7 +541,8 @@ def _GetClusterIPolicy():
> (ret_policy, ret_specs) = qa_utils.ParseIPolicy(policy)
>
> # Sanity checks
> - assert "min" in ret_specs and "std" in ret_specs and "max" in ret_specs
> + assert "minmax" in ret_specs and "std" in ret_specs
> + assert len(ret_specs["minmax"]) > 0
> assert len(ret_policy) > 0
> return (ret_policy, ret_specs)
>
> @@ -613,8 +614,9 @@ def TestClusterSetISpecs(new_specs=None,
> diff_specs=None, fail=False,
> @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.
> + @param diff_specs: partial specs, it can be an incomplete
> specifications, but
> + if min/max specs are specified, their number must match the number
> of the
> + existing specs
> @type fail: bool
> @param fail: if the change is expected to fail
> @type old_values: tuple
> @@ -634,6 +636,8 @@ def TestClusterModifyISpecs():
> """gnt-cluster modify --specs-*"""
> params = ["memory-size", "disk-size", "disk-count", "cpu-count",
> "nic-count"]
> (cur_policy, cur_specs) = _GetClusterIPolicy()
> + # This test assumes that there is only one min/max bound
> + assert len(cur_specs[constants.ISPECS_MINMAX]) == 1
> for par in params:
> test_values = [
> (True, 0, 4, 12),
> @@ -650,13 +654,17 @@ def TestClusterModifyISpecs():
> (False, 0, 4, "a"),
> # This is to restore the old values
> (True,
> - cur_specs["min"][par], cur_specs["std"][par],
> cur_specs["max"][par])
> + cur_specs[constants.ISPECS_MINMAX][0][constants.ISPECS_MIN][par],
> + cur_specs[constants.ISPECS_STD][par],
> + cur_specs[constants.ISPECS_MINMAX][0][constants.ISPECS_MAX][par])
> ]
> for (good, mn, st, mx) in test_values:
> new_vals = {
> - "min": {par: mn},
> - "std": {par: st},
> - "max": {par: mx}
> + constants.ISPECS_MINMAX: [{
> + constants.ISPECS_MIN: {par: mn},
> + constants.ISPECS_MAX: {par: mx}
> + }],
> + constants.ISPECS_STD: {par: st}
> }
> cur_state = (cur_policy, cur_specs)
> # We update cur_specs, as we've copied the values to restore already
> diff --git a/qa/qa_group.py b/qa/qa_group.py
> index 202bde8..e71451a 100644
> --- a/qa/qa_group.py
> +++ b/qa/qa_group.py
> @@ -87,8 +87,9 @@ 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[key][par] is a spec value, where key
> is
> - "min" or "max"
> + - specs is a dictionary containing only the specs, using the
> internal
> + format (see L{constants.IPOLICY_DEFAULTS} for an example), but
> without
> + the standard values
>
> """
> info = qa_utils.GetObjectInfo(["gnt-group", "info", groupname])
> @@ -98,7 +99,8 @@ def _GetGroupIPolicy(groupname):
> (ret_policy, ret_specs) = qa_utils.ParseIPolicy(policy)
>
> # Sanity checks
> - assert "min" in ret_specs and "max" in ret_specs
> + assert "minmax" in ret_specs
> + assert len(ret_specs["minmax"]) > 0
> assert len(ret_policy) > 0
> return (ret_policy, ret_specs)
>
> @@ -115,8 +117,9 @@ def _TestGroupSetISpecs(groupname, new_specs=None,
> diff_specs=None,
> @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.
> + @param diff_specs: partial specs, it can be an incomplete
> specifications, but
> + if min/max specs are specified, their number must match the number
> of the
> + existing specs
> @type fail: bool
> @param fail: if the change is expected to fail
> @type old_values: tuple
> @@ -138,15 +141,30 @@ def _TestGroupModifyISpecs(groupname):
> # the node group under test
> old_values = _GetGroupIPolicy(groupname)
> samevals = dict((p, 4) for p in constants.ISPECS_PARAMETERS)
> - base_specs = {"min": samevals, "max": samevals}
> + base_specs = {
> + constants.ISPECS_MINMAX: [{
> + constants.ISPECS_MIN: samevals,
> + constants.ISPECS_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
> - good_specs = {"min": {par: 8}, "max": {par: 8}}
> + good_specs = {
> + constants.ISPECS_MINMAX: [{
> + constants.ISPECS_MIN: {par: 8},
> + constants.ISPECS_MAX: {par: 8},
> + }],
> + }
> mod_values = _TestGroupSetISpecs(groupname, diff_specs=good_specs,
> old_values=mod_values)
> - bad_specs = {"min": {par: 8}, "max": {par: 4}}
> + bad_specs = {
> + constants.ISPECS_MINMAX: [{
> + constants.ISPECS_MIN: {par: 8},
> + constants.ISPECS_MAX: {par: 4},
> + }],
> + }
> _TestGroupSetISpecs(groupname, diff_specs=bad_specs, fail=True,
> old_values=mod_values)
> AssertCommand(["gnt-group", "modify", "--ipolicy-bounds-specs",
> "default",
> diff --git a/qa/qa_utils.py b/qa/qa_utils.py
> index 412f913..7ec0d49 100644
> --- a/qa/qa_utils.py
> +++ b/qa/qa_utils.py
> @@ -797,8 +797,9 @@ def TestSetISpecs(new_specs=None, diff_specs=None,
> get_policy_fn=None,
> @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.
> + @param diff_specs: partial specs, it can be an incomplete
> specifications, but
> + if min/max specs are specified, their number must match the number
> of the
> + existing specs
> @type get_policy_fn: function
> @param get_policy_fn: function that returns the current policy as in
> L{ParseIPolicy}
> @@ -824,26 +825,35 @@ def TestSetISpecs(new_specs=None, diff_specs=None,
> get_policy_fn=None,
>
> 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 constants.ISPECS_MINMAX in diff_specs:
> + AssertEqual(len(new_specs[constants.ISPECS_MINMAX]),
> + len(diff_specs[constants.ISPECS_MINMAX]))
> + for (new_minmax, diff_minmax) in
> zip(new_specs[constants.ISPECS_MINMAX],
> +
> diff_specs[constants.ISPECS_MINMAX]):
> + for (key, parvals) in diff_minmax.items():
> + for (par, val) in parvals.items():
> + new_minmax[key][par] = val
> + for (par, val) in diff_specs.get(constants.ISPECS_STD, {}).items():
> + new_specs[constants.ISPECS_STD][par] = val
>
> if new_specs:
> cmd = []
> - if (diff_specs is None or
> - ("min" in diff_specs or "max" in diff_specs)):
> + if (diff_specs is None or constants.ISPECS_MINMAX in diff_specs):
> minmax_opt_items = []
> - for key in ["min", "max"]:
> - keyopt = _GetParameterOptions(new_specs[key])
> - minmax_opt_items.append("%s:%s" % (key, keyopt))
> + for minmax in new_specs[constants.ISPECS_MINMAX]:
> + minmax_opts = []
> + for key in ["min", "max"]:
> + keyopt = _GetParameterOptions(minmax[key])
> + minmax_opts.append("%s:%s" % (key, keyopt))
> + minmax_opt_items.append("/".join(minmax_opts))
> cmd.extend([
> "--ipolicy-bounds-specs",
> - "/".join(minmax_opt_items)
> + "//".join(minmax_opt_items)
> ])
> - if diff_specs:
> - std_source = diff_specs
> - else:
> + if diff_specs is None:
> std_source = new_specs
> + else:
> + std_source = diff_specs
> std_opt = _GetParameterOptions(std_source.get("std", {}))
> if std_opt:
> cmd.extend(["--ipolicy-std-specs", std_opt])
> @@ -871,15 +881,23 @@ 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[key][par] is a spec value, where key
> is
> - "min", "max", or "std"
> + - specs is a dictionary containing only the specs, using the
> internal
> + format (see L{constants.IPOLICY_DEFAULTS} for an example)
>
> """
> ret_specs = {}
> ret_policy = {}
> - ispec_keys = constants.ISPECS_MINMAX_KEYS |
> frozenset([constants.ISPECS_STD])
> for (key, val) in policy.items():
> - if key in ispec_keys:
> + if key == "bounds specs":
> + ret_specs[constants.ISPECS_MINMAX] = []
> + for minmax in val:
> + ret_minmax = {}
> + for key in minmax:
> + keyparts = key.split("/", 1)
> + assert len(keyparts) > 1
> + ret_minmax[keyparts[0]] = minmax[key]
> + ret_specs[constants.ISPECS_MINMAX].append(ret_minmax)
> + elif key == constants.ISPECS_STD:
> ret_specs[key] = val
> else:
> ret_policy[key] = val
> --
> 1.8.2.1
>
> LGTM, thanks
