Merge branch 'feature/detect-dependency-cycle' into 'master'

Detect dependency cycle as soon as possible

See merge request ungleich-public/cdist!774
This commit is contained in:
poljakowski 2019-05-03 14:38:09 +02:00
commit de1c198dc0
2 changed files with 123 additions and 4 deletions

View file

@ -43,6 +43,33 @@ from cdist import core, inventory
from cdist.util.remoteutil import inspect_ssh_mux_opts from cdist.util.remoteutil import inspect_ssh_mux_opts
def graph_check_cycle(graph):
# Start from each node in the graph and check for cycle starting from it.
for node in graph:
# Cycle path.
path = [node]
has_cycle = _graph_dfs_cycle( graph, node, path)
if has_cycle:
return has_cycle, path
return False, None
def _graph_dfs_cycle(graph, node, path):
for neighbour in graph.get(node, ()):
# If node is already in path then this is cycle.
if neighbour in path:
path.append(neighbour)
return True
path.append(neighbour)
rv = _graph_dfs_cycle(graph, neighbour, path)
if rv:
return True
# Remove last item from list - neighbour whose DFS path we have have
# just checked.
del path[-1]
return False
class Config(object): class Config(object):
"""Cdist main class to hold arbitrary data""" """Cdist main class to hold arbitrary data"""
@ -254,14 +281,14 @@ class Config(object):
cls.onehost(host, host_tags, host_base_path, hostdir, cls.onehost(host, host_tags, host_base_path, hostdir,
args, parallel=False, args, parallel=False,
configuration=configuration) configuration=configuration)
except cdist.Error as e: except cdist.Error:
failed_hosts.append(host) failed_hosts.append(host)
if args.parallel and len(process_args) == 1: if args.parallel and len(process_args) == 1:
log.debug("Only 1 host for parallel processing, doing it " log.debug("Only 1 host for parallel processing, doing it "
"sequentially") "sequentially")
try: try:
cls.onehost(*process_args[0]) cls.onehost(*process_args[0])
except cdist.Error as e: except cdist.Error:
failed_hosts.append(host) failed_hosts.append(host)
elif args.parallel: elif args.parallel:
log.trace("Multiprocessing start method is {}".format( log.trace("Multiprocessing start method is {}".format(
@ -653,6 +680,28 @@ class Config(object):
self.__dict__.update(state) self.__dict__.update(state)
self._open_logger() self._open_logger()
def _validate_dependencies(self):
'''
Build dependency graph for unfinished objects and
check for cycles.
'''
graph = {}
for cdist_object in self.object_list():
obj_name = cdist_object.name
if obj_name not in graph:
graph[obj_name] = []
if cdist_object.state == cdist_object.STATE_DONE:
continue
for requirement in cdist_object.requirements_unfinished(
cdist_object.requirements):
graph[obj_name].append(requirement.name)
for requirement in cdist_object.requirements_unfinished(
cdist_object.autorequire):
graph[obj_name].append(requirement.name)
return graph_check_cycle(graph)
def iterate_until_finished(self): def iterate_until_finished(self):
""" """
Go through all objects and solve them Go through all objects and solve them
@ -662,6 +711,12 @@ class Config(object):
objects_changed = True objects_changed = True
while objects_changed: while objects_changed:
# Check for cycles as early as possible.
has_cycle, path = self._validate_dependencies()
if has_cycle:
raise cdist.UnresolvableRequirementsError(
"Cycle detected in object dependencies:\n{}!".format(
" -> ".join(path)))
objects_changed = self.iterate_once() objects_changed = self.iterate_once()
# Check whether all objects have been finished # Check whether all objects have been finished

View file

@ -23,7 +23,6 @@
import os import os
import shutil import shutil
import tempfile
from cdist import test from cdist import test
from cdist import core from cdist import core
@ -212,7 +211,7 @@ class ConfigRunTestCase(test.CdistTestCase):
dryrun.run() dryrun.run()
# if we are here, dryrun works like expected # if we are here, dryrun works like expected
def test_desp_resolver(self): def test_deps_resolver(self):
"""Test to show dependency resolver warning message.""" """Test to show dependency resolver warning message."""
local = cdist.exec.local.Local( local = cdist.exec.local.Local(
target_host=self.target_host, target_host=self.target_host,
@ -229,6 +228,71 @@ class ConfigRunTestCase(test.CdistTestCase):
config = cdist.config.Config(local, self.remote, dry_run=True) config = cdist.config.Config(local, self.remote, dry_run=True)
config.run() config.run()
def test_graph_check_cycle_empty(self):
graph = {}
has_cycle, path = cdist.config.graph_check_cycle(graph)
self.assertFalse(has_cycle)
def test_graph_check_cycle_1(self):
#
# a -> b -> c
# |
# +--> d -> e
graph = {
'a': ['b', ],
'b': ['c', 'd', ],
'd': ['e', ],
}
has_cycle, path = cdist.config.graph_check_cycle(graph)
self.assertFalse(has_cycle)
def test_graph_check_cycle_2(self):
#
# a -> b -> c
# /\ |
# \ |
# +-------+
graph = {
'a': ['b', ],
'b': ['c', ],
'c': ['a', ],
}
has_cycle, path = cdist.config.graph_check_cycle(graph)
self.assertTrue(has_cycle)
self.assertGreater(path.count(path[-1]), 1)
def test_graph_check_cycle_3(self):
#
# a -> b -> c
# \ \
# \ +--> g
# \ /\
# \ /|
# +-> d -> e |
# \ |
# + --> f
#
# h -> i --> j
# | /\ |
# \/ | \/
# n m <- k
graph = {
'a': ['b', 'd', ],
'b': ['c', ],
'c': ['g', ],
'd': ['e', 'f', ],
'e': ['g', ],
'f': ['g', ],
'h': ['i', 'n', ],
'i': ['j', ],
'j': ['k', ],
'k': ['m', ],
'm': ['i', ],
}
has_cycle, path = cdist.config.graph_check_cycle(graph)
self.assertTrue(has_cycle)
self.assertGreater(path.count(path[-1]), 1)
# Currently the resolving code will simply detect that this object does # Currently the resolving code will simply detect that this object does
# not exist. It should probably check if the type is a singleton as well # not exist. It should probably check if the type is a singleton as well