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 <[email protected]> --- 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 <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
