From 0c2e59ac756f8de88aedfe718abb687b4cf5303b Mon Sep 17 00:00:00 2001
From: Iustin Pop <iustin@google.com>
Date: Mon, 14 May 2012 15:21:27 +0200
Subject: [PATCH] Beautify disk ipolicy violations in cluster-verify
Currently, we only get:
instance3: ['disk-size value 512 is not in range [1024, 1048576]'
which doesn't explain which disk we are talking about. This patch
extends the verification functions to take an additional parameter
that qualifies the disk:
instance3: ['disk-size/0 value 512 is not in range [1024, 1048576]'
Future patch will make the formatting of the list better.
Signed-off-by: Iustin Pop <iustin@google.com>
Reviewed-by: Michael Hanselmann <hansmi@google.com>
---
lib/cmdlib.py | 27 +++++++++++++++++----------
test/ganeti.cmdlib_unittest.py | 21 +++++++++++++--------
2 files changed, 30 insertions(+), 18 deletions(-)
diff --git a/lib/cmdlib.py b/lib/cmdlib.py
index d8f929f7d..18b2554d0 100644
--- a/lib/cmdlib.py
+++ b/lib/cmdlib.py
@@ -1116,10 +1116,12 @@ def _CheckInstanceState(lu, instance, req_states, msg=None):
(instance.name, msg), errors.ECODE_STATE)
-def _ComputeMinMaxSpec(name, ipolicy, value):
+def _ComputeMinMaxSpec(name, qualifier, ipolicy, value):
"""Computes if value is in the desired range.
@param name: name of the parameter for which we perform the check
+ @param qualifier: a qualifier used in the error message (e.g. 'disk/1',
+ not just 'disk')
@param ipolicy: dictionary containing min, max and std values
@param value: actual value that we want to use
@return: None or element not meeting the criteria
@@ -1131,8 +1133,12 @@ def _ComputeMinMaxSpec(name, ipolicy, value):
max_v = ipolicy[constants.ISPECS_MAX].get(name, value)
min_v = ipolicy[constants.ISPECS_MIN].get(name, value)
if value > max_v or min_v > value:
+ if qualifier:
+ fqn = "%s/%s" % (name, qualifier)
+ else:
+ fqn = name
return ("%s value %s is not in range [%s, %s]" %
- (name, value, min_v, max_v))
+ (fqn, value, min_v, max_v))
return None
@@ -1162,16 +1168,17 @@ def _ComputeIPolicySpecViolation(ipolicy, mem_size, cpu_count, disk_count,
assert disk_count == len(disk_sizes)
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),
- ] + map((lambda d: (constants.ISPEC_DISK_SIZE, d)), disk_sizes)
+ (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)]
return filter(None,
- (_compute_fn(name, ipolicy, value)
- for (name, value) in test_settings))
+ (_compute_fn(name, qualifier, ipolicy, value)
+ for (name, qualifier, value) in test_settings))
def _ComputeIPolicyInstanceViolation(ipolicy, instance,
diff --git a/test/ganeti.cmdlib_unittest.py b/test/ganeti.cmdlib_unittest.py
index b2aa6c7cb..485161f09 100755
--- a/test/ganeti.cmdlib_unittest.py
+++ b/test/ganeti.cmdlib_unittest.py
@@ -1,7 +1,7 @@
#!/usr/bin/python
#
-# Copyright (C) 2008, 2011 Google Inc.
+# Copyright (C) 2008, 2011, 2012 Google Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -586,24 +586,24 @@ class TestComputeMinMaxSpec(unittest.TestCase):
}
def testNoneValue(self):
- self.assertTrue(cmdlib._ComputeMinMaxSpec(constants.ISPEC_MEM_SIZE,
+ self.assertTrue(cmdlib._ComputeMinMaxSpec(constants.ISPEC_MEM_SIZE, None,
self.ipolicy, None) is None)
def testAutoValue(self):
- self.assertTrue(cmdlib._ComputeMinMaxSpec(constants.ISPEC_MEM_SIZE,
+ self.assertTrue(cmdlib._ComputeMinMaxSpec(constants.ISPEC_MEM_SIZE, None,
self.ipolicy,
constants.VALUE_AUTO) is None)
def testNotDefined(self):
- self.assertTrue(cmdlib._ComputeMinMaxSpec(constants.ISPEC_NIC_COUNT,
+ self.assertTrue(cmdlib._ComputeMinMaxSpec(constants.ISPEC_NIC_COUNT, None,
self.ipolicy, 3) is None)
def testNoMinDefined(self):
- self.assertTrue(cmdlib._ComputeMinMaxSpec(constants.ISPEC_DISK_SIZE,
+ self.assertTrue(cmdlib._ComputeMinMaxSpec(constants.ISPEC_DISK_SIZE, None,
self.ipolicy, 128) is None)
def testNoMaxDefined(self):
- self.assertTrue(cmdlib._ComputeMinMaxSpec(constants.ISPEC_DISK_COUNT,
+ self.assertTrue(cmdlib._ComputeMinMaxSpec(constants.ISPEC_DISK_COUNT, None,
self.ipolicy, 16) is None)
def testOutOfRange(self):
@@ -613,9 +613,14 @@ class TestComputeMinMaxSpec(unittest.TestCase):
(constants.ISPEC_DISK_COUNT, 0)):
min_v = self.ipolicy[constants.ISPECS_MIN].get(name, val)
max_v = self.ipolicy[constants.ISPECS_MAX].get(name, val)
- self.assertEqual(cmdlib._ComputeMinMaxSpec(name, self.ipolicy, val),
+ self.assertEqual(cmdlib._ComputeMinMaxSpec(name, None,
+ self.ipolicy, val),
"%s value %s is not in range [%s, %s]" %
(name, val,min_v, max_v))
+ self.assertEqual(cmdlib._ComputeMinMaxSpec(name, "1",
+ self.ipolicy, val),
+ "%s/1 value %s is not in range [%s, %s]" %
+ (name, val,min_v, max_v))
def test(self):
for (name, val) in ((constants.ISPEC_MEM_SIZE, 256),
@@ -625,7 +630,7 @@ class TestComputeMinMaxSpec(unittest.TestCase):
(constants.ISPEC_DISK_SIZE, 0),
(constants.ISPEC_DISK_COUNT, 1),
(constants.ISPEC_DISK_COUNT, 5)):
- self.assertTrue(cmdlib._ComputeMinMaxSpec(name, self.ipolicy, val)
+ self.assertTrue(cmdlib._ComputeMinMaxSpec(name, None, self.ipolicy, val)
is None)
--
GitLab