diff --git a/lib/objects.py b/lib/objects.py index 9be79acba12be599564984e7b9f2a59442cd53a2..ebf6fc00cdf7ac113e8b95804060f0984ab1a044 100644 --- a/lib/objects.py +++ b/lib/objects.py @@ -942,14 +942,7 @@ class InstancePolicy(ConfigObject): @raise errors.ConfigurationError: when the policy is not legal """ - 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) + InstancePolicy.CheckISpecSyntax(ipolicy, check_std) if constants.IPOLICY_DTS in ipolicy: InstancePolicy.CheckDiskTemplates(ipolicy[constants.IPOLICY_DTS]) for key in constants.IPOLICY_PARAMETERS: @@ -961,7 +954,35 @@ class InstancePolicy(ConfigObject): utils.CommaJoin(wrong_keys)) @classmethod - def CheckISpecSyntax(cls, minmaxspecs, stdspec, name, check_std): + def CheckISpecSyntax(cls, ipolicy, check_std): + """Check the instance policy specs for validity. + + @type ipolicy: dict + @param ipolicy: dictionary with min/max/std specs + @type check_std: bool + @param check_std: Whether to check std value or just assume compliance + @raise errors.ConfigurationError: when specs are not valid + + """ + if constants.ISPECS_MINMAX not in ipolicy: + # Nothing to check + return + + 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) + missing = constants.ISPECS_MINMAX_KEYS - frozenset(minmaxspecs.keys()) + if missing: + msg = "Missing instance specification: %s" % utils.CommaJoin(missing) + raise errors.ConfigurationError(msg) + for param in constants.ISPECS_PARAMETERS: + InstancePolicy._CheckISpecParamSyntax(minmaxspecs, stdspec, param, + check_std) + + @classmethod + def _CheckISpecParamSyntax(cls, minmaxspecs, stdspec, name, check_std): """Check the instance policy specs for validity on a given key. We check if the instance specs makes sense for a given key, that is @@ -979,11 +1000,6 @@ class InstancePolicy(ConfigObject): valid """ - 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) diff --git a/test/py/ganeti.objects_unittest.py b/test/py/ganeti.objects_unittest.py index 720b22a91d5e145e24be9624f30d1888f8140e7f..4bc7f95a666e9710545ed6c2c5d00489689d842c 100755 --- a/test/py/ganeti.objects_unittest.py +++ b/test/py/ganeti.objects_unittest.py @@ -428,17 +428,49 @@ class TestInstancePolicy(unittest.TestCase): self._AssertIPolicyIsFull(constants.IPOLICY_DEFAULTS) def testCheckISpecSyntax(self): + incomplete_ipolicies = [ + { + constants.ISPECS_MINMAX: {}, + constants.ISPECS_STD: NotImplemented, + }, + { + constants.ISPECS_MINMAX: { + constants.ISPECS_MIN: NotImplemented, + }, + constants.ISPECS_STD: NotImplemented, + }, + { + constants.ISPECS_MINMAX: { + constants.ISPECS_MAX: NotImplemented, + }, + constants.ISPECS_STD: NotImplemented, + }, + { + constants.ISPECS_MINMAX: { + constants.ISPECS_MIN: NotImplemented, + constants.ISPECS_MAX: NotImplemented, + }, + }, + ] + for ipol in incomplete_ipolicies: + self.assertRaises(errors.ConfigurationError, + objects.InstancePolicy.CheckISpecSyntax, + ipol, True) + + 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.CheckISpecSyntax(minmax, {}, par, check_std) + 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.CheckISpecSyntax(minmax, stdspec, par, check_std) + objects.InstancePolicy._CheckISpecParamSyntax(minmax, stdspec, par, + check_std) # Min and max only good_values = [(11, 11), (11, 40), (0, 0)] @@ -446,12 +478,13 @@ class TestInstancePolicy(unittest.TestCase): 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) + objects.InstancePolicy._CheckISpecParamSyntax(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, + objects.InstancePolicy._CheckISpecParamSyntax, minmax, {}, par, check_std) # Min, std, max good_values = [ @@ -465,7 +498,7 @@ class TestInstancePolicy(unittest.TestCase): constants.ISPECS_MAX: {par: mx}, } stdspec = {par: st} - objects.InstancePolicy.CheckISpecSyntax(minmax, stdspec, par, True) + objects.InstancePolicy._CheckISpecParamSyntax(minmax, stdspec, par, True) bad_values = [ (11, 11, 5), (40, 11, 11), @@ -481,7 +514,7 @@ class TestInstancePolicy(unittest.TestCase): } stdspec = {par: st} self.assertRaises(errors.ConfigurationError, - objects.InstancePolicy.CheckISpecSyntax, + objects.InstancePolicy._CheckISpecParamSyntax, minmax, stdspec, par, True) def testCheckDiskTemplates(self):