From 48fb66bdf39f93381d33fb22d82f8824f0f5f02f Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Fri, 17 Feb 2017 13:33:05 -0500 Subject: [PATCH] Separate management of parent resource into separate class The facade_resource intrinsic function needs to access the parent resource of a stack, which it does via the stack.parent_resource property. Accessing this for the first time loads the parent stack and all of its resources if they were not already. This made sense when all nested stacks in a tree were handled in-memory at the same time, but now that they are processed by separate engines it is inefficient. This change moves responsibility for lazy-loading the parent stack to a separate ParentResourceProxy class, and makes access more efficient again by avoiding the loading of resources if the stack was not already in memory. It also resolves a circular reference between nested stacks and their parent stacks. We now have a well-defined API behind which we can potentially make further efficiency improvements while giving third-party Function plugin developers confidence that we won't break them without notice. Change-Id: Ibfd80544889778f3499bcbe421b83f0a5aa6a7f7 Partially-Implements: blueprint stack-definition --- heat/engine/parent_rsrc.py | 88 +++++++++++++++++++++++++++++ heat/engine/stack.py | 40 ++++++------- heat/tests/test_engine_api_utils.py | 4 +- heat/tests/test_hot.py | 27 +++++---- heat/tests/test_stack_resource.py | 3 +- heat/tests/test_template.py | 30 +++++++--- 6 files changed, 152 insertions(+), 40 deletions(-) create mode 100644 heat/engine/parent_rsrc.py diff --git a/heat/engine/parent_rsrc.py b/heat/engine/parent_rsrc.py new file mode 100644 index 0000000000..27e628854b --- /dev/null +++ b/heat/engine/parent_rsrc.py @@ -0,0 +1,88 @@ +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import weakref + +from heat.objects import resource as resource_object + + +class ParentResourceProxy(object): + """Proxy for the TemplateResource that owns a provider stack. + + This is an interface through which the Fn::ResourceFacade/resource_facade + intrinsic functions in a stack can access data about the TemplateResource + in the parent stack for which it was created. + + This API can be considered stable by third-party Function plugins, and no + part of it should be changed or removed without an appropriate deprecation + process. + """ + + def __new__(cls, context, parent_resource_name, parent_stack_id): + if parent_resource_name is None: + return None + return super(ParentResourceProxy, cls).__new__(cls) + + def __init__(self, context, parent_resource_name, parent_stack_id): + self._context = context + self.name = parent_resource_name + self._stack_id = parent_stack_id + self._stack_ref = None + self._parent_stack = None + + def _stack(self): + if self._stack_ref is not None: + stk = self._stack_ref() + if stk is not None: + return stk + + assert self._stack_id is not None, "Must provide parent stack or ID" + + from heat.engine import stack + self._parent_stack = stack.Stack.load(self._context, + stack_id=self._stack_id) + self._stack_ref = weakref.ref(self._parent_stack) + return self._parent_stack + + def metadata_get(self): + """Return the resource metadata.""" + # If we're using an existing stack that was passed in, assume that its + # resources are already in memory. If they haven't been stored to the + # DB yet, this avoids an unnecessary attempt to read from it. + if self._parent_stack is None: + refd_stk = self._stack_ref and self._stack_ref() + if refd_stk is not None: + return refd_stk[self.name].metadata_get() + + assert self._stack_id is not None, "Must provide parent stack or ID" + + # Try to read just this resource from the DB + rs = resource_object.Resource.get_by_name_and_stack(self._context, + self.name, + self._stack_id) + if rs is not None: + return rs.rsrc_metadata + + # Resource not stored, just return the data from the template + return self.t.metadata() + + @property + def t(self): + """The resource definition.""" + stk = self._stack() + return stk.t.resource_definitions(stk)[self.name] + + +def use_parent_stack(parent_proxy, stack): + parent_proxy._stack_ref = weakref.ref(stack) + parent_proxy._parent_stack = None diff --git a/heat/engine/stack.py b/heat/engine/stack.py index 29416def1f..9c58ffef78 100644 --- a/heat/engine/stack.py +++ b/heat/engine/stack.py @@ -45,6 +45,7 @@ from heat.engine import environment from heat.engine import event from heat.engine.notification import stack as notification from heat.engine import parameter_groups as param_groups +from heat.engine import parent_rsrc from heat.engine import resource from heat.engine import resources from heat.engine import scheduler @@ -166,8 +167,9 @@ class Stack(collections.Mapping): self.status_reason = status_reason self.timeout_mins = timeout_mins self.disable_rollback = disable_rollback - self.parent_resource_name = parent_resource - self._parent_stack = None + self._parent_info = parent_rsrc.ParentResourceProxy(context, + parent_resource, + owner_id) self._outputs = None self._resources = None self._dependencies = None @@ -254,27 +256,22 @@ class Stack(collections.Mapping): """This is a helper to allow resources to access stack.env.""" return self.t.env + @property + def parent_resource_name(self): + if self._parent_info is None: + return None + return self._parent_info.name + @property def parent_resource(self): """Dynamically load up the parent_resource. Note: this should only be used by "Fn::ResourceFacade" """ - if self._parent_stack is None: - # we need both parent name and owner id. - if self.parent_resource_name is None or self.owner_id is None: - return None - - try: - owner = self.load(self.context, stack_id=self.owner_id) - except exception.NotFound: - return None - self._parent_stack = owner - - return self._parent_stack[self.parent_resource_name] + return self._parent_info def set_parent_stack(self, parent_stack): - self._parent_stack = parent_stack + parent_rsrc.use_parent_stack(self._parent_info, parent_stack) def stored_context(self): if self.user_creds_id: @@ -418,12 +415,17 @@ class Stack(collections.Mapping): If this is not nested return (None, self), else return stack resources and stacks in path from the root stack and including this stack. + Note that this is horribly inefficient, as it requires us to load every + stack in the chain back to the root in memory at the same time. + :returns: a list of (stack_resource, stack) tuples. """ - if self.parent_resource and self.parent_resource.stack: - path = self.parent_resource.stack.object_path_in_stack() - path.extend([(self.parent_resource, self)]) - return path + if self.parent_resource: + parent_stack = self.parent_resource._stack() + if parent_stack is not None: + path = parent_stack.object_path_in_stack() + path.extend([(self.parent_resource, self)]) + return path return [(None, self)] def path_in_stack(self): diff --git a/heat/tests/test_engine_api_utils.py b/heat/tests/test_engine_api_utils.py index 70f26360b2..8326ec0d74 100644 --- a/heat/tests/test_engine_api_utils.py +++ b/heat/tests/test_engine_api_utils.py @@ -26,6 +26,7 @@ from heat.db.sqlalchemy import models from heat.engine import api from heat.engine.cfn import parameters as cfn_param from heat.engine import event +from heat.engine import parent_rsrc from heat.engine import stack as parser from heat.engine import template from heat.objects import event as event_object @@ -282,7 +283,8 @@ class FormatTest(common.HeatTestCase): def test_format_stack_resource_with_parent_stack(self): res = self.stack['generic1'] - res.stack.parent_resource_name = 'foobar' + res.stack._parent_info = parent_rsrc.ParentResourceProxy( + self.stack.context, 'foobar', None) formatted = api.format_stack_resource(res, False) self.assertEqual('foobar', formatted[rpc_api.RES_PARENT_RESOURCE]) diff --git a/heat/tests/test_hot.py b/heat/tests/test_hot.py index 3e82c8c062..7604f39aa0 100644 --- a/heat/tests/test_hot.py +++ b/heat/tests/test_hot.py @@ -1666,13 +1666,16 @@ conditions: 'parent', 'SomeType', deletion_policy=rsrc_defn.ResourceDefinition.RETAIN, update_policy={"blarg": "wibble"}) + tmpl = copy.deepcopy(hot_tpl_empty) + tmpl['resources'] = {'parent': parent_resource.t.render_hot()} parent_resource.stack = parser.Stack(utils.dummy_context(), 'toplevel_stack', - template.Template(hot_tpl_empty)) + template.Template(tmpl)) + parent_resource.stack._resources = {'parent': parent_resource} stack = parser.Stack(utils.dummy_context(), 'test_stack', template.Template(hot_tpl_empty), parent_resource='parent') - stack._parent_stack = dict(parent=parent_resource) + stack.set_parent_stack(parent_resource.stack) self.assertEqual({"foo": "bar"}, self.resolve(metadata_snippet, stack.t, stack)) self.assertEqual('Retain', @@ -1685,20 +1688,22 @@ conditions: parent_resource = DummyClass() parent_resource.metadata_set({"foo": "bar"}) - tmpl = template.Template(hot_juno_tpl_empty) - parent_resource.stack = parser.Stack(utils.dummy_context(), - 'toplevel_stack', - tmpl) - del_policy = hot_functions.Join(parent_resource.stack, + del_policy = hot_functions.Join(None, 'list_join', ['eta', ['R', 'in']]) parent_resource.t = rsrc_defn.ResourceDefinition( 'parent', 'SomeType', deletion_policy=del_policy) + tmpl = copy.deepcopy(hot_juno_tpl_empty) + tmpl['resources'] = {'parent': parent_resource.t.render_hot()} + parent_resource.stack = parser.Stack(utils.dummy_context(), + 'toplevel_stack', + template.Template(tmpl)) + parent_resource.stack._resources = {'parent': parent_resource} stack = parser.Stack(utils.dummy_context(), 'test_stack', template.Template(hot_tpl_empty), parent_resource='parent') - stack._parent_stack = dict(parent=parent_resource) + stack.set_parent_stack(parent_resource.stack) self.assertEqual('Retain', self.resolve(deletion_policy_snippet, stack.t, stack)) @@ -1718,14 +1723,16 @@ conditions: parent_resource = DummyClass() parent_resource.metadata_set({"foo": "bar"}) parent_resource.t = rsrc_defn.ResourceDefinition('parent', 'SomeType') + tmpl = copy.deepcopy(hot_tpl_empty) + tmpl['resources'] = {'parent': parent_resource.t.render_hot()} parent_stack = parser.Stack(utils.dummy_context(), 'toplevel_stack', - template.Template(hot_tpl_empty)) + template.Template(tmpl)) parent_stack._resources = {'parent': parent_resource} stack = parser.Stack(utils.dummy_context(), 'test_stack', template.Template(hot_tpl_empty), parent_resource='parent') - stack._parent_stack = parent_stack + stack.set_parent_stack(parent_stack) self.assertEqual('Delete', self.resolve(snippet, stack.t, stack)) def test_removed_function(self): diff --git a/heat/tests/test_stack_resource.py b/heat/tests/test_stack_resource.py index 0ddec04afc..11fe9ae0d8 100644 --- a/heat/tests/test_stack_resource.py +++ b/heat/tests/test_stack_resource.py @@ -350,7 +350,8 @@ class StackResourceTest(StackResourceBaseTest): nest_stack = stk_resource._parse_nested_stack( "test_nest_stack", stk_resource.child_template(), stk_resource.child_params()) - self.assertEqual(nest_stack._parent_stack, self.parent_stack) + self.assertEqual(self.parent_stack, + nest_stack.parent_resource._stack()) def test_preview_dict_validates_nested_resources(self): parent_t = self.parent_stack.t diff --git a/heat/tests/test_template.py b/heat/tests/test_template.py index a409d68483..497ebf6cd0 100644 --- a/heat/tests/test_template.py +++ b/heat/tests/test_template.py @@ -1306,13 +1306,18 @@ class TemplateTest(common.HeatTestCase): 'parent', 'SomeType', deletion_policy=rsrc_defn.ResourceDefinition.RETAIN, update_policy={"blarg": "wibble"}) - + tmpl = copy.deepcopy(empty_template) + tmpl['Resources'] = {'parent': {'Type': 'SomeType', + 'DeletionPolicy': 'Retain', + 'UpdatePolicy': {"blarg": "wibble"}}} parent_resource.stack = stack.Stack(self.ctx, 'toplevel_stack', - template.Template(empty_template)) + template.Template(tmpl)) + parent_resource.stack._resources = {'parent': parent_resource} + stk = stack.Stack(self.ctx, 'test_stack', template.Template(empty_template), parent_resource='parent', owner_id=45) - stk._parent_stack = dict(parent=parent_resource) + stk.set_parent_stack(parent_resource.stack) self.assertEqual({"foo": "bar"}, self.resolve(metadata_snippet, stk.t, stk)) self.assertEqual('Retain', @@ -1325,18 +1330,22 @@ class TemplateTest(common.HeatTestCase): parent_resource = DummyClass() parent_resource.metadata_set({"foo": "bar"}) - parent_resource.stack = stack.Stack(self.ctx, 'toplevel_stack', - template.Template(empty_template)) - del_policy = cfn_funcs.Join(parent_resource.stack, + del_policy = cfn_funcs.Join(None, 'Fn::Join', ['eta', ['R', 'in']]) parent_resource.t = rsrc_defn.ResourceDefinition( 'parent', 'SomeType', deletion_policy=del_policy) + tmpl = copy.deepcopy(empty_template) + tmpl['Resources'] = {'parent': {'Type': 'SomeType', + 'DeletionPolicy': del_policy}} + parent_resource.stack = stack.Stack(self.ctx, 'toplevel_stack', + template.Template(tmpl)) + parent_resource.stack._resources = {'parent': parent_resource} stk = stack.Stack(self.ctx, 'test_stack', template.Template(empty_template), parent_resource='parent') - stk._parent_stack = dict(parent=parent_resource) + stk.set_parent_stack(parent_resource.stack) self.assertEqual('Retain', self.resolve(deletion_policy_snippet, stk.t, stk)) @@ -1354,13 +1363,16 @@ class TemplateTest(common.HeatTestCase): parent_resource = DummyClass() parent_resource.metadata_set({"foo": "bar"}) parent_resource.t = rsrc_defn.ResourceDefinition('parent', 'SomeType') + tmpl = copy.deepcopy(empty_template) + tmpl['Resources'] = {'parent': {'Type': 'SomeType'}} parent_resource.stack = stack.Stack(self.ctx, 'toplevel_stack', - template.Template(empty_template)) + template.Template(tmpl)) + parent_resource.stack._resources = {'parent': parent_resource} stk = stack.Stack(self.ctx, 'test_stack', template.Template(empty_template), parent_resource='parent', owner_id=78) - stk._parent_stack = dict(parent=parent_resource) + stk.set_parent_stack(parent_resource.stack) self.assertEqual('Delete', self.resolve(snippet, stk.t, stk)) def test_prevent_parameters_access(self):