From 633f4c9f00f70310e28b470d9aae399622ccf7d4 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Wed, 19 Jul 2017 17:35:40 -0400 Subject: [PATCH] Get dep_attrs from StackDefinition Since function.dep_attrs() returns logical resource names (rather than actual objects), we can just as easily use the StackDefinition to calculate it instead of the Stack and Resource objects. In the legacy path, we must ensure we use the StackDefinition from the *new* stack to determine which attributes to include in the NodeData, since that's what we're going to be using it for. In the convergence path the current stack definition already contains the new template. Also, update the *new* stack's definition with the NodeData obtained from completed resources (in addition to the existing stack's), so that that data may be used in calculating the dep_attrs for future resources. This is required when get_attr functions are nested in the template. Change-Id: I23efcc091eae53470f7f9cb3ca21e09f00f43808 Partially-Implements: blueprint stack-definition --- heat/engine/resource.py | 44 ++++++++++++------- heat/engine/stack.py | 14 +----- heat/engine/update.py | 23 +++++++--- heat/tests/test_hot.py | 22 +++++++--- heat/tests/test_stack_collect_attributes.py | 6 +-- .../functional/test_stack_outputs.py | 38 ++++++++++++++++ 6 files changed, 102 insertions(+), 45 deletions(-) diff --git a/heat/engine/resource.py b/heat/engine/resource.py index 63eea024a8..94e46615f7 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -14,6 +14,7 @@ import collections import contextlib import datetime as dt +import itertools import pydoc import tenacity import weakref @@ -659,9 +660,6 @@ class Resource(status.ResourceStatus): text = '%s "%s"' % (class_name, self.name) return six.text_type(text) - def dep_attrs(self, resource_name): - return self.t.dep_attrs(resource_name) - def add_explicit_dependencies(self, deps): """Add all dependencies explicitly specified in the template. @@ -949,7 +947,8 @@ class Resource(status.ResourceStatus): self._rsrc_prop_data = None self.attributes.reset_resolved_values() - def referenced_attrs(self, in_resources=True, in_outputs=True): + def referenced_attrs(self, stk_defn=None, + in_resources=True, in_outputs=True): """Return the set of all attributes referenced in the template. This enables the resource to calculate which of its attributes will @@ -958,22 +957,30 @@ class Resource(status.ResourceStatus): `in_resources` or `in_outputs` parameters to False. To limit to a subset of outputs, pass an iterable of the output names to examine for the `in_outputs` parameter. + + The set of referenced attributes is calculated from the + StackDefinition object provided, or from the stack's current + definition if none is passed. """ - def get_dep_attrs(source_dict): - return set(self.stack.get_dep_attrs(six.itervalues(source_dict), - self.name)) + if stk_defn is None: + stk_defn = self.stack.defn + + def get_dep_attrs(source): + return set(itertools.chain.from_iterable(s.dep_attrs(self.name) + for s in source)) refd_attrs = set() if in_resources: - refd_attrs |= get_dep_attrs(self.stack.resources) + enabled_resources = stk_defn.enabled_rsrc_names() + refd_attrs |= get_dep_attrs(stk_defn.resource_definition(r_name) + for r_name in enabled_resources) subset_outputs = isinstance(in_outputs, collections.Iterable) if subset_outputs or in_outputs: - if subset_outputs: - outputs = {k: self.stack.outputs[k] for k in in_outputs} - else: - outputs = self.stack.outputs - refd_attrs |= get_dep_attrs(outputs) + if not subset_outputs: + in_outputs = stk_defn.enabled_output_names() + refd_attrs |= get_dep_attrs(stk_defn.output_definition(op_name) + for op_name in in_outputs) if attributes.ALL_ATTRIBUTES in refd_attrs: refd_attrs.remove(attributes.ALL_ATTRIBUTES) @@ -981,7 +988,7 @@ class Resource(status.ResourceStatus): return refd_attrs - def node_data(self, for_resources=True, for_outputs=False): + def node_data(self, stk_defn=None, for_resources=True, for_outputs=False): """Return a NodeData object representing the resource. The NodeData object returned contains basic data about the resource, @@ -996,6 +1003,10 @@ class Resource(status.ResourceStatus): only those attribute values referenced by the specified stack outputs are included. + The set of referenced attributes is calculated from the + StackDefinition object provided, or from the stack's current + definition if none is passed. + After calling this method, the resource's attribute cache is populated with any cacheable attribute values referenced by stack outputs, even if they are not also referenced by other resources. @@ -1019,12 +1030,13 @@ class Resource(status.ResourceStatus): except exception.InvalidTemplateAttribute as ita: LOG.info('%s', ita) - dep_attrs = self.referenced_attrs(in_resources=for_resources, + dep_attrs = self.referenced_attrs(stk_defn, + in_resources=for_resources, in_outputs=for_outputs) # Ensure all attributes referenced in outputs get cached if for_outputs is False and self.stack.convergence: - out_attrs = self.referenced_attrs(in_resources=False) + out_attrs = self.referenced_attrs(stk_defn, in_resources=False) for e in get_attrs(out_attrs - dep_attrs, cacheable_only=True): pass diff --git a/heat/engine/stack.py b/heat/engine/stack.py index 3f2b3f5e0a..9ab6e484cf 100644 --- a/heat/engine/stack.py +++ b/heat/engine/stack.py @@ -15,7 +15,6 @@ import collections import copy import eventlet import functools -import itertools import re import warnings @@ -213,7 +212,8 @@ class Stack(collections.Mapping): owner_id) if tmpl is not None: self.defn = stk_defn.StackDefinition(context, tmpl, - self.identifier(), {}, + self.identifier(), + cache_data or {}, parent_info) else: self.defn = None @@ -486,16 +486,6 @@ class Stack(collections.Mapping): if not self.parameters.set_stack_id(self.identifier()): LOG.warning("Unable to set parameters StackId identifier") - @staticmethod - def get_dep_attrs(resources, resource_name): - """Return the attributes of the specified resource that are referenced. - - Return an iterator over any attributes of the specified resource that - are referenced in resources. - """ - return set(itertools.chain.from_iterable( - res.dep_attrs(resource_name) for res in resources)) - def _explicit_dependencies(self): """Return dependencies without making any resource plugin calls. diff --git a/heat/engine/update.py b/heat/engine/update.py index d6faeadf7e..727a9c360d 100644 --- a/heat/engine/update.py +++ b/heat/engine/update.py @@ -135,6 +135,8 @@ class StackUpdate(object): yield new_res.create() + self._update_resource_data(new_res) + def _check_replace_restricted(self, res): registry = res.stack.env.registry restricted_actions = registry.get_rsrc_restricted_actions(res.name) @@ -147,6 +149,19 @@ class StackUpdate(object): six.text_type(ex)) raise failure + def _update_resource_data(self, resource): + # Use the *new* template to determine the attrs to cache + node_data = resource.node_data(self.new_stack.defn) + stk_defn.update_resource_data(self.existing_stack.defn, + resource.name, node_data) + + # Also update the new stack's definition with the data, so that + # following resources can calculate dep_attr values correctly (e.g. if + # the actual attribute name in a get_attr function also comes from a + # get_attr function.) + stk_defn.update_resource_data(self.new_stack.defn, + resource.name, node_data) + @scheduler.wrappertask def _process_new_resource_update(self, new_res): res_name = new_res.name @@ -175,19 +190,13 @@ class StackUpdate(object): {'res_name': res_name, 'stack_name': self.existing_stack.name}) - stk_defn.update_resource_data(self.existing_stack.defn, - res_name, - existing_res.node_data()) + self._update_resource_data(existing_res) return else: self._check_replace_restricted(new_res) yield self._create_resource(new_res) - node_data = self.existing_stack[res_name].node_data() - stk_defn.update_resource_data(self.existing_stack.defn, res_name, - node_data) - def _update_in_place(self, existing_res, new_res, is_substituted=False): existing_snippet = self.existing_snippets[existing_res.name] prev_res = self.previous_stack.get(new_res.name) diff --git a/heat/tests/test_hot.py b/heat/tests/test_hot.py index c7f26eb1d7..e4054f80e3 100644 --- a/heat/tests/test_hot.py +++ b/heat/tests/test_hot.py @@ -2361,7 +2361,10 @@ class HotStackTest(common.HeatTestCase): repeat = self.stack.t.parse(self.stack, snippet) self.stack.store() - self.stack.create() + with mock.patch.object(rsrc_defn.ResourceDefinition, + 'dep_attrs') as mock_da: + mock_da.return_value = ['list'] + self.stack.create() self.assertEqual((parser.Stack.CREATE, parser.Stack.COMPLETE), self.stack.state) self.assertEqual(['this is foo', 'this is bar'], @@ -2374,7 +2377,10 @@ class HotStackTest(common.HeatTestCase): self.stack = parser.Stack(self.ctx, 'test_get_attr', template.Template(hot_tpl)) self.stack.store() - self.stack.create() + with mock.patch.object(rsrc_defn.ResourceDefinition, + 'dep_attrs') as mock_da: + mock_da.return_value = ['foo'] + self.stack.create() self.assertEqual((parser.Stack.CREATE, parser.Stack.COMPLETE), self.stack.state) @@ -2717,6 +2723,9 @@ class StackAttributesTest(common.HeatTestCase): self.stack = parser.Stack(self.ctx, 'test_get_attr', template.Template(self.hot_tpl)) self.stack.store() + + parsed = self.stack.t.parse(self.stack, self.snippet) + self.stack.create() self.assertEqual((parser.Stack.CREATE, parser.Stack.COMPLETE), self.stack.state) @@ -2739,9 +2748,7 @@ class StackAttributesTest(common.HeatTestCase): (rsrc.ADOPT, rsrc.COMPLETE)): rsrc.state_set(action, status) - resolved = function.resolve(self.stack.t.parse(self.stack, - self.snippet)) - self.assertEqual(self.expected, resolved) + self.assertEqual(self.expected, function.resolve(parsed)) class StackGetAttributesTestConvergence(common.HeatTestCase): @@ -2865,8 +2872,9 @@ class StackGetAttributesTestConvergence(common.HeatTestCase): dep_attrs = function.dep_attrs( self.stack.t.parse(self.stack, self.snippet), self.resource_name) - with mock.patch.object(rsrc.stack, 'get_dep_attrs') as mock_da: - mock_da.return_value = list(dep_attrs) + with mock.patch.object(rsrc_defn.ResourceDefinition, + 'dep_attrs') as mock_da: + mock_da.return_value = dep_attrs rsrc_data = rsrc.node_data() # store as cache data self.stack.cache_data = { diff --git a/heat/tests/test_stack_collect_attributes.py b/heat/tests/test_stack_collect_attributes.py index 4decbeae0a..381a1aec56 100644 --- a/heat/tests/test_stack_collect_attributes.py +++ b/heat/tests/test_stack_collect_attributes.py @@ -11,6 +11,7 @@ # License for the specific language governing permissions and limitations # under the License. +import itertools import six from heat.common import template_format @@ -231,9 +232,8 @@ class DepAttrsTest(common.HeatTestCase): for res in six.itervalues(self.stack): resources = six.itervalues(self.stack.resources) self.assertEqual(self.expected[res.name], - self.stack.get_dep_attrs( - resources, - res.name)) + set(itertools.chain.from_iterable( + r.t.dep_attrs(res.name) for r in resources))) class ReferencedAttrsTest(common.HeatTestCase): diff --git a/heat_integrationtests/functional/test_stack_outputs.py b/heat_integrationtests/functional/test_stack_outputs.py index 536e5899e9..161e0b360a 100644 --- a/heat_integrationtests/functional/test_stack_outputs.py +++ b/heat_integrationtests/functional/test_stack_outputs.py @@ -61,3 +61,41 @@ outputs: stack_identifier, 'resource_output_b')['output'] self.assertEqual(expected_output_a, actual_output_a) self.assertEqual(expected_output_b, actual_output_b) + + before_template = ''' +heat_template_version: 2015-10-15 +resources: + test_resource_a: + type: OS::Heat::TestResource + properties: + value: 'foo' +outputs: +''' + + after_template = ''' +heat_template_version: 2015-10-15 +resources: + test_resource_a: + type: OS::Heat::TestResource + properties: + value: 'foo' + test_resource_b: + type: OS::Heat::TestResource + properties: + value: {get_attr: [test_resource_a, output]} +outputs: + output_value: + description: 'Output of resource b' + value: {get_attr: [test_resource_b, output]} +''' + + def test_outputs_update_new_resource(self): + stack_identifier = self.stack_create(template=self.before_template) + self.update_stack(stack_identifier, template=self.after_template) + + expected_output_value = { + u'output_value': u'foo', u'output_key': u'output_value', + u'description': u'Output of resource b'} + actual_output_value = self.client.stacks.output_show( + stack_identifier, 'output_value')['output'] + self.assertEqual(expected_output_value, actual_output_value)