From 252ae5ea569f8a789f8605dc7f9e0be267d50d96 Mon Sep 17 00:00:00 2001 From: Steven Armstrong Date: Thu, 19 Jan 2012 07:51:02 +0100 Subject: [PATCH] new feature: dependency resolver --- doc/dev/todo/TAKEME | 1 - lib/cdist/config_install.py | 37 ++--- lib/cdist/core/object.py | 16 +- lib/cdist/emulator.py | 26 +--- lib/cdist/resolver.py | 139 ++++++++++++++++++ lib/cdist/test/emulator/__init__.py | 7 + lib/cdist/test/resolver/__init__.py | 88 +++++++++++ .../resolver/fixtures/object/__first/.keep | 0 .../object/__first/child/.cdist/.keep | 0 .../fixtures/object/__first/dog/.cdist/.keep | 0 .../fixtures/object/__first/man/.cdist/.keep | 0 .../object/__first/woman/.cdist/.keep | 0 .../resolver/fixtures/object/__second/.keep | 0 .../object/__second/on-the/.cdist/.keep | 0 .../object/__second/under-the/.cdist/.keep | 0 .../resolver/fixtures/object/__third/.keep | 0 .../fixtures/object/__third/moon/.cdist/.keep | 0 .../object/__third/moon/.cdist/parameter/name | 1 + .../__third/moon/.cdist/parameter/planet | 1 + .../test/resolver/fixtures/type/__first/.keep | 0 .../resolver/fixtures/type/__second/.keep | 0 .../test/resolver/fixtures/type/__third/.keep | 0 22 files changed, 271 insertions(+), 45 deletions(-) create mode 100644 lib/cdist/resolver.py create mode 100644 lib/cdist/test/resolver/__init__.py create mode 100644 lib/cdist/test/resolver/fixtures/object/__first/.keep create mode 100644 lib/cdist/test/resolver/fixtures/object/__first/child/.cdist/.keep create mode 100644 lib/cdist/test/resolver/fixtures/object/__first/dog/.cdist/.keep create mode 100644 lib/cdist/test/resolver/fixtures/object/__first/man/.cdist/.keep create mode 100644 lib/cdist/test/resolver/fixtures/object/__first/woman/.cdist/.keep create mode 100644 lib/cdist/test/resolver/fixtures/object/__second/.keep create mode 100644 lib/cdist/test/resolver/fixtures/object/__second/on-the/.cdist/.keep create mode 100644 lib/cdist/test/resolver/fixtures/object/__second/under-the/.cdist/.keep create mode 100644 lib/cdist/test/resolver/fixtures/object/__third/.keep create mode 100644 lib/cdist/test/resolver/fixtures/object/__third/moon/.cdist/.keep create mode 100644 lib/cdist/test/resolver/fixtures/object/__third/moon/.cdist/parameter/name create mode 100644 lib/cdist/test/resolver/fixtures/object/__third/moon/.cdist/parameter/planet create mode 100644 lib/cdist/test/resolver/fixtures/type/__first/.keep create mode 100644 lib/cdist/test/resolver/fixtures/type/__second/.keep create mode 100644 lib/cdist/test/resolver/fixtures/type/__third/.keep diff --git a/doc/dev/todo/TAKEME b/doc/dev/todo/TAKEME index 875d2970..25d088b9 100644 --- a/doc/dev/todo/TAKEME +++ b/doc/dev/todo/TAKEME @@ -3,7 +3,6 @@ UNASSIGNED TODOS The following list of todos has not been assigned to any developer. Feel free to pick one! - TESTS ----- - multiple defines of object: diff --git a/lib/cdist/config_install.py b/lib/cdist/config_install.py index 2d2ab949..542f2024 100644 --- a/lib/cdist/config_install.py +++ b/lib/cdist/config_install.py @@ -27,9 +27,12 @@ import shutil import sys import tempfile import time +import itertools +import pprint import cdist from cdist import core +from cdist import resolver class ConfigInstall(object): @@ -105,29 +108,12 @@ class ConfigInstall(object): def object_run(self, cdist_object): """Run gencode and code for an object""" self.log.debug("Trying to run object " + cdist_object.name) - if cdist_object.state == core.Object.STATE_RUNNING: - # FIXME: resolve dependency circle / show problem source - raise cdist.Error("Detected circular dependency in " + cdist_object.name) - elif cdist_object.state == core.Object.STATE_DONE: - self.log.debug("Ignoring run of already finished object %s", cdist_object) - return - else: - cdist_object.state = core.Object.STATE_RUNNING + if cdist_object.state == core.Object.STATE_DONE: + # TODO: remove once we are sure that this really never happens. + raise cdist.Error("Attempting to run an already finished object: %s", cdist_object) cdist_type = cdist_object.type - for requirement in cdist_object.requirements: - self.log.debug("Object %s requires %s", cdist_object, requirement) - required_object = cdist_object.object_from_name(requirement) - - # The user may have created dependencies without satisfying them - if not required_object.exists: - raise cdist.Error(cdist_object.name + " requires non-existing " + required_object.name) - else: - self.log.debug("Required object %s exists", required_object.name) - - self.object_run(required_object) - # Generate self.log.info("Generating and executing code for " + cdist_object.name) cdist_object.code_local = self.code.run_gencode_local(cdist_object) @@ -149,7 +135,14 @@ class ConfigInstall(object): def stage_run(self): """The final (and real) step of deployment""" self.log.info("Generating and executing code") - for cdist_object in core.Object.list_objects(self.local.object_path, - self.local.type_path): + + objects = core.Object.list_objects( + self.local.object_path, + self.local.type_path) + + dependency_resolver = resolver.DependencyResolver(objects) + self.log.debug(pprint.pformat(dependency_resolver.graph)) + + for cdist_object in dependency_resolver: self.log.debug("Run object: %s", cdist_object) self.object_run(cdist_object) diff --git a/lib/cdist/core/object.py b/lib/cdist/core/object.py index 9abb11eb..da2f21a6 100644 --- a/lib/cdist/core/object.py +++ b/lib/cdist/core/object.py @@ -96,12 +96,18 @@ class Object(object): """ return os.path.join(type_name, object_id) - def __init__(self, cdist_type, base_path, object_id=None): + @staticmethod + def validate_object_id(object_id): + """Validate the given object_id and raise IllegalObjectIdError if it's not valid. + """ if object_id: if object_id.startswith('/'): raise IllegalObjectIdError(object_id, 'object_id may not start with /') if OBJECT_MARKER in object_id.split(os.sep): raise IllegalObjectIdError(object_id, 'object_id may not contain \'%s\'' % OBJECT_MARKER) + + def __init__(self, cdist_type, base_path, object_id=None): + self.validate_object_id(object_id) self.type = cdist_type # instance of Type self.base_path = base_path self.object_id = object_id @@ -116,8 +122,12 @@ class Object(object): return '' % self.name def __eq__(self, other): - """define equality as 'attributes are the same'""" - return self.__dict__ == other.__dict__ + """define equality as 'name is the same'""" + return self.name == other.name + + def __hash__(self): + return hash(self.name) + def __lt__(self, other): return isinstance(other, self.__class__) and self.name < other.name diff --git a/lib/cdist/emulator.py b/lib/cdist/emulator.py index bb67e7ee..05202a39 100644 --- a/lib/cdist/emulator.py +++ b/lib/cdist/emulator.py @@ -154,30 +154,18 @@ class Emulator(object): if len(requirement) == 0: continue - self.log.debug("Recording requirement: " + requirement) - requirement_parts = requirement.split(os.sep, 1) - requirement_type_name = requirement_parts[0] - try: - requirement_object_id = requirement_parts[1] - except IndexError: - # no object id, assume singleton - requirement_object_id = 'singleton' - - # Remove leading / from object id - requirement_object_id = requirement_object_id.lstrip('/') - + requirement_type_name, requirement_object_id = core.Object.split_name(requirement) # Instantiate type which fails if type does not exist requirement_type = core.Type(self.type_base_path, requirement_type_name) - if requirement_object_id == 'singleton' \ - and not requirement_type.is_singleton: + if requirement_object_id: + # Validate object_id if any + core.Object.validate_object_id(requirement_object_id) + elif not requirement_type.is_singleton: + # Only singeltons have no object_id raise IllegalRequirementError(requirement, "Missing object_id and type is not a singleton.") - # Instantiate object which fails if the object_id is illegal - requirement_object = core.Object(requirement_type, self.object_base_path, requirement_object_id) - - # Construct cleaned up requirement with only one / :-) - requirement = requirement_type_name + '/' + requirement_object_id + self.log.debug("Recording requirement: " + requirement) self.cdist_object.requirements.append(requirement) # Record / Append source diff --git a/lib/cdist/resolver.py b/lib/cdist/resolver.py new file mode 100644 index 00000000..24a5e496 --- /dev/null +++ b/lib/cdist/resolver.py @@ -0,0 +1,139 @@ +# -*- coding: utf-8 -*- +# +# 2011 Steven Armstrong (steven-cdist at armstrong.cc) +# +# This file is part of cdist. +# +# cdist is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# cdist is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with cdist. If not, see . +# +# + +import logging +import os +import itertools +import fnmatch + +import cdist + +log = logging.getLogger(__name__) + + +class CircularReferenceError(cdist.Error): + def __init__(self, cdist_object, required_object): + self.cdist_object = cdist_object + self.required_object = required_object + + def __str__(self): + return 'Circular reference detected: %s -> %s' % (self.cdist_object.name, self.required_object.name) + + +class RequirementNotFoundError(cdist.Error): + def __init__(self, requirement): + self.requirement = requirement + + def __str__(self): + return 'Requirement could not be found: %s' % self.requirement + + +class DependencyResolver(object): + """Cdist's dependency resolver. + + Usage: + resolver = DependencyResolver(list_of_objects) + from pprint import pprint + pprint(resolver.graph) + + for cdist_object in resolver: + do_something_with(cdist_object) + + """ + def __init__(self, objects, logger=None): + self.objects = list(objects) # make sure we store as list, not generator + self._object_index = dict((o.name, o) for o in self.objects) + self._graph = None + self.log = logger or log + + @property + def graph(self): + """Build the dependency graph. + + Returns a dict where the keys are the object names and the values are + lists of all dependencies including the key object itself. + """ + if self._graph is None: + graph = {} + for o in self.objects: + resolved = [] + unresolved = [] + self.resolve_object_dependencies(o, resolved, unresolved) + graph[o.name] = resolved + self._graph = graph + return self._graph + + def find_requirements_by_name(self, requirements): + """Takes a list of requirement patterns and returns a list of matching object instances. + + Patterns are expected to be Unix shell-style wildcards for use with fnmatch.filter. + + find_requirements_by_name(['__type/object_id', '__other_type/*']) -> + [, , ] + """ + object_names = self._object_index.keys() + for pattern in requirements: + found = False + for requirement in fnmatch.filter(object_names, pattern): + found = True + yield self._object_index[requirement] + if not found: + # FIXME: get rid of the singleton object_id, it should be invisible to the code -> hide it in Object + singleton = os.path.join(pattern, 'singleton') + if singleton in self._object_index: + yield self._object_index[singleton] + else: + raise RequirementNotFoundError(pattern) + + def resolve_object_dependencies(self, cdist_object, resolved, unresolved): + """Resolve all dependencies for the given cdist_object and store them + in the list which is passed as the 'resolved' arguments. + + e.g. + resolved = [] + unresolved = [] + resolve_object_dependencies(some_object, resolved, unresolved) + print("Dependencies for %s: %s" % (some_object, resolved)) + """ + self.log.debug('Resolving dependencies for: %s' % cdist_object.name) + try: + unresolved.append(cdist_object) + for required_object in self.find_requirements_by_name(cdist_object.requirements): + self.log.debug("Object %s requires %s", cdist_object, required_object) + if required_object not in resolved: + if required_object in unresolved: + raise CircularReferenceError(cdist_object, required_object) + self.resolve_object_dependencies(required_object, resolved, unresolved) + resolved.append(cdist_object) + unresolved.remove(cdist_object) + except RequirementNotFoundError as e: + raise cdist.Error(cdist_object.name + " requires non-existing " + e.requirement) + + def __iter__(self): + """Iterate over all unique objects while resolving dependencies. + """ + iterable = itertools.chain(*self.graph.values()) + # Keep record of objects that have already been seen + seen = set() + seen_add = seen.add + for cdist_object in itertools.filterfalse(seen.__contains__, iterable): + seen_add(cdist_object) + yield cdist_object diff --git a/lib/cdist/test/emulator/__init__.py b/lib/cdist/test/emulator/__init__.py index 5a660755..e67bed4a 100644 --- a/lib/cdist/test/emulator/__init__.py +++ b/lib/cdist/test/emulator/__init__.py @@ -89,6 +89,13 @@ class EmulatorTestCase(test.CdistTestCase): emu.run() # if we get here all is fine + def test_requirement_pattern(self): + argv = ['__file', '/tmp/foobar'] + os.environ.update(self.env) + os.environ['require'] = '__file/etc/*' + emu = emulator.Emulator(argv) + # if we get here all is fine + import os.path as op my_dir = op.abspath(op.dirname(__file__)) diff --git a/lib/cdist/test/resolver/__init__.py b/lib/cdist/test/resolver/__init__.py new file mode 100644 index 00000000..cca058a4 --- /dev/null +++ b/lib/cdist/test/resolver/__init__.py @@ -0,0 +1,88 @@ +# -*- coding: utf-8 -*- +# +# 2010-2011 Steven Armstrong (steven-cdist at armstrong.cc) +# +# This file is part of cdist. +# +# cdist is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# cdist is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with cdist. If not, see . +# +# + +import os +import shutil + +import cdist +from cdist import test +from cdist import core +from cdist import resolver + +import os.path as op +my_dir = op.abspath(op.dirname(__file__)) +fixtures = op.join(my_dir, 'fixtures') +object_base_path = op.join(fixtures, 'object') +type_base_path = op.join(fixtures, 'type') + + +class ResolverTestCase(test.CdistTestCase): + + def setUp(self): + self.objects = list(core.Object.list_objects(object_base_path, type_base_path)) + self.object_index = dict((o.name, o) for o in self.objects) + self.dependency_resolver = resolver.DependencyResolver(self.objects) + + def tearDown(self): + for o in self.objects: + o.requirements = [] + + def test_find_requirements_by_name_string(self): + requirements = ['__first/man', '__second/on-the', '__third/moon'] + required_objects = [self.object_index[name] for name in requirements] + self.assertEqual(sorted(list(self.dependency_resolver.find_requirements_by_name(requirements))), + sorted(required_objects)) + + def test_find_requirements_by_name_pattern(self): + requirements = ['__first/*', '__second/*-the', '__third/moon'] + requirements_expanded = [ + '__first/child', '__first/dog', '__first/man', '__first/woman', + '__second/on-the', '__second/under-the', + '__third/moon' + ] + required_objects = [self.object_index[name] for name in requirements_expanded] + self.assertEqual(sorted(list(self.dependency_resolver.find_requirements_by_name(requirements))), + sorted(required_objects)) + + def test_dependency_resolution(self): + first_man = self.object_index['__first/man'] + second_on_the = self.object_index['__second/on-the'] + third_moon = self.object_index['__third/moon'] + first_man.requirements = [second_on_the.name] + second_on_the.requirements = [third_moon.name] + self.assertEqual( + self.dependency_resolver.graph['__first/man'], + [third_moon, second_on_the, first_man] + ) + + def test_circular_reference(self): + first_man = self.object_index['__first/man'] + first_woman = self.object_index['__first/woman'] + first_man.requirements = [first_woman.name] + first_woman.requirements = [first_man.name] + with self.assertRaises(resolver.CircularReferenceError): + self.dependency_resolver.graph + + def test_requirement_not_found(self): + first_man = self.object_index['__first/man'] + first_man.requirements = ['__does/not/exist'] + with self.assertRaises(cdist.Error): + self.dependency_resolver.graph diff --git a/lib/cdist/test/resolver/fixtures/object/__first/.keep b/lib/cdist/test/resolver/fixtures/object/__first/.keep new file mode 100644 index 00000000..e69de29b diff --git a/lib/cdist/test/resolver/fixtures/object/__first/child/.cdist/.keep b/lib/cdist/test/resolver/fixtures/object/__first/child/.cdist/.keep new file mode 100644 index 00000000..e69de29b diff --git a/lib/cdist/test/resolver/fixtures/object/__first/dog/.cdist/.keep b/lib/cdist/test/resolver/fixtures/object/__first/dog/.cdist/.keep new file mode 100644 index 00000000..e69de29b diff --git a/lib/cdist/test/resolver/fixtures/object/__first/man/.cdist/.keep b/lib/cdist/test/resolver/fixtures/object/__first/man/.cdist/.keep new file mode 100644 index 00000000..e69de29b diff --git a/lib/cdist/test/resolver/fixtures/object/__first/woman/.cdist/.keep b/lib/cdist/test/resolver/fixtures/object/__first/woman/.cdist/.keep new file mode 100644 index 00000000..e69de29b diff --git a/lib/cdist/test/resolver/fixtures/object/__second/.keep b/lib/cdist/test/resolver/fixtures/object/__second/.keep new file mode 100644 index 00000000..e69de29b diff --git a/lib/cdist/test/resolver/fixtures/object/__second/on-the/.cdist/.keep b/lib/cdist/test/resolver/fixtures/object/__second/on-the/.cdist/.keep new file mode 100644 index 00000000..e69de29b diff --git a/lib/cdist/test/resolver/fixtures/object/__second/under-the/.cdist/.keep b/lib/cdist/test/resolver/fixtures/object/__second/under-the/.cdist/.keep new file mode 100644 index 00000000..e69de29b diff --git a/lib/cdist/test/resolver/fixtures/object/__third/.keep b/lib/cdist/test/resolver/fixtures/object/__third/.keep new file mode 100644 index 00000000..e69de29b diff --git a/lib/cdist/test/resolver/fixtures/object/__third/moon/.cdist/.keep b/lib/cdist/test/resolver/fixtures/object/__third/moon/.cdist/.keep new file mode 100644 index 00000000..e69de29b diff --git a/lib/cdist/test/resolver/fixtures/object/__third/moon/.cdist/parameter/name b/lib/cdist/test/resolver/fixtures/object/__third/moon/.cdist/parameter/name new file mode 100644 index 00000000..4129a761 --- /dev/null +++ b/lib/cdist/test/resolver/fixtures/object/__third/moon/.cdist/parameter/name @@ -0,0 +1 @@ +Prometheus diff --git a/lib/cdist/test/resolver/fixtures/object/__third/moon/.cdist/parameter/planet b/lib/cdist/test/resolver/fixtures/object/__third/moon/.cdist/parameter/planet new file mode 100644 index 00000000..8e6ee422 --- /dev/null +++ b/lib/cdist/test/resolver/fixtures/object/__third/moon/.cdist/parameter/planet @@ -0,0 +1 @@ +Saturn diff --git a/lib/cdist/test/resolver/fixtures/type/__first/.keep b/lib/cdist/test/resolver/fixtures/type/__first/.keep new file mode 100644 index 00000000..e69de29b diff --git a/lib/cdist/test/resolver/fixtures/type/__second/.keep b/lib/cdist/test/resolver/fixtures/type/__second/.keep new file mode 100644 index 00000000..e69de29b diff --git a/lib/cdist/test/resolver/fixtures/type/__third/.keep b/lib/cdist/test/resolver/fixtures/type/__third/.keep new file mode 100644 index 00000000..e69de29b