From: Bernardo Dal Seno Date: Thu, 7 Mar 2013 00:46:14 +0000 (+0100) Subject: Fix policy check for disk templates X-Git-Tag: v2.7.0beta2~17 X-Git-Url: https://code.grnet.gr/git/ganeti-local/commitdiff_plain/cc4b26760f9e79bf98938857c24b37e6d3bed2e4 Fix policy check for disk templates Instance disk template is checked against the policy, and diskless instances aren't checked for the number of disks. Signed-off-by: Bernardo Dal Seno Reviewed-by: Guido Trotter --- diff --git a/lib/cmdlib.py b/lib/cmdlib.py index 765902e..e9a820b 100644 --- a/lib/cmdlib.py +++ b/lib/cmdlib.py @@ -1231,6 +1231,7 @@ def _ComputeMinMaxSpec(name, qualifier, ipolicy, value): def _ComputeIPolicySpecViolation(ipolicy, mem_size, cpu_count, disk_count, nic_count, disk_sizes, spindle_use, + disk_template, _compute_fn=_ComputeMinMaxSpec): """Verifies ipolicy against provided specs. @@ -1248,6 +1249,8 @@ def _ComputeIPolicySpecViolation(ipolicy, mem_size, cpu_count, disk_count, @param disk_sizes: Disk sizes of used disk (len must match C{disk_count}) @type spindle_use: int @param spindle_use: The number of spindles this instance uses + @type disk_template: string + @param disk_template: The disk template of the instance @param _compute_fn: The compute function (unittest only) @return: A list of violations, or an empty list of no violations are found @@ -1257,15 +1260,22 @@ def _ComputeIPolicySpecViolation(ipolicy, mem_size, cpu_count, disk_count, test_settings = [ (constants.ISPEC_MEM_SIZE, "", mem_size), (constants.ISPEC_CPU_COUNT, "", cpu_count), - (constants.ISPEC_DISK_COUNT, "", disk_count), (constants.ISPEC_NIC_COUNT, "", nic_count), (constants.ISPEC_SPINDLE_USE, "", spindle_use), ] + [(constants.ISPEC_DISK_SIZE, str(idx), d) for idx, d in enumerate(disk_sizes)] + if disk_template != constants.DT_DISKLESS: + # This check doesn't make sense for diskless instances + test_settings.append((constants.ISPEC_DISK_COUNT, "", disk_count)) + ret = [] + allowed_dts = ipolicy[constants.IPOLICY_DTS] + if disk_template not in allowed_dts: + ret.append("Disk template %s is not allowed (allowed templates: %s)" % + (disk_template, utils.CommaJoin(allowed_dts))) - return filter(None, - (_compute_fn(name, qualifier, ipolicy, value) - for (name, qualifier, value) in test_settings)) + return ret + filter(None, + (_compute_fn(name, qualifier, ipolicy, value) + for (name, qualifier, value) in test_settings)) def _ComputeIPolicyInstanceViolation(ipolicy, instance, @@ -1286,19 +1296,23 @@ def _ComputeIPolicyInstanceViolation(ipolicy, instance, disk_count = len(instance.disks) disk_sizes = [disk.size for disk in instance.disks] nic_count = len(instance.nics) + disk_template = instance.disk_template return _compute_fn(ipolicy, mem_size, cpu_count, disk_count, nic_count, - disk_sizes, spindle_use) + disk_sizes, spindle_use, disk_template) def _ComputeIPolicyInstanceSpecViolation( - ipolicy, instance_spec, _compute_fn=_ComputeIPolicySpecViolation): + ipolicy, instance_spec, disk_template, + _compute_fn=_ComputeIPolicySpecViolation): """Compute if instance specs meets the specs of ipolicy. @type ipolicy: dict @param ipolicy: The ipolicy to verify against @param instance_spec: dict @param instance_spec: The instance spec to verify + @type disk_template: string + @param disk_template: the disk template of the instance @param _compute_fn: The function to verify ipolicy (unittest only) @see: L{_ComputeIPolicySpecViolation} @@ -1311,7 +1325,7 @@ def _ComputeIPolicyInstanceSpecViolation( spindle_use = instance_spec.get(constants.ISPEC_SPINDLE_USE, None) return _compute_fn(ipolicy, mem_size, cpu_count, disk_count, nic_count, - disk_sizes, spindle_use) + disk_sizes, spindle_use, disk_template) def _ComputeIPolicyNodeViolation(ipolicy, instance, current_group, @@ -10863,7 +10877,8 @@ class LUInstanceCreate(LogicalUnit): group_info = self.cfg.GetNodeGroup(pnode.group) ipolicy = ganeti.masterd.instance.CalculateGroupIPolicy(cluster, group_info) - res = _ComputeIPolicyInstanceSpecViolation(ipolicy, ispec) + res = _ComputeIPolicyInstanceSpecViolation(ipolicy, ispec, + self.op.disk_template) if not self.op.ignore_ipolicy and res: msg = ("Instance allocation to group %s (%s) violates policy: %s" % (pnode.group, group_info.name, utils.CommaJoin(res))) @@ -13889,14 +13904,20 @@ class LUInstanceSetParams(LogicalUnit): None) # Copy ispec to verify parameters with min/max values separately + if self.op.disk_template: + new_disk_template = self.op.disk_template + else: + new_disk_template = instance.disk_template ispec_max = ispec.copy() ispec_max[constants.ISPEC_MEM_SIZE] = \ self.be_new.get(constants.BE_MAXMEM, None) - res_max = _ComputeIPolicyInstanceSpecViolation(ipolicy, ispec_max) + res_max = _ComputeIPolicyInstanceSpecViolation(ipolicy, ispec_max, + new_disk_template) ispec_min = ispec.copy() ispec_min[constants.ISPEC_MEM_SIZE] = \ self.be_new.get(constants.BE_MINMEM, None) - res_min = _ComputeIPolicyInstanceSpecViolation(ipolicy, ispec_min) + res_min = _ComputeIPolicyInstanceSpecViolation(ipolicy, ispec_min, + new_disk_template) if (res_max or res_min): # FIXME: Improve error message by including information about whether diff --git a/test/py/ganeti.cmdlib_unittest.py b/test/py/ganeti.cmdlib_unittest.py index e43b1fb..bcb574e 100755 --- a/test/py/ganeti.cmdlib_unittest.py +++ b/test/py/ganeti.cmdlib_unittest.py @@ -654,6 +654,13 @@ def _ValidateComputeMinMaxSpec(name, *_): return None +def _NoDiskComputeMinMaxSpec(name, *_): + if name == constants.ISPEC_DISK_COUNT: + return name + else: + return None + + class _SpecWrapper: def __init__(self, spec): self.spec = spec @@ -663,43 +670,75 @@ class _SpecWrapper: class TestComputeIPolicySpecViolation(unittest.TestCase): + # Minimal policy accepted by _ComputeIPolicySpecViolation() + _MICRO_IPOL = { + constants.IPOLICY_DTS: [constants.DT_PLAIN, constants.DT_DISKLESS], + } + def test(self): compute_fn = _ValidateComputeMinMaxSpec - ret = cmdlib._ComputeIPolicySpecViolation(NotImplemented, 1024, 1, 1, 1, - [1024], 1, _compute_fn=compute_fn) + ret = cmdlib._ComputeIPolicySpecViolation(self._MICRO_IPOL, 1024, 1, 1, 1, + [1024], 1, constants.DT_PLAIN, + _compute_fn=compute_fn) self.assertEqual(ret, []) + def testDiskFull(self): + compute_fn = _NoDiskComputeMinMaxSpec + ret = cmdlib._ComputeIPolicySpecViolation(self._MICRO_IPOL, 1024, 1, 1, 1, + [1024], 1, constants.DT_PLAIN, + _compute_fn=compute_fn) + self.assertEqual(ret, [constants.ISPEC_DISK_COUNT]) + + def testDiskLess(self): + compute_fn = _NoDiskComputeMinMaxSpec + ret = cmdlib._ComputeIPolicySpecViolation(self._MICRO_IPOL, 1024, 1, 1, 1, + [1024], 1, constants.DT_DISKLESS, + _compute_fn=compute_fn) + self.assertEqual(ret, []) + + def testWrongTemplates(self): + compute_fn = _ValidateComputeMinMaxSpec + ret = cmdlib._ComputeIPolicySpecViolation(self._MICRO_IPOL, 1024, 1, 1, 1, + [1024], 1, constants.DT_DRBD8, + _compute_fn=compute_fn) + self.assertEqual(len(ret), 1) + self.assertTrue("Disk template" in ret[0]) + def testInvalidArguments(self): self.assertRaises(AssertionError, cmdlib._ComputeIPolicySpecViolation, - NotImplemented, 1024, 1, 1, 1, [], 1) + self._MICRO_IPOL, 1024, 1, 1, 1, [], 1, + constants.DT_PLAIN,) def testInvalidSpec(self): spec = _SpecWrapper([None, False, "foo", None, "bar", None]) compute_fn = spec.ComputeMinMaxSpec - ret = cmdlib._ComputeIPolicySpecViolation(NotImplemented, 1024, 1, 1, 1, - [1024], 1, _compute_fn=compute_fn) + ret = cmdlib._ComputeIPolicySpecViolation(self._MICRO_IPOL, 1024, 1, 1, 1, + [1024], 1, constants.DT_PLAIN, + _compute_fn=compute_fn) self.assertEqual(ret, ["foo", "bar"]) self.assertFalse(spec.spec) class _StubComputeIPolicySpecViolation: def __init__(self, mem_size, cpu_count, disk_count, nic_count, disk_sizes, - spindle_use): + spindle_use, disk_template): self.mem_size = mem_size self.cpu_count = cpu_count self.disk_count = disk_count self.nic_count = nic_count self.disk_sizes = disk_sizes self.spindle_use = spindle_use + self.disk_template = disk_template def __call__(self, _, mem_size, cpu_count, disk_count, nic_count, disk_sizes, - spindle_use): + spindle_use, disk_template): assert self.mem_size == mem_size assert self.cpu_count == cpu_count assert self.disk_count == disk_count assert self.nic_count == nic_count assert self.disk_sizes == disk_sizes assert self.spindle_use == spindle_use + assert self.disk_template == disk_template return [] @@ -712,8 +751,10 @@ class TestComputeIPolicyInstanceViolation(unittest.TestCase): constants.BE_SPINDLE_USE: 4, } disks = [objects.Disk(size=512)] - instance = objects.Instance(beparams=beparams, disks=disks, nics=[]) - stub = _StubComputeIPolicySpecViolation(2048, 2, 1, 0, [512], 4) + instance = objects.Instance(beparams=beparams, disks=disks, nics=[], + disk_template=constants.DT_PLAIN) + stub = _StubComputeIPolicySpecViolation(2048, 2, 1, 0, [512], 4, + constants.DT_PLAIN) ret = cmdlib._ComputeIPolicyInstanceViolation(NotImplemented, instance, _compute_fn=stub) self.assertEqual(ret, []) @@ -729,8 +770,10 @@ class TestComputeIPolicyInstanceSpecViolation(unittest.TestCase): constants.ISPEC_NIC_COUNT: 0, constants.ISPEC_SPINDLE_USE: 1, } - stub = _StubComputeIPolicySpecViolation(2048, 2, 1, 0, [512], 1) + stub = _StubComputeIPolicySpecViolation(2048, 2, 1, 0, [512], 1, + constants.DT_PLAIN) ret = cmdlib._ComputeIPolicyInstanceSpecViolation(NotImplemented, ispec, + constants.DT_PLAIN, _compute_fn=stub) self.assertEqual(ret, [])