Merge remote-tracking branch 'steven/integration_dependency-resolver'

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:

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:
+        if cdist_object.state == core.Object.STATE_DONE:
-            # FIXME: resolve dependency circle / show problem source
+            # TODO: remove once we are sure that this really never happens.
-            raise cdist.Error("Detected circular dependency in " + cdist_object.name)
+            raise cdist.Error("Attempting to run an already finished object: %s", cdist_object)
-        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
 
         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)

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 '<Object %s>' % self.name
 
     def __eq__(self, other):
-        """define equality as 'attributes are the same'"""
+        """define equality as 'name is the same'"""
-        return self.__dict__ == other.__dict__
+        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

lib/cdist/emulator.py

@@ -154,30 +154,18 @@ class Emulator(object):
                 if len(requirement) == 0:
                     continue
 
-                self.log.debug("Recording requirement: " + requirement)
+                requirement_type_name, requirement_object_id = core.Object.split_name(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('/')
-
                 # 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' \
+                if requirement_object_id:
-                    and not requirement_type.is_singleton:
+                    # 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
+                self.log.debug("Recording requirement: " + requirement)
-                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.cdist_object.requirements.append(requirement)
 
         # Record / Append source

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 <http://www.gnu.org/licenses/>.
+#
+#
+
+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 __type/object_id>, <Object __other_type/any>, <Object __other_type/match>]
+        """
+        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

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__))

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 <http://www.gnu.org/licenses/>.
+#
+#
+
+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

lib/cdist/test/resolver/fixtures/object/__third/moon/.cdist/parameter/name

@@ -0,0 +1 @@
+Prometheus

lib/cdist/test/resolver/fixtures/object/__third/moon/.cdist/parameter/planet

@@ -0,0 +1 @@
+Saturn