From 0f511c8a0c558f214a601273f15b8658fc258a4e Mon Sep 17 00:00:00 2001
From: Bernardo Dal Seno <bdalseno@google.com>
Date: Mon, 8 Apr 2013 20:36:34 +0200
Subject: [PATCH] Limit specs in instance policies are always complete
Specs used to specify limits in instance policies are always complete, even
at group level (only the whole limit set can be overridden). This is in
preparation for introducing multiple limits.
Signed-off-by: Bernardo Dal Seno <bdalseno@google.com>
Reviewed-by: Helga Velroyen <helgav@google.com>
---
lib/cli.py | 2 +-
lib/cmdlib.py | 70 ++++++--------
lib/objects.py | 59 +++++-------
src/Ganeti/Objects.hs | 39 ++------
test/hs/Test/Ganeti/Objects.hs | 3 +-
test/py/ganeti.cmdlib_unittest.py | 146 ++++++++++++++++++-----------
test/py/ganeti.config_unittest.py | 76 +++++++++------
test/py/ganeti.objects_unittest.py | 40 --------
8 files changed, 201 insertions(+), 234 deletions(-)
diff --git a/lib/cli.py b/lib/cli.py
index 2113308d1..6152ed7ce 100644
--- a/lib/cli.py
+++ b/lib/cli.py
@@ -3731,7 +3731,7 @@ def FormatPolicyInfo(custom_ipolicy, eff_ipolicy, iscluster):
if iscluster:
eff_ipolicy = custom_ipolicy
- custom_minmax = custom_ipolicy.get(constants.ISPECS_MINMAX)
+ custom_minmax = custom_ipolicy.get(constants.ISPECS_MINMAX, {})
ret = [
(key,
FormatParamsDictInfo(custom_minmax.get(key, {}),
diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index d6f16d07d..19898e730 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -813,20 +813,6 @@ 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 an instance policy.
@@ -834,41 +820,43 @@ def _GetUpdatedIPolicy(old_ipolicy, new_ipolicy, group_policy=False):
we should support removal of policy entries
"""
- use_none = use_default = group_policy
ipolicy = copy.deepcopy(old_ipolicy)
for key, value in new_ipolicy.items():
if key not in constants.IPOLICY_ALL_KEYS:
raise errors.OpPrereqError("Invalid key in new ipolicy: %s" % key,
errors.ECODE_INVAL)
- 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)
- utils.ForceDictType(ipolicy[key], constants.ISPECS_PARAMETER_TYPES)
+ if (not value or value == [constants.VALUE_DEFAULT] or
+ value == constants.VALUE_DEFAULT):
+ if group_policy:
+ del ipolicy[key]
+ else:
+ raise errors.OpPrereqError("Can't unset ipolicy attribute '%s'"
+ " on the cluster'" % key,
+ errors.ECODE_INVAL)
else:
- if (not value or value == [constants.VALUE_DEFAULT] or
- value == constants.VALUE_DEFAULT):
+ if key in constants.IPOLICY_PARAMETERS:
+ # FIXME: we assume all such values are float
+ try:
+ ipolicy[key] = float(value)
+ except (TypeError, ValueError), err:
+ raise errors.OpPrereqError("Invalid value for attribute"
+ " '%s': '%s', error: %s" %
+ (key, value, err), errors.ECODE_INVAL)
+ elif key == constants.ISPECS_MINMAX:
+ for k in value.keys():
+ utils.ForceDictType(value[k], constants.ISPECS_PARAMETER_TYPES)
+ ipolicy[key] = value
+ elif key == constants.ISPECS_STD:
if group_policy:
- del ipolicy[key]
- else:
- raise errors.OpPrereqError("Can't unset ipolicy attribute '%s'"
- " on the cluster'" % key,
- errors.ECODE_INVAL)
+ msg = "%s cannot appear in group instance specs" % key
+ raise errors.OpPrereqError(msg, errors.ECODE_INVAL)
+ ipolicy[key] = _GetUpdatedParams(old_ipolicy.get(key, {}), value,
+ use_none=False, use_default=False)
+ utils.ForceDictType(ipolicy[key], constants.ISPECS_PARAMETER_TYPES)
else:
- if key in constants.IPOLICY_PARAMETERS:
- # FIXME: we assume all such values are float
- try:
- ipolicy[key] = float(value)
- except (TypeError, ValueError), err:
- raise errors.OpPrereqError("Invalid value for attribute"
- " '%s': '%s', error: %s" %
- (key, value, err), errors.ECODE_INVAL)
- else:
- # FIXME: we assume all others are lists; this should be redone
- # in a nicer way
- ipolicy[key] = list(value)
+ # FIXME: we assume all others are lists; this should be redone
+ # in a nicer way
+ ipolicy[key] = list(value)
try:
objects.InstancePolicy.CheckParameterSyntax(ipolicy, not group_policy)
except errors.ConfigurationError, err:
diff --git a/lib/objects.py b/lib/objects.py
index ebf6fc00c..282aece4d 100644
--- a/lib/objects.py
+++ b/lib/objects.py
@@ -82,35 +82,17 @@ def FillDict(defaults_dict, custom_dict, skip_keys=None):
return ret_dict
-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 = {}
- # 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]))
- # other items which we know we can directly copy (immutables)
- for key in constants.IPOLICY_PARAMETERS:
- ret_dict[key] = custom_ipolicy.get(key, default_ipolicy[key])
-
+ ret_dict = copy.deepcopy(custom_ipolicy)
+ for key in default_ipolicy:
+ if key not in ret_dict:
+ ret_dict[key] = copy.deepcopy(default_ipolicy[key])
+ elif key == constants.ISPECS_STD:
+ ret_dict[key] = FillDict(default_ipolicy[key], ret_dict[key])
return ret_dict
@@ -198,13 +180,7 @@ def MakeEmptyIPolicy():
"""Create empty IPolicy dictionary.
"""
- return {
- constants.ISPECS_MINMAX: {
- constants.ISPECS_MIN: {},
- constants.ISPECS_MAX: {},
- },
- constants.ISPECS_STD: {},
- }
+ return {}
class ConfigObject(outils.ValidatedSlots):
@@ -953,6 +929,14 @@ class InstancePolicy(ConfigObject):
raise errors.ConfigurationError("Invalid keys in ipolicy: %s" %
utils.CommaJoin(wrong_keys))
+ @classmethod
+ def _CheckIncompleteSpec(cls, spec, keyname):
+ missing_params = constants.ISPECS_PARAMETERS - frozenset(spec.keys())
+ if missing_params:
+ msg = ("Missing instance specs parameters for %s: %s" %
+ (keyname, utils.CommaJoin(missing_params)))
+ raise errors.ConfigurationError(msg)
+
@classmethod
def CheckISpecSyntax(cls, ipolicy, check_std):
"""Check the instance policy specs for validity.
@@ -977,6 +961,10 @@ class InstancePolicy(ConfigObject):
if missing:
msg = "Missing instance specification: %s" % utils.CommaJoin(missing)
raise errors.ConfigurationError(msg)
+ for (key, spec) in minmaxspecs.items():
+ InstancePolicy._CheckIncompleteSpec(spec, key)
+ if check_std:
+ InstancePolicy._CheckIncompleteSpec(stdspec, constants.ISPECS_STD)
for param in constants.ISPECS_PARAMETERS:
InstancePolicy._CheckISpecParamSyntax(minmaxspecs, stdspec, param,
check_std)
@@ -1002,7 +990,7 @@ class InstancePolicy(ConfigObject):
"""
minspec = minmaxspecs[constants.ISPECS_MIN]
maxspec = minmaxspecs[constants.ISPECS_MAX]
- min_v = minspec.get(name, 0)
+ min_v = minspec[name]
if check_std:
std_v = stdspec.get(name, min_v)
@@ -1011,13 +999,10 @@ class InstancePolicy(ConfigObject):
std_v = min_v
std_msg = "-"
- max_v = maxspec.get(name, std_v)
+ max_v = maxspec[name]
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))
+ (name, min_v, max_v, std_msg))
raise errors.ConfigurationError(err)
@classmethod
diff --git a/src/Ganeti/Objects.hs b/src/Ganeti/Objects.hs
index d6cd12488..908daecf6 100644
--- a/src/Ganeti/Objects.hs
+++ b/src/Ganeti/Objects.hs
@@ -66,9 +66,7 @@ module Ganeti.Objects
, PartialISpecParams(..)
, fillISpecParams
, allISpecParamFields
- , FilledMinMaxISpecs(..)
- , PartialMinMaxISpecs(..)
- , fillMinMaxISpecs
+ , MinMaxISpecs(..)
, FilledIPolicy(..)
, PartialIPolicy(..)
, fillIPolicy
@@ -505,16 +503,7 @@ $(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"
+$(buildObject "MinMaxISpecs" "mmis"
[ renameField "MinSpec" $ simpleField "min" [t| FilledISpecParams |]
, renameField "MaxSpec" $ simpleField "max" [t| FilledISpecParams |]
])
@@ -523,8 +512,9 @@ $(buildObject "FilledMinMaxISpecs" "mmis"
-- has a special 2-level inheritance mode.
$(buildObject "PartialIPolicy" "ipolicy"
[ optionalField . renameField "MinMaxISpecsP"
- $ simpleField C.ispecsMinmax [t| PartialMinMaxISpecs |]
- , renameField "StdSpecP" $ simpleField "std" [t| PartialISpecParams |]
+ $ simpleField C.ispecsMinmax [t| MinMaxISpecs |]
+ , optionalField . renameField "StdSpecP"
+ $ simpleField "std" [t| PartialISpecParams |]
, optionalField . renameField "SpindleRatioP"
$ simpleField "spindle-ratio" [t| Double |]
, optionalField . renameField "VcpuRatioP"
@@ -537,24 +527,13 @@ $(buildObject "PartialIPolicy" "ipolicy"
-- has a special 2-level inheritance mode.
$(buildObject "FilledIPolicy" "ipolicy"
[ renameField "MinMaxISpecs"
- $ simpleField C.ispecsMinmax [t| FilledMinMaxISpecs |]
+ $ simpleField C.ispecsMinmax [t| MinMaxISpecs |]
, 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 { ipolicyMinMaxISpecs = fminmax
@@ -567,8 +546,10 @@ fillIPolicy (FilledIPolicy { ipolicyMinMaxISpecs = fminmax
, ipolicySpindleRatioP = pspindleRatio
, ipolicyVcpuRatioP = pvcpuRatio
, ipolicyDiskTemplatesP = pdiskTemplates}) =
- FilledIPolicy { ipolicyMinMaxISpecs = fillMinMaxISpecs fminmax pminmax
- , ipolicyStdSpec = fillISpecParams fstd pstd
+ FilledIPolicy { ipolicyMinMaxISpecs = fromMaybe fminmax pminmax
+ , ipolicyStdSpec = case pstd of
+ Nothing -> fstd
+ Just p -> fillISpecParams fstd p
, ipolicySpindleRatio = fromMaybe fspindleRatio pspindleRatio
, ipolicyVcpuRatio = fromMaybe fvcpuRatio pvcpuRatio
, ipolicyDiskTemplates = fromMaybe fdiskTemplates
diff --git a/test/hs/Test/Ganeti/Objects.hs b/test/hs/Test/Ganeti/Objects.hs
index 81bf3fd98..23ea054eb 100644
--- a/test/hs/Test/Ganeti/Objects.hs
+++ b/test/hs/Test/Ganeti/Objects.hs
@@ -143,14 +143,13 @@ 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 ''MinMaxISpecs)
$(genArbitrary ''FilledIPolicy)
$(genArbitrary ''IpFamily)
$(genArbitrary ''FilledNDParams)
diff --git a/test/py/ganeti.cmdlib_unittest.py b/test/py/ganeti.cmdlib_unittest.py
index 5603e4ae3..68ee95e27 100755
--- a/test/py/ganeti.cmdlib_unittest.py
+++ b/test/py/ganeti.cmdlib_unittest.py
@@ -1733,81 +1733,81 @@ class TestGetUpdatedIPolicy(unittest.TestCase):
"""Tests for cmdlib._GetUpdatedIPolicy()"""
_OLD_CLUSTER_POLICY = {
constants.IPOLICY_VCPU_RATIO: 1.5,
- constants.ISPECS_MINMAX: {
- constants.ISPECS_MIN: {
- constants.ISPEC_MEM_SIZE: 20,
- constants.ISPEC_CPU_COUNT: 2,
- },
- constants.ISPECS_MAX: {},
- },
- constants.ISPECS_STD: {},
+ constants.ISPECS_MINMAX: constants.ISPECS_MINMAX_DEFAULTS,
+ constants.ISPECS_STD: constants.IPOLICY_DEFAULTS[constants.ISPECS_STD],
}
_OLD_GROUP_POLICY = {
constants.IPOLICY_SPINDLE_RATIO: 2.5,
constants.ISPECS_MINMAX: {
constants.ISPECS_MIN: {
- constants.ISPEC_DISK_SIZE: 20,
- constants.ISPEC_NIC_COUNT: 2,
+ constants.ISPEC_MEM_SIZE: 128,
+ constants.ISPEC_CPU_COUNT: 1,
+ constants.ISPEC_DISK_COUNT: 1,
+ constants.ISPEC_DISK_SIZE: 1024,
+ constants.ISPEC_NIC_COUNT: 1,
+ constants.ISPEC_SPINDLE_USE: 1,
+ },
+ constants.ISPECS_MAX: {
+ constants.ISPEC_MEM_SIZE: 32768,
+ constants.ISPEC_CPU_COUNT: 8,
+ constants.ISPEC_DISK_COUNT: 5,
+ constants.ISPEC_DISK_SIZE: 1024 * 1024,
+ constants.ISPEC_NIC_COUNT: 3,
+ constants.ISPEC_SPINDLE_USE: 12,
},
- constants.ISPECS_MAX: {},
},
}
def _TestSetSpecs(self, old_policy, isgroup):
- ispec_key = constants.ISPECS_MIN
- diff_ispec = {
- constants.ISPEC_MEM_SIZE: 50,
- constants.ISPEC_DISK_SIZE: 30,
+ diff_minmax = {
+ constants.ISPECS_MIN: {
+ constants.ISPEC_MEM_SIZE: 64,
+ constants.ISPEC_CPU_COUNT: 1,
+ constants.ISPEC_DISK_COUNT: 2,
+ constants.ISPEC_DISK_SIZE: 64,
+ constants.ISPEC_NIC_COUNT: 1,
+ constants.ISPEC_SPINDLE_USE: 1,
+ },
+ constants.ISPECS_MAX: {
+ constants.ISPEC_MEM_SIZE: 16384,
+ constants.ISPEC_CPU_COUNT: 10,
+ constants.ISPEC_DISK_COUNT: 12,
+ constants.ISPEC_DISK_SIZE: 1024,
+ constants.ISPEC_NIC_COUNT: 9,
+ constants.ISPEC_SPINDLE_USE: 18,
+ },
}
+ diff_std = {
+ constants.ISPEC_DISK_COUNT: 10,
+ constants.ISPEC_DISK_SIZE: 512,
+ }
diff_policy = {
- constants.ISPECS_MINMAX: {
- ispec_key: diff_ispec,
- },
+ constants.ISPECS_MINMAX: diff_minmax
}
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)
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])
+ self.assertEqual(new_policy[constants.ISPECS_MINMAX], diff_minmax)
for key in old_policy:
if not key in diff_policy:
self.assertTrue(key in new_policy)
self.assertEqual(new_policy[key], old_policy[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])
+ old_std = old_policy.get(constants.ISPECS_STD, {})
+ for key in old_std:
+ self.assertTrue(key in new_std)
+ if key not in diff_std:
+ self.assertEqual(new_std[key], old_std[key])
-
- def _TestSet(self, old_policy, isgroup):
- diff_policy = {
- constants.IPOLICY_VCPU_RATIO: 3,
- constants.IPOLICY_SPINDLE_RATIO: 1.9,
- }
+ def _TestSet(self, old_policy, diff_policy, isgroup):
new_policy = cmdlib._GetUpdatedIPolicy(old_policy, diff_policy,
group_policy=isgroup)
for key in diff_policy:
@@ -1819,9 +1819,15 @@ class TestGetUpdatedIPolicy(unittest.TestCase):
self.assertEqual(new_policy[key], old_policy[key])
def testSet(self):
- self._TestSet(self._OLD_GROUP_POLICY, True)
+ diff_policy = {
+ constants.IPOLICY_VCPU_RATIO: 3,
+ constants.IPOLICY_DTS: [constants.DT_FILE],
+ }
+ self._TestSet(self._OLD_GROUP_POLICY, diff_policy, True)
self._TestSetSpecs(self._OLD_GROUP_POLICY, True)
- self._TestSet(self._OLD_CLUSTER_POLICY, False)
+ self._TestSet({}, diff_policy, True)
+ self._TestSetSpecs({}, True)
+ self._TestSet(self._OLD_CLUSTER_POLICY, diff_policy, False)
self._TestSetSpecs(self._OLD_CLUSTER_POLICY, False)
def testUnset(self):
@@ -1838,9 +1844,13 @@ class TestGetUpdatedIPolicy(unittest.TestCase):
self.assertTrue(key in new_policy)
self.assertEqual(new_policy[key], old_policy[key])
+ self.assertRaises(errors.OpPrereqError, cmdlib._GetUpdatedIPolicy,
+ old_policy, diff_policy, group_policy=False)
+
def _TestInvalidKeys(self, old_policy, isgroup):
+ INVALID_KEY = "this_key_shouldnt_be_allowed"
INVALID_DICT = {
- "this_key_shouldnt_be_allowed": 3,
+ INVALID_KEY: 3,
}
invalid_policy = INVALID_DICT
self.assertRaises(errors.OpPrereqError, cmdlib._GetUpdatedIPolicy,
@@ -1848,21 +1858,47 @@ class TestGetUpdatedIPolicy(unittest.TestCase):
invalid_ispecs = {
constants.ISPECS_MINMAX: INVALID_DICT,
}
- self.assertRaises(errors.OpPrereqError, cmdlib._GetUpdatedIPolicy,
+ self.assertRaises(errors.TypeEnforcementError, cmdlib._GetUpdatedIPolicy,
old_policy, invalid_ispecs, group_policy=isgroup)
- for key in constants.ISPECS_MINMAX_KEYS:
- invalid_ispec = {
- constants.ISPECS_MINMAX: {
- key: INVALID_DICT,
- },
+ if isgroup:
+ invalid_for_group = {
+ constants.ISPECS_STD: constants.IPOLICY_DEFAULTS[constants.ISPECS_STD],
}
+ self.assertRaises(errors.OpPrereqError, cmdlib._GetUpdatedIPolicy,
+ old_policy, invalid_for_group, group_policy=isgroup)
+ good_ispecs = self._OLD_CLUSTER_POLICY[constants.ISPECS_MINMAX]
+ invalid_ispecs = copy.deepcopy(good_ispecs)
+ invalid_policy = {
+ constants.ISPECS_MINMAX: invalid_ispecs,
+ }
+ for key in constants.ISPECS_MINMAX_KEYS:
+ ispec = invalid_ispecs[key]
+ ispec[INVALID_KEY] = None
self.assertRaises(errors.TypeEnforcementError, cmdlib._GetUpdatedIPolicy,
- old_policy, invalid_ispec, group_policy=isgroup)
+ old_policy, invalid_policy, group_policy=isgroup)
+ del ispec[INVALID_KEY]
+ for par in constants.ISPECS_PARAMETERS:
+ oldv = ispec[par]
+ ispec[par] = "this_is_not_good"
+ self.assertRaises(errors.TypeEnforcementError,
+ cmdlib._GetUpdatedIPolicy,
+ old_policy, invalid_policy, group_policy=isgroup)
+ ispec[par] = oldv
+ # This is to make sure that no two errors were present during the tests
+ cmdlib._GetUpdatedIPolicy(old_policy, invalid_policy, group_policy=isgroup)
def testInvalidKeys(self):
self._TestInvalidKeys(self._OLD_GROUP_POLICY, True)
self._TestInvalidKeys(self._OLD_CLUSTER_POLICY, False)
+ def testInvalidValues(self):
+ for par in (constants.IPOLICY_PARAMETERS |
+ frozenset([constants.IPOLICY_DTS])):
+ bad_policy = {
+ par: "invalid_value",
+ }
+ self.assertRaises(errors.OpPrereqError, cmdlib._GetUpdatedIPolicy, {},
+ bad_policy, group_policy=True)
if __name__ == "__main__":
testutils.GanetiTestProgram()
diff --git a/test/py/ganeti.config_unittest.py b/test/py/ganeti.config_unittest.py
index 71d60e6cc..7eff2e6d1 100755
--- a/test/py/ganeti.config_unittest.py
+++ b/test/py/ganeti.config_unittest.py
@@ -436,11 +436,9 @@ class TestConfigRunner(unittest.TestCase):
# depending on the owner (cluster or group)
if isgroup:
errs = cfg.VerifyConfig()
- # FIXME: A bug in FillIPolicy (issue 401) makes this test fail, so we
- # invert the assertions for the time being
- self.assertFalse(len(errs) >= 1)
+ self.assertTrue(len(errs) >= 1)
errstr = "%s has invalid instance policy" % ipowner
- self.assertFalse(_IsErrorInList(errstr, errs))
+ self.assertTrue(_IsErrorInList(errstr, errs))
else:
self.assertRaises(AssertionError, cfg.VerifyConfig)
del ipolicy[INVALID_KEY]
@@ -461,13 +459,17 @@ class TestConfigRunner(unittest.TestCase):
else:
del ipolicy[key]
- ispeclist = [
- (ipolicy[constants.ISPECS_MINMAX][constants.ISPECS_MIN],
- "%s/%s" % (constants.ISPECS_MINMAX, constants.ISPECS_MIN)),
- (ipolicy[constants.ISPECS_MINMAX][constants.ISPECS_MAX],
- "%s/%s" % (constants.ISPECS_MINMAX, constants.ISPECS_MAX)),
- (ipolicy[constants.ISPECS_STD], constants.ISPECS_STD),
- ]
+ ispeclist = []
+ if constants.ISPECS_MINMAX in ipolicy:
+ ispeclist.extend([
+ (ipolicy[constants.ISPECS_MINMAX][constants.ISPECS_MIN],
+ "%s/%s" % (constants.ISPECS_MINMAX, constants.ISPECS_MIN)),
+ (ipolicy[constants.ISPECS_MINMAX][constants.ISPECS_MAX],
+ "%s/%s" % (constants.ISPECS_MINMAX, constants.ISPECS_MAX)),
+ ])
+ if constants.ISPECS_STD in ipolicy:
+ ispeclist.append((ipolicy[constants.ISPECS_STD], constants.ISPECS_STD))
+
for (ispec, ispecpath) in ispeclist:
ispec[INVALID_KEY] = None
errs = cfg.VerifyConfig()
@@ -494,6 +496,28 @@ class TestConfigRunner(unittest.TestCase):
errs = cfg.VerifyConfig()
self.assertFalse(errs)
+ if constants.ISPECS_MINMAX in ipolicy:
+ # Test partial minmax specs
+ minmax = ipolicy[constants.ISPECS_MINMAX]
+ for key in constants.ISPECS_MINMAX_KEYS:
+ self.assertTrue(key in minmax)
+ ispec = minmax[key]
+ del minmax[key]
+ errs = cfg.VerifyConfig()
+ self.assertTrue(len(errs) >= 1)
+ self.assertTrue(_IsErrorInList("Missing instance specification", errs))
+ minmax[key] = ispec
+ for par in constants.ISPECS_PARAMETERS:
+ oldv = ispec[par]
+ del ispec[par]
+ errs = cfg.VerifyConfig()
+ self.assertTrue(len(errs) >= 1)
+ self.assertTrue(_IsErrorInList("Missing instance specs parameters",
+ errs))
+ ispec[par] = oldv
+ errs = cfg.VerifyConfig()
+ self.assertFalse(errs)
+
def _TestVerifyConfigGroupIPolicy(self, groupinfo, cfg):
old_ipolicy = groupinfo.ipolicy
ipolicy = cfg.GetClusterInfo().SimpleFillIPolicy({})
@@ -506,16 +530,6 @@ class TestConfigRunner(unittest.TestCase):
errs = cfg.VerifyConfig()
self.assertFalse(errs)
ipolicy[key] = oldv
- # Test partial minmax specs
- minmax = ipolicy[constants.ISPECS_MINMAX]
- for ispec_key in minmax.keys():
- ispec = minmax[ispec_key]
- for par in constants.ISPECS_PARAMETERS:
- oldv = ispec[par]
- del ispec[par]
- errs = cfg.VerifyConfig()
- self.assertFalse(errs)
- ispec[par] = oldv
groupinfo.ipolicy = old_ipolicy
def _TestVerifyConfigClusterIPolicy(self, ipolicy, cfg):
@@ -526,14 +540,18 @@ class TestConfigRunner(unittest.TestCase):
del ipolicy[key]
self.assertRaises(AssertionError, cfg.VerifyConfig)
ipolicy[key] = oldv
- # Test partial minmax specs
- minmax = ipolicy[constants.ISPECS_MINMAX]
- for key in constants.ISPECS_MINMAX_KEYS:
- self.assertTrue(key in minmax)
- oldv = minmax[key]
- del minmax[key]
- self.assertRaises(AssertionError, cfg.VerifyConfig)
- minmax[key] = oldv
+ errs = cfg.VerifyConfig()
+ self.assertFalse(errs)
+ # Partial standard specs
+ ispec = ipolicy[constants.ISPECS_STD]
+ for par in constants.ISPECS_PARAMETERS:
+ oldv = ispec[par]
+ del ispec[par]
+ errs = cfg.VerifyConfig()
+ self.assertTrue(len(errs) >= 1)
+ self.assertTrue(_IsErrorInList("Missing instance specs parameters",
+ errs))
+ ispec[par] = oldv
errs = cfg.VerifyConfig()
self.assertFalse(errs)
diff --git a/test/py/ganeti.objects_unittest.py b/test/py/ganeti.objects_unittest.py
index 4bc7f95a6..f1b211d5c 100755
--- a/test/py/ganeti.objects_unittest.py
+++ b/test/py/ganeti.objects_unittest.py
@@ -460,18 +460,6 @@ class TestInstancePolicy(unittest.TestCase):
def testCheckISpecParamSyntax(self):
par = "my_parameter"
for check_std in [True, False]:
- # Only one policy limit
- for key in constants.ISPECS_MINMAX_KEYS:
- minmax = dict((k, {}) for k in constants.ISPECS_MINMAX_KEYS)
- minmax[key][par] = 11
- objects.InstancePolicy._CheckISpecParamSyntax(minmax, {}, par,
- check_std)
- if check_std:
- minmax = dict((k, {}) for k in constants.ISPECS_MINMAX_KEYS)
- stdspec = {par: 11}
- objects.InstancePolicy._CheckISpecParamSyntax(minmax, stdspec, par,
- check_std)
-
# Min and max only
good_values = [(11, 11), (11, 40), (0, 0)]
for (mn, mx) in good_values:
@@ -588,34 +576,6 @@ class TestInstancePolicy(unittest.TestCase):
self._AssertIPolicyIsFull(policy)
self._AssertIPolicyMerged(constants.IPOLICY_DEFAULTS, diff_pol, policy)
- def testFillIPolicySpecs(self):
- 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_ipolicies:
- policy = objects.FillIPolicy(constants.IPOLICY_DEFAULTS, diff_pol)
- objects.InstancePolicy.CheckParameterSyntax(policy, True)
- self._AssertIPolicyIsFull(policy)
- self._AssertIPolicyMerged(constants.IPOLICY_DEFAULTS, diff_pol, policy)
-
if __name__ == "__main__":
testutils.GanetiTestProgram()
--
GitLab