From da5f09ef60fae8766b298be6789c02b59073c280 Mon Sep 17 00:00:00 2001 From: Bernardo Dal Seno <bdalseno@google.com> Date: Tue, 19 Feb 2013 22:14:13 +0100 Subject: [PATCH] Refactor ispecs in ipolicy structures 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> --- doc/rapi.rst | 45 +++++----- lib/cli.py | 11 ++- lib/client/gnt_cluster.py | 8 +- lib/cmdlib.py | 34 ++++++-- lib/config.py | 25 +++--- lib/constants.py | 14 ++-- lib/objects.py | 91 +++++++++++++------- qa/qa_cluster.py | 3 +- src/Ganeti/HTools/Backend/Text.hs | 8 +- src/Ganeti/HTools/Instance.hs | 7 +- src/Ganeti/HTools/Program/Hspace.hs | 3 +- src/Ganeti/HTools/Types.hs | 50 ++++++----- src/Ganeti/Objects.hs | 45 +++++++--- test/data/htools/hail-alloc-drbd.json | 32 ++++---- test/data/htools/hail-change-group.json | 96 ++++++++++++---------- test/data/htools/hail-node-evac.json | 32 ++++---- test/data/htools/hail-reloc-drbd.json | 32 ++++---- test/data/htools/rapi/groups.json | 32 ++++---- test/data/htools/rapi/info.json | 32 ++++---- test/hs/Test/Ganeti/HTools/Types.hs | 6 +- test/hs/Test/Ganeti/Objects.hs | 2 + test/hs/Test/Ganeti/TestHTools.hs | 31 +++---- test/py/ganeti.cli_unittest.py | 46 ++++++----- test/py/ganeti.cmdlib_unittest.py | 96 +++++++++++++++------- test/py/ganeti.objects_unittest.py | 105 ++++++++++++++---------- 25 files changed, 543 insertions(+), 343 deletions(-) diff --git a/doc/rapi.rst b/doc/rapi.rst index 4a7d708b0..10cd80e7f 100644 --- a/doc/rapi.rst +++ b/doc/rapi.rst @@ -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` diff --git a/lib/cli.py b/lib/cli.py index 1fa0eda16..7857e1810 100644 --- a/lib/cli.py +++ b/lib/cli.py @@ -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, diff --git a/lib/client/gnt_cluster.py b/lib/client/gnt_cluster.py index 1ce13b4ac..bbee602fb 100644 --- a/lib/client/gnt_cluster.py +++ b/lib/client/gnt_cluster.py @@ -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])), ] + diff --git a/lib/cmdlib.py b/lib/cmdlib.py index 1c0ad2109..55d313c9f 100644 --- a/lib/cmdlib.py +++ b/lib/cmdlib.py @@ -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)) diff --git a/lib/config.py b/lib/config.py index 3870e9712..58f665764 100644 --- a/lib/config.py +++ b/lib/config.py @@ -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), diff --git a/lib/constants.py b/lib/constants.py index 5a4e46531..3926f02ad 100644 --- a/lib/constants.py +++ b/lib/constants.py @@ -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, diff --git a/lib/objects.py b/lib/objects.py index 782dacc9b..d8dbca0d8 100644 --- a/lib/objects.py +++ b/lib/objects.py @@ -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 diff --git a/qa/qa_cluster.py b/qa/qa_cluster.py index e608b8caf..c7e02c56b 100644 --- a/qa/qa_cluster.py +++ b/qa/qa_cluster.py @@ -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" diff --git a/src/Ganeti/HTools/Backend/Text.hs b/src/Ganeti/HTools/Backend/Text.hs index cb3719cba..2ca928f37 100644 --- a/src/Ganeti/HTools/Backend/Text.hs +++ b/src/Ganeti/HTools/Backend/Text.hs @@ -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 diff --git a/src/Ganeti/HTools/Instance.hs b/src/Ganeti/HTools/Instance.hs index 9c20af1b2..d227cf174 100644 --- a/src/Ganeti/HTools/Instance.hs +++ b/src/Ganeti/HTools/Instance.hs @@ -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 diff --git a/src/Ganeti/HTools/Program/Hspace.hs b/src/Ganeti/HTools/Program/Hspace.hs index 02c81bf4a..73d39a485 100644 --- a/src/Ganeti/HTools/Program/Hspace.hs +++ b/src/Ganeti/HTools/Program/Hspace.hs @@ -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) <- diff --git a/src/Ganeti/HTools/Types.hs b/src/Ganeti/HTools/Types.hs index c394c6c69..e579e3634 100644 --- a/src/Ganeti/HTools/Types.hs +++ b/src/Ganeti/HTools/Types.hs @@ -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 diff --git a/src/Ganeti/Objects.hs b/src/Ganeti/Objects.hs index 7e85cf1b3..bfbab6baa 100644 --- a/src/Ganeti/Objects.hs +++ b/src/Ganeti/Objects.hs @@ -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 diff --git a/test/data/htools/hail-alloc-drbd.json b/test/data/htools/hail-alloc-drbd.json index 1f5686a2d..f650fbe60 100644 --- a/test/data/htools/hail-alloc-drbd.json +++ b/test/data/htools/hail-alloc-drbd.json @@ -14,21 +14,23 @@ "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": [ diff --git a/test/data/htools/hail-change-group.json b/test/data/htools/hail-change-group.json index 08d8908df..f698638e8 100644 --- a/test/data/htools/hail-change-group.json +++ b/test/data/htools/hail-change-group.json @@ -14,21 +14,23 @@ "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": [ @@ -56,21 +58,23 @@ "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": [ @@ -98,21 +102,23 @@ "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": [ diff --git a/test/data/htools/hail-node-evac.json b/test/data/htools/hail-node-evac.json index 941d41db6..8b80e2534 100644 --- a/test/data/htools/hail-node-evac.json +++ b/test/data/htools/hail-node-evac.json @@ -14,21 +14,23 @@ "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": [ diff --git a/test/data/htools/hail-reloc-drbd.json b/test/data/htools/hail-reloc-drbd.json index 09ef2bbdf..ce66041ce 100644 --- a/test/data/htools/hail-reloc-drbd.json +++ b/test/data/htools/hail-reloc-drbd.json @@ -14,21 +14,23 @@ "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": [ diff --git a/test/data/htools/rapi/groups.json b/test/data/htools/rapi/groups.json index 226eed78d..dab65698f 100644 --- a/test/data/htools/rapi/groups.json +++ b/test/data/htools/rapi/groups.json @@ -11,21 +11,23 @@ "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": [ diff --git a/test/data/htools/rapi/info.json b/test/data/htools/rapi/info.json index 821a32050..a37d4e52e 100644 --- a/test/data/htools/rapi/info.json +++ b/test/data/htools/rapi/info.json @@ -86,21 +86,23 @@ "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": [ diff --git a/test/hs/Test/Ganeti/HTools/Types.hs b/test/hs/Test/Ganeti/HTools/Types.hs index da2172543..3980edb86 100644 --- a/test/hs/Test/Ganeti/HTools/Types.hs +++ b/test/hs/Test/Ganeti/HTools/Types.hs @@ -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 diff --git a/test/hs/Test/Ganeti/Objects.hs b/test/hs/Test/Ganeti/Objects.hs index 1655aafb5..744d2b699 100644 --- a/test/hs/Test/Ganeti/Objects.hs +++ b/test/hs/Test/Ganeti/Objects.hs @@ -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) diff --git a/test/hs/Test/Ganeti/TestHTools.hs b/test/hs/Test/Ganeti/TestHTools.hs index b27c34c04..875850954 100644 --- a/test/hs/Test/Ganeti/TestHTools.hs +++ b/test/hs/Test/Ganeti/TestHTools.hs @@ -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 diff --git a/test/py/ganeti.cli_unittest.py b/test/py/ganeti.cli_unittest.py index 4a3072e31..c38ad0ed6 100755 --- a/test/py/ganeti.cli_unittest.py +++ b/test/py/ganeti.cli_unittest.py @@ -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: { }, diff --git a/test/py/ganeti.cmdlib_unittest.py b/test/py/ganeti.cmdlib_unittest.py index 85f1969a9..5603e4ae3 100755 --- a/test/py/ganeti.cmdlib_unittest.py +++ b/test/py/ganeti.cmdlib_unittest.py @@ -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) diff --git a/test/py/ganeti.objects_unittest.py b/test/py/ganeti.objects_unittest.py index a90afb2d2..720b22a91 100755 --- a/test/py/ganeti.objects_unittest.py +++ b/test/py/ganeti.objects_unittest.py @@ -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) -- GitLab