diff --git a/lib/cli.py b/lib/cli.py
index 2113308d146ce39600842314f0c48a231f1bfe2f..6152ed7ceb5536d86ac2582b48e5a1bbda62ebef 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 d6f16d07df5cd74351b24208de3af8c5782fdb95..19898e73057988fdca55180f3f75a79c2ea78f52 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 ebf6fc00cdf7ac113e8b95804060f0984ab1a044..282aece4d24c2ff5f61a10d609f89b68ad320e47 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 d6cd124889fb648220543f68c5e35e82ba74e2b5..908daecf63e24b47a16a039ec9bee6c0b80e6449 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 81bf3fd988ed8f6ee05d8dbe322afc1064f7c9e4..23ea054eb96c4ac3ff844b596b2786877055d287 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 5603e4ae34d3dff3b969c76496b3a42019607b4d..68ee95e27f70988906408afdc52e6f998f4584e3 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 71d60e6cc45ad0bfa8812db1f330b1f31756363a..7eff2e6d1a2836e270f77be6623d3cff6fee51db 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 4bc7f95a666e9710545ed6c2c5d00489689d842c..f1b211d5c895a1a4fd4c13cafae3c8f3ef494475 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()