Change in vdsm[master]: tool: change configure verb logic.

mtayer at redhat.com mtayer at redhat.com
Mon Oct 13 23:55:40 UTC 2014


mooli tayer has uploaded a new change for review.

Change subject: tool: change configure verb logic.
......................................................................

tool: change configure verb logic.

In the following case configure errors:
configure(isconfigured()=NO, validate()=False, force()= False).

It will raise InvalidConfig while it should configure.

I'm suggesting a new algorithm. IMO It is easier to understand
(though I cannot speak for others).

Added a test containing a matrix of all possible module values.

Change-Id: I166fbae855ebff93ef84270f11b74f32fcc115c2
Signed-off-by: Mooli Tayer <mtayer at redhat.com>
---
M lib/vdsm/tool/configurator.py
M tests/toolTests.py
2 files changed, 43 insertions(+), 10 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/00/34100/1

diff --git a/lib/vdsm/tool/configurator.py b/lib/vdsm/tool/configurator.py
index 83be809..a768dc7 100644
--- a/lib/vdsm/tool/configurator.py
+++ b/lib/vdsm/tool/configurator.py
@@ -108,6 +108,20 @@
         setattr(_CONFIGURATORS[module], 'name', module)
 
 
+def filter_(c, args):
+    ret = True
+    configured = getattr(c, 'isconfigured', lambda: configurators.NO)()
+    configure_allowed = (configured != configurators.YES and
+                         (configured == configurators.NO or args.force))
+    if getattr(c, 'validate', lambda: True)():
+        ret = configure_allowed
+    elif not configure_allowed:
+        raise configurators.InvalidConfig(
+            "Confiuration of %s is invalid" % c.name
+        )
+    return ret
+
+
 @expose("configure")
 @requiresRoot
 def configure(*args):
@@ -117,18 +131,9 @@
     Invoke with -h for complete usage.
     """
     args = _parse_args(*args)
-    configurer_to_trigger = []
 
     sys.stdout.write("\nChecking configuration status...\n\n")
-    for c in args.modules:
-        isconfigured = getattr(c, 'isconfigured', lambda: configurators.NO)()
-        override = args.force and isconfigured != configurators.YES
-        if not override and not getattr(c, 'validate', lambda: True)():
-            raise configurators.InvalidConfig(
-                "Configuration of %s is invalid" % c.name
-            )
-        if override or isconfigured == configurators.NO:
-            configurer_to_trigger.append(c)
+    configurer_to_trigger = [c for c in args.modules if filter_(c, args)]
 
     services = []
     for c in configurer_to_trigger:
diff --git a/tests/toolTests.py b/tests/toolTests.py
index 22fe735..690e7ed 100644
--- a/tests/toolTests.py
+++ b/tests/toolTests.py
@@ -235,6 +235,34 @@
             'remove-config',
         )
 
+    def testConfigureFiltering(self):
+        class Thing(object):
+            pass
+        c, args = Thing(), Thing()
+        setattr(c, 'name', lambda: "Mock")
+
+        for (isconfigured, isvalid, force, expected) in (
+            (YES,          True,    True,  False),
+            (YES,          True,    False, False),
+            (YES,          False,   True,  InvalidConfig),
+            (YES,          False,   False, InvalidConfig),
+            (NO,           True,    True,  True),
+            (NO,           True,    False, True),
+            (NO,           False,   True,  True),
+            (NO,           False,   False, True),
+            (MAYBE,        True,    True,  True),
+            (MAYBE,        True,    False, False),
+            (MAYBE,        False,   True,  True),
+            (MAYBE,        False,   False, InvalidConfig),
+        ):
+            setattr(c, 'isconfigured', lambda: isconfigured)
+            setattr(c, 'validate', lambda: isvalid)
+            setattr(args, 'force', force)
+            if isinstance(expected, bool):
+                self.assertEquals(configurator.filter_(c, args), expected)
+            else:
+                self.assertRaises(InvalidConfig, configurator.filter_, c, args)
+
 
 class LibvirtModuleConfigureTests(TestCase):
 


-- 
To view, visit http://gerrit.ovirt.org/34100
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I166fbae855ebff93ef84270f11b74f32fcc115c2
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: mooli tayer <mtayer at redhat.com>


More information about the vdsm-patches mailing list