Commit cc4b2676 authored by Bernardo Dal Seno's avatar Bernardo Dal Seno
Browse files

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: default avatarBernardo Dal Seno <bdalseno@google.com>
Reviewed-by: default avatarGuido Trotter <ultrotter@google.com>
parent 6a327093
......@@ -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
......
......@@ -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, [])
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment