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
(cherry picked from commit 38a46e8d21
)
This commit is contained in:
parent
e13e00734d
commit
3cb015e410
|
@ -547,11 +547,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' names which require the resource as dependency.
|
||||
|
||||
|
|
|
@ -303,7 +303,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):
|
||||
|
@ -375,14 +376,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
|
||||
|
||||
|
|
|
@ -310,6 +310,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])
|
||||
|
@ -320,6 +321,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)
|
||||
|
|
|
@ -2205,12 +2205,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': {
|
||||
|
@ -2220,10 +2220,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):
|
||||
|
@ -2242,7 +2266,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)
|
||||
|
@ -2265,7 +2289,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)
|
||||
|
@ -2287,7 +2311,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)
|
||||
|
@ -2309,7 +2333,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)
|
||||
|
@ -2333,7 +2357,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)
|
||||
|
@ -2357,7 +2381,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)
|
||||
|
@ -2438,7 +2462,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)
|
||||
|
@ -2460,7 +2484,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)
|
||||
|
@ -2482,7 +2506,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)
|
||||
|
@ -2504,7 +2528,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)
|
||||
|
@ -2529,7 +2553,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)
|
||||
|
@ -2554,7 +2578,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)
|
||||
|
@ -2658,7 +2682,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)
|
||||
|
@ -2678,7 +2702,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