Change in vdsm[ovirt-3.5]: vdsm-tool: suppoort dependencies between ModuleConfigure

mtayer at redhat.com mtayer at redhat.com
Mon Aug 25 14:04:45 UTC 2014


mooli tayer has uploaded a new change for review.

Change subject: vdsm-tool: suppoort dependencies between ModuleConfigure
......................................................................

vdsm-tool: suppoort dependencies between ModuleConfigure

Master patches were applied to this 3.5 patch[1][2][3]
[4][5].

[1] http://gerrit.ovirt.org/#/c/31735/
[2] http://gerrit.ovirt.org/#/c/31742/
[3] http://gerrit.ovirt.org/#/c/31738/
[4] http://gerrit.ovirt.org/#/c/31785/
[5] http://gerrit.ovirt.org/#/c/31739/

Change-Id: I6d5a8e92e61b518b6318d854a7c03251dc99ef33
Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1127877
Signed-off-by: Mooli Tayer <mtayer at redhat.com>
---
M lib/vdsm/tool/configurator.py
M lib/vdsm/tool/configurators/__init__.py
M tests/toolTests.py
3 files changed, 193 insertions(+), 12 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/13/31913/1

diff --git a/lib/vdsm/tool/configurator.py b/lib/vdsm/tool/configurator.py
index 3dfdb41..751fba2 100644
--- a/lib/vdsm/tool/configurator.py
+++ b/lib/vdsm/tool/configurator.py
@@ -17,11 +17,12 @@
 # Refer to the README and COPYING files for full details of the license
 #
 
+from collections import deque
 import argparse
 import sys
 import traceback
 
-from . import service, expose
+from . import service, expose, UsageError
 from .configurators import \
     CONFIGURED, \
     InvalidConfig, \
@@ -31,12 +32,10 @@
     sanlock
 
 
-__configurers = {
-    m.getName(): m for m in (
-        libvirt.Libvirt(),
-        sanlock.Sanlock()
-    )
-}
+_CONFIGURATORS = dict((m.getName(), m) for m in (
+    libvirt.Libvirt(),
+    sanlock.Sanlock()
+))
 
 
 @expose("configure")
@@ -168,20 +167,65 @@
         raise InvalidRun("Remove configuration failed")
 
 
+def _add_dependencies(modulesNames):
+    queue = deque(modulesNames)
+    retNames = set(queue)
+
+    while queue:
+        next_ = queue.popleft()
+        try:
+            requiredNames = _CONFIGURATORS[next_].getRequires()
+        except KeyError:
+            available = ', '.join(sorted(_CONFIGURATORS))
+            raise UsageError(
+                "error: argument --module: invalid choice: %s\n"
+                "(available: %s)\n" % (next_, available)
+            )
+
+        for requiredName in requiredNames:
+            if requiredName not in retNames:
+                retNames.add(requiredName)
+                queue.append(requiredName)
+
+    return retNames
+
+
+def _sort_modules(modulesNames):
+    # Greedy topological sort algorithm(Dijkstra).
+    # At each step go over all tasks and find a task that can be executed
+    # before all others. If at any point there is none, there is a circle!
+    # Note: there's an improved performance variant, but this is good enough.
+    modulesNames = set(modulesNames)
+    sortedModules = []
+    while modulesNames:
+
+        for c in modulesNames:
+            requires = _CONFIGURATORS[c].getRequires()
+            if requires.issubset(set(sortedModules)):
+                modulesNames.remove(c)
+                sortedModules.append(c)
+                break
+
+        else:
+            raise RuntimeError("Dependency circle found!")
+
+    return sortedModules
+
+
 def _parse_args(action, *args):
     parser = argparse.ArgumentParser('vdsm-tool %s' % (action))
     parser.add_argument(
         '--module',
         dest='modules',
-        choices=__configurers.keys(),
         default=[],
         metavar='STRING',
         action='append',
         help=(
             'Specify the module to run the action on '
-            '(e.g %(choices)s).\n'
+            '(e.g %s).\n'
             'If non is specified, operation will run for '
             'all related modules.'
+            % _CONFIGURATORS.keys()
         ),
     )
     if action == "configure":
@@ -192,8 +236,13 @@
             action='store_true',
             help='Force configuration, trigger services restart',
         )
+
     args = parser.parse_args(args)
     if not args.modules:
-        args.modules = __configurers.keys()
-    args.modules = [__configurers[cName] for cName in args.modules]
+        args.modules = _CONFIGURATORS.keys()
+
+    args.modules = _sort_modules(_add_dependencies(args.modules))
+
+    args.modules = [_CONFIGURATORS[cName] for cName in args.modules]
+
     return args
diff --git a/lib/vdsm/tool/configurators/__init__.py b/lib/vdsm/tool/configurators/__init__.py
index 41bc1d0..219d119 100644
--- a/lib/vdsm/tool/configurators/__init__.py
+++ b/lib/vdsm/tool/configurators/__init__.py
@@ -64,3 +64,6 @@
 
     def removeConf(self):
         pass
+
+    def getRequires(self):
+        return set()
diff --git a/tests/toolTests.py b/tests/toolTests.py
index ad08ba8..dd6551b 100644
--- a/tests/toolTests.py
+++ b/tests/toolTests.py
@@ -17,9 +17,14 @@
 #
 # Refer to the README and COPYING files for full details of the license
 #
-from vdsm.tool.configurators import NOT_CONFIGURED, NOT_SURE
+from vdsm.tool import configurator
+from vdsm.tool.configurators import \
+    ModuleConfigure, \
+    NOT_CONFIGURED,\
+    NOT_SURE
 from vdsm.tool.configurators.configfile import ConfigFile, ParserWrapper
 from vdsm.tool.configurators.libvirt import Libvirt
+from vdsm.tool import UsageError
 from vdsm.tool import upgrade
 from vdsm import utils
 import monkeypatch
@@ -31,6 +36,130 @@
 dirName = os.path.dirname(os.path.realpath(__file__))
 
 
+class MockModuleConfigurator(ModuleConfigure):
+
+    def __init__(self, name, dependencies):
+        self._name = name
+        self._dependencies = dependencies
+
+    def __repr__(self):
+        return "name: %s, dependencies: %s" % (self._name, self._dependencies)
+
+    def getName(self):
+        return self._name
+
+    def getRequires(self):
+        return self._dependencies
+
+
+class ConfiguratorTests(TestCase):
+
+    @monkeypatch.MonkeyPatch(
+        configurator,
+        '_CONFIGURATORS',
+        {
+            'a': MockModuleConfigurator('a', set(['b'])),
+            'b': MockModuleConfigurator('b', set(['a']))
+        }
+    )
+    def testDependencyCircle(self):
+        self.assertRaises(
+            RuntimeError,
+            configurator._parse_args,
+            'validate-config'
+        )
+
+    @monkeypatch.MonkeyPatch(
+        configurator,
+        '_CONFIGURATORS',
+        {
+            'a': MockModuleConfigurator('a', set(['b', 'd'])),
+            'b': MockModuleConfigurator('b', set(['c'])),
+            'c': MockModuleConfigurator('c', set(['e', 'd'])),
+            'd': MockModuleConfigurator('d', set(['e', 'e'])),
+            'e': MockModuleConfigurator('e', set()),
+            'f': MockModuleConfigurator('f', set()),
+
+        }
+    )
+    def testNormalDependencies(self):
+        modules = configurator._parse_args('validate-config').modules
+        # make sure this is indeed a topological sort.
+        before_m = set()
+        for m in modules:
+            for n in m.getRequires():
+                self.assertIn(n, before_m)
+            before_m.add(m.getName())
+
+    @monkeypatch.MonkeyPatch(
+        configurator,
+        '_CONFIGURATORS',
+        {
+            'a': MockModuleConfigurator('a', set()),
+            'b': MockModuleConfigurator('b', set()),
+            'c': MockModuleConfigurator('c', set())
+        }
+    )
+    def testNoDependencies(self):
+        configurator._parse_args('validate-config').modules
+
+    @monkeypatch.MonkeyPatch(
+        configurator,
+        '_CONFIGURATORS',
+        {
+            'a': MockModuleConfigurator('a', set(['b', 'c'])),
+            'b': MockModuleConfigurator('b', set()),
+            'c': MockModuleConfigurator('c', set())
+        }
+    )
+    def testDependenciesAdditionPositive(self):
+        modules = configurator._parse_args(
+            'validate-config',
+            '--module=a'
+        ).modules
+        modulesNames = [m.getName() for m in modules]
+        self.assertTrue('a' in modulesNames)
+        self.assertTrue('b' in modulesNames)
+        self.assertTrue('c' in modulesNames)
+
+    @monkeypatch.MonkeyPatch(
+        configurator,
+        '_CONFIGURATORS',
+        {
+            'a': MockModuleConfigurator('a', set()),
+            'b': MockModuleConfigurator('b', set()),
+            'c': MockModuleConfigurator('c', set())
+        }
+    )
+    def testDependenciesAdditionNegative(self):
+
+        modules = configurator._parse_args(
+            'validate-config',
+            '--module=a'
+        ).modules
+        moduleNames = [m.getName() for m in modules]
+        self.assertTrue('a' in moduleNames)
+        self.assertFalse('b' in moduleNames)
+        self.assertFalse('c' in moduleNames)
+
+    @monkeypatch.MonkeyPatch(
+        configurator,
+        '_CONFIGURATORS',
+        {
+            'libvirt': MockModuleConfigurator('libvirt', set()),
+            'sanlock': MockModuleConfigurator('sanlock', set()),
+        }
+    )
+    def testNonExistentModule(self):
+
+        self.assertRaises(
+            UsageError,
+            configurator._parse_args,
+            'validate-config',
+            '--module=multipath'
+        )
+
+
 class LibvirtModuleConfigureTests(TestCase):
 
     test_env = {}


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

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


More information about the vdsm-patches mailing list