Catch exceptions calculating implicit dependencies
Uncaught exceptions in an overridden add_dependencies() method of a plugin can prevent a stack being loaded from the database. To protect against programming errors that can result in uncaught exceptions, separate the calculation of implicit dependencies out from the calculation of explicit dependencies, and ignore exceptions in the latter when dealing with an existing stack. Change-Id: I939dba57eeba5710bb77a1b30a872fca5d38ad71 Closes-Bug: #1554625
This commit is contained in:
parent
bbda7712dd
commit
38a46e8d21
|
@ -560,11 +560,31 @@ class Resource(object):
|
|||
def dep_attrs(self, resource_name):
|
||||
return self.t.dep_attrs(resource_name)
|
||||
|
||||
def add_dependencies(self, deps):
|
||||
def add_explicit_dependencies(self, deps):
|
||||
"""Add all dependencies explicitly specified in the template.
|
||||
|
||||
The deps parameter is a Dependencies object to which dependency pairs
|
||||
are added.
|
||||
"""
|
||||
for dep in self.t.dependencies(self.stack):
|
||||
deps += (self, dep)
|
||||
deps += (self, None)
|
||||
|
||||
def add_dependencies(self, deps):
|
||||
"""Add implicit dependencies specific to the resource type.
|
||||
|
||||
Some resource types may have implicit dependencies on other resources
|
||||
in the same stack that are not linked by a property value (that would
|
||||
be set using get_resource or get_attr for example, thus creating an
|
||||
explicit dependency). Such dependencies are opaque to the user and
|
||||
should be avoided wherever possible, however in some circumstances they
|
||||
are required due to magic in the underlying API.
|
||||
|
||||
The deps parameter is a Dependencies object to which dependency pairs
|
||||
may be added.
|
||||
"""
|
||||
return
|
||||
|
||||
def required_by(self):
|
||||
"""List of resources that require this one as a dependency.
|
||||
|
||||
|
|
|
@ -340,7 +340,8 @@ class Stack(collections.Mapping):
|
|||
def dependencies(self):
|
||||
if self._dependencies is None:
|
||||
self._dependencies = self._get_dependencies(
|
||||
six.itervalues(self.resources))
|
||||
six.itervalues(self.resources),
|
||||
ignore_errors=self.id is not None)
|
||||
return self._dependencies
|
||||
|
||||
def reset_dependencies(self):
|
||||
|
@ -412,14 +413,22 @@ class Stack(collections.Mapping):
|
|||
return set(itertools.chain.from_iterable(attr_lists))
|
||||
|
||||
@staticmethod
|
||||
def _get_dependencies(resources):
|
||||
def _get_dependencies(resources, ignore_errors=True):
|
||||
"""Return the dependency graph for a list of resources."""
|
||||
deps = dependencies.Dependencies()
|
||||
for res in resources:
|
||||
res.add_explicit_dependencies(deps)
|
||||
|
||||
try:
|
||||
res.add_dependencies(deps)
|
||||
except ValueError:
|
||||
pass
|
||||
except Exception as exc:
|
||||
if not ignore_errors:
|
||||
raise
|
||||
else:
|
||||
LOG.warning(_LW('Ignoring error adding implicit '
|
||||
'dependencies for %(res)s: %(err)s') %
|
||||
{'res': six.text_type(res),
|
||||
'err': six.text_type(exc)})
|
||||
|
||||
return deps
|
||||
|
||||
|
|
|
@ -326,6 +326,7 @@ class ServersTest(common.HeatTestCase):
|
|||
server_rsrc = stack['server']
|
||||
subnet_rsrc = stack['subnet']
|
||||
deps = []
|
||||
server_rsrc.add_explicit_dependencies(deps)
|
||||
server_rsrc.add_dependencies(deps)
|
||||
self.assertEqual(4, len(deps))
|
||||
self.assertEqual(subnet_rsrc, deps[3])
|
||||
|
@ -339,6 +340,7 @@ class ServersTest(common.HeatTestCase):
|
|||
server_rsrc = stack['server']
|
||||
subnet_rsrc = stack['subnet']
|
||||
deps = []
|
||||
server_rsrc.add_explicit_dependencies(deps)
|
||||
server_rsrc.add_dependencies(deps)
|
||||
self.assertEqual(2, len(deps))
|
||||
self.assertNotIn(subnet_rsrc, deps)
|
||||
|
|
|
@ -2289,12 +2289,12 @@ class ResourceDependenciesTest(common.HeatTestCase):
|
|||
stack = parser.Stack(utils.dummy_context(), 'test', tmpl)
|
||||
|
||||
res = stack['foo']
|
||||
res.add_dependencies(self.deps)
|
||||
res.add_explicit_dependencies(self.deps)
|
||||
graph = self.deps.graph()
|
||||
|
||||
self.assertIn(res, graph)
|
||||
|
||||
def test_hot_add_dep_error(self):
|
||||
def test_hot_add_dep_error_create(self):
|
||||
tmpl = template.Template({
|
||||
'heat_template_version': '2013-05-23',
|
||||
'resources': {
|
||||
|
@ -2304,10 +2304,34 @@ class ResourceDependenciesTest(common.HeatTestCase):
|
|||
})
|
||||
stack = parser.Stack(utils.dummy_context(), 'test', tmpl)
|
||||
res = stack['bar']
|
||||
|
||||
class TestException(Exception):
|
||||
pass
|
||||
|
||||
self.patchobject(res, 'add_dependencies',
|
||||
side_effect=TestException)
|
||||
|
||||
def get_dependencies():
|
||||
return stack.dependencies
|
||||
|
||||
self.assertRaises(TestException, get_dependencies)
|
||||
|
||||
def test_hot_add_dep_error_load(self):
|
||||
tmpl = template.Template({
|
||||
'heat_template_version': '2013-05-23',
|
||||
'resources': {
|
||||
'foo': {'type': 'GenericResourceType'},
|
||||
'bar': {'type': 'ResourceWithPropsType'}
|
||||
}
|
||||
})
|
||||
stack = parser.Stack(utils.dummy_context(), 'test_hot_add_dep_err',
|
||||
tmpl)
|
||||
stack.store()
|
||||
res = stack['bar']
|
||||
self.patchobject(res, 'add_dependencies',
|
||||
side_effect=ValueError)
|
||||
graph = stack.dependencies.graph()
|
||||
self.assertNotIn(res, graph)
|
||||
self.assertIn(res, graph)
|
||||
self.assertIn(stack['foo'], graph)
|
||||
|
||||
def test_ref(self):
|
||||
|
@ -2326,7 +2350,7 @@ class ResourceDependenciesTest(common.HeatTestCase):
|
|||
stack = parser.Stack(utils.dummy_context(), 'test', tmpl)
|
||||
|
||||
res = stack['bar']
|
||||
res.add_dependencies(self.deps)
|
||||
res.add_explicit_dependencies(self.deps)
|
||||
graph = self.deps.graph()
|
||||
|
||||
self.assertIn(res, graph)
|
||||
|
@ -2349,7 +2373,7 @@ class ResourceDependenciesTest(common.HeatTestCase):
|
|||
stack = parser.Stack(utils.dummy_context(), 'test', tmpl)
|
||||
|
||||
res = stack['bar']
|
||||
res.add_dependencies(self.deps)
|
||||
res.add_explicit_dependencies(self.deps)
|
||||
graph = self.deps.graph()
|
||||
|
||||
self.assertIn(res, graph)
|
||||
|
@ -2371,7 +2395,7 @@ class ResourceDependenciesTest(common.HeatTestCase):
|
|||
stack = parser.Stack(utils.dummy_context(), 'test', tmpl)
|
||||
|
||||
res = stack['bar']
|
||||
res.add_dependencies(self.deps)
|
||||
res.add_explicit_dependencies(self.deps)
|
||||
graph = self.deps.graph()
|
||||
|
||||
self.assertIn(res, graph)
|
||||
|
@ -2393,7 +2417,7 @@ class ResourceDependenciesTest(common.HeatTestCase):
|
|||
stack = parser.Stack(utils.dummy_context(), 'test', tmpl)
|
||||
|
||||
res = stack['bar']
|
||||
res.add_dependencies(self.deps)
|
||||
res.add_explicit_dependencies(self.deps)
|
||||
graph = self.deps.graph()
|
||||
|
||||
self.assertIn(res, graph)
|
||||
|
@ -2417,7 +2441,7 @@ class ResourceDependenciesTest(common.HeatTestCase):
|
|||
stack = parser.Stack(utils.dummy_context(), 'test', tmpl)
|
||||
|
||||
res = stack['bar']
|
||||
res.add_dependencies(self.deps)
|
||||
res.add_explicit_dependencies(self.deps)
|
||||
graph = self.deps.graph()
|
||||
|
||||
self.assertIn(res, graph)
|
||||
|
@ -2441,7 +2465,7 @@ class ResourceDependenciesTest(common.HeatTestCase):
|
|||
stack = parser.Stack(utils.dummy_context(), 'test', tmpl)
|
||||
|
||||
res = stack['bar']
|
||||
res.add_dependencies(self.deps)
|
||||
res.add_explicit_dependencies(self.deps)
|
||||
graph = self.deps.graph()
|
||||
|
||||
self.assertIn(res, graph)
|
||||
|
@ -2522,7 +2546,7 @@ class ResourceDependenciesTest(common.HeatTestCase):
|
|||
stack = parser.Stack(utils.dummy_context(), 'test', tmpl)
|
||||
|
||||
res = stack['bar']
|
||||
res.add_dependencies(self.deps)
|
||||
res.add_explicit_dependencies(self.deps)
|
||||
graph = self.deps.graph()
|
||||
|
||||
self.assertIn(res, graph)
|
||||
|
@ -2544,7 +2568,7 @@ class ResourceDependenciesTest(common.HeatTestCase):
|
|||
stack = parser.Stack(utils.dummy_context(), 'test', tmpl)
|
||||
|
||||
res = stack['bar']
|
||||
res.add_dependencies(self.deps)
|
||||
res.add_explicit_dependencies(self.deps)
|
||||
graph = self.deps.graph()
|
||||
|
||||
self.assertIn(res, graph)
|
||||
|
@ -2566,7 +2590,7 @@ class ResourceDependenciesTest(common.HeatTestCase):
|
|||
stack = parser.Stack(utils.dummy_context(), 'test', tmpl)
|
||||
|
||||
res = stack['bar']
|
||||
res.add_dependencies(self.deps)
|
||||
res.add_explicit_dependencies(self.deps)
|
||||
graph = self.deps.graph()
|
||||
|
||||
self.assertIn(res, graph)
|
||||
|
@ -2588,7 +2612,7 @@ class ResourceDependenciesTest(common.HeatTestCase):
|
|||
stack = parser.Stack(utils.dummy_context(), 'test', tmpl)
|
||||
|
||||
res = stack['bar']
|
||||
res.add_dependencies(self.deps)
|
||||
res.add_explicit_dependencies(self.deps)
|
||||
graph = self.deps.graph()
|
||||
|
||||
self.assertIn(res, graph)
|
||||
|
@ -2613,7 +2637,7 @@ class ResourceDependenciesTest(common.HeatTestCase):
|
|||
stack = parser.Stack(utils.dummy_context(), 'test', tmpl)
|
||||
|
||||
res = stack['bar']
|
||||
res.add_dependencies(self.deps)
|
||||
res.add_explicit_dependencies(self.deps)
|
||||
graph = self.deps.graph()
|
||||
|
||||
self.assertIn(res, graph)
|
||||
|
@ -2638,7 +2662,7 @@ class ResourceDependenciesTest(common.HeatTestCase):
|
|||
stack = parser.Stack(utils.dummy_context(), 'test', tmpl)
|
||||
|
||||
res = stack['bar']
|
||||
res.add_dependencies(self.deps)
|
||||
res.add_explicit_dependencies(self.deps)
|
||||
graph = self.deps.graph()
|
||||
|
||||
self.assertIn(res, graph)
|
||||
|
@ -2742,7 +2766,7 @@ class ResourceDependenciesTest(common.HeatTestCase):
|
|||
stack = parser.Stack(utils.dummy_context(), 'test', tmpl)
|
||||
|
||||
res = stack['bar']
|
||||
res.add_dependencies(self.deps)
|
||||
res.add_explicit_dependencies(self.deps)
|
||||
graph = self.deps.graph()
|
||||
|
||||
self.assertIn(res, graph)
|
||||
|
@ -2762,7 +2786,7 @@ class ResourceDependenciesTest(common.HeatTestCase):
|
|||
stack = parser.Stack(utils.dummy_context(), 'test', tmpl)
|
||||
|
||||
res = stack['bar']
|
||||
res.add_dependencies(self.deps)
|
||||
res.add_explicit_dependencies(self.deps)
|
||||
graph = self.deps.graph()
|
||||
|
||||
self.assertIn(res, graph)
|
||||
|
|
Loading…
Reference in New Issue