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

@@ -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

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

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

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

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

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