From ac87bc7c7932b9a0245d12a9a46b6fbad42be9b5 Mon Sep 17 00:00:00 2001 From: Zane Bitter Date: Mon, 8 Jan 2018 17:23:12 -0500 Subject: [PATCH] Get ResourceGroup/Chain attributes from nested stack outputs Since Pike we've generated the templates for various nested stack resource types with outputs that produce the attribute values that are referenced in the parent stack, and a previous patch did the same for resource IDs. Use these output values when available to calculate the attribute values. This is efficient, because the all of the outputs are fetched together and cached, and avoids using the grouputils functions that cause the nested stack to get loaded into memory in the same process. Fall back to the existing implementation (using grouputils) if the required output isn't available (generally this would be due to the nested stack being created on an earlier version of Heat that did not include the outputs in the generated template, and not updated since). Change-Id: I9146e1d99f494213daef7d61ae75c92a4ef981a9 Partial-Bug: #1731349 --- .../openstack/heat/resource_chain.py | 39 +++++++++++--- .../openstack/heat/resource_group.py | 50 ++++++++++++++--- .../openstack/heat/software_deployment.py | 3 +- .../openstack/heat/test_resource_chain.py | 40 ++++++++++++++ .../openstack/heat/test_resource_group.py | 53 ++++++++++++++++--- .../heat/test_software_deployment.py | 41 ++++++++++++++ 6 files changed, 203 insertions(+), 23 deletions(-) diff --git a/heat/engine/resources/openstack/heat/resource_chain.py b/heat/engine/resources/openstack/heat/resource_chain.py index bdc11d8c07..317148a80f 100644 --- a/heat/engine/resources/openstack/heat/resource_chain.py +++ b/heat/engine/resources/openstack/heat/resource_chain.py @@ -14,6 +14,8 @@ import functools import six +from oslo_log import log as logging + from heat.common import exception from heat.common import grouputils from heat.common.i18n import _ @@ -25,6 +27,8 @@ from heat.engine import rsrc_defn from heat.engine import support from heat.scaling import template as scl_template +LOG = logging.getLogger(__name__) + class ResourceChain(stack_resource.StackResource): """Creates one or more resources with the same configuration. @@ -142,19 +146,41 @@ class ResourceChain(stack_resource.StackResource): def child_params(self): return {} + def _attribute_output_name(self, *attr_path): + return ', '.join(six.text_type(a) for a in attr_path) + def get_attribute(self, key, *path): + if key == self.ATTR_ATTRIBUTES and not path: + raise exception.InvalidTemplateAttribute(resource=self.name, + key=key) + + try: + output = self.get_output(self._attribute_output_name(key, *path)) + except (exception.NotFound, + exception.TemplateOutputError) as op_err: + resource_types = self.properties[self.RESOURCES] + names = self._resource_names(resource_types) + if key.startswith('resource.'): + target = key.split('.', 2)[1] + if target not in names: + raise exception.NotFound(_("Member '%(mem)s' not " + "found in group resource " + "'%(grp)s'.") % + {'mem': target, + 'grp': self.name}) + LOG.debug('Falling back to grouputils due to %s', op_err) + else: + if key == self.REFS: + return attributes.select_from_attribute(output, path) + return output + if key.startswith('resource.'): return grouputils.get_nested_attrs(self, key, False, *path) - resource_types = self.properties[self.RESOURCES] - names = self._resource_names(resource_types) if key == self.REFS: vals = [grouputils.get_rsrc_id(self, key, False, n) for n in names] return attributes.select_from_attribute(vals, path) if key == self.ATTR_ATTRIBUTES: - if not path: - raise exception.InvalidTemplateAttribute( - resource=self.name, key=key) return dict((n, grouputils.get_rsrc_attr( self, key, False, n, *path)) for n in names) @@ -166,10 +192,9 @@ class ResourceChain(stack_resource.StackResource): for attr in self.referenced_attrs(): if isinstance(attr, six.string_types): key, path = attr, [] - output_name = attr else: key, path = attr[0], list(attr[1:]) - output_name = ', '.join(six.text_type(a) for a in attr) + output_name = self._attribute_output_name(key, *path) value = None if key.startswith("resource."): diff --git a/heat/engine/resources/openstack/heat/resource_group.py b/heat/engine/resources/openstack/heat/resource_group.py index def39be388..742bb5e387 100644 --- a/heat/engine/resources/openstack/heat/resource_group.py +++ b/heat/engine/resources/openstack/heat/resource_group.py @@ -15,9 +15,10 @@ import collections import copy import functools import itertools - import six +from oslo_log import log as logging + from heat.common import exception from heat.common import grouputils from heat.common.i18n import _ @@ -34,6 +35,8 @@ from heat.engine import support from heat.scaling import rolling_update from heat.scaling import template as scl_template +LOG = logging.getLogger(__name__) + class ResourceGroup(stack_resource.StackResource): """Creates one or more identically configured nested resources. @@ -455,7 +458,44 @@ class ResourceGroup(stack_resource.StackResource): checkers[0].start() return checkers + def _attribute_output_name(self, *attr_path): + if attr_path[0] == self.REFS: + return self.REFS + return ', '.join(six.text_type(a) for a in attr_path) + def get_attribute(self, key, *path): + if key == self.REMOVED_RSRC_LIST: + return self._current_blacklist() + if key == self.ATTR_ATTRIBUTES and not path: + raise exception.InvalidTemplateAttribute(resource=self.name, + key=key) + + is_resource_ref = (key.startswith("resource.") and + not path and (len(key.split('.', 2)) == 2)) + if is_resource_ref: + output_name = self.REFS_MAP + else: + output_name = self._attribute_output_name(key, *path) + try: + output = self.get_output(output_name) + except (exception.NotFound, + exception.TemplateOutputError) as op_err: + LOG.debug('Falling back to grouputils due to %s', op_err) + else: + if is_resource_ref: + try: + target = key.split('.', 2)[1] + return output[target] + except KeyError: + raise exception.NotFound(_("Member '%(mem)s' not " + "found in group resource " + "'%(grp)s'.") % + {'mem': target, + 'grp': self.name}) + if key == self.REFS: + return attributes.select_from_attribute(output, path) + return output + if key.startswith("resource."): return grouputils.get_nested_attrs(self, key, False, *path) @@ -467,12 +507,7 @@ class ResourceGroup(stack_resource.StackResource): refs_map = {n: grouputils.get_rsrc_id(self, key, False, n) for n in names} return refs_map - if key == self.REMOVED_RSRC_LIST: - return self._current_blacklist() if key == self.ATTR_ATTRIBUTES: - if not path: - raise exception.InvalidTemplateAttribute( - resource=self.name, key=key) return dict((n, grouputils.get_rsrc_attr( self, key, False, n, *path)) for n in names) @@ -484,10 +519,9 @@ class ResourceGroup(stack_resource.StackResource): for attr in self.referenced_attrs(): if isinstance(attr, six.string_types): key, path = attr, [] - output_name = attr else: key, path = attr[0], list(attr[1:]) - output_name = ', '.join(six.text_type(a) for a in attr) + output_name = self._attribute_output_name(key, *path) value = None if key.startswith("resource."): diff --git a/heat/engine/resources/openstack/heat/software_deployment.py b/heat/engine/resources/openstack/heat/software_deployment.py index 5bcc1680ce..64285ed971 100644 --- a/heat/engine/resources/openstack/heat/software_deployment.py +++ b/heat/engine/resources/openstack/heat/software_deployment.py @@ -739,7 +739,8 @@ class SoftwareDeploymentGroup(resource_group.ResourceGroup): for attr in self.referenced_attrs(): key = attr if isinstance(attr, six.string_types) else attr[0] n_attr = self._member_attribute_name(key) - output_name = '%s, %s' % (self.ATTR_ATTRIBUTES, n_attr) + output_name = self._attribute_output_name(self.ATTR_ATTRIBUTES, + n_attr) value = {r: get_attr_fn([r, n_attr]) for r in resource_names} yield output.OutputDefinition(output_name, value) diff --git a/heat/tests/openstack/heat/test_resource_chain.py b/heat/tests/openstack/heat/test_resource_chain.py index 11e8869e50..282500c499 100644 --- a/heat/tests/openstack/heat/test_resource_chain.py +++ b/heat/tests/openstack/heat/test_resource_chain.py @@ -340,6 +340,46 @@ class ResourceChainAttrTest(common.HeatTestCase): return chain def _stub_get_attr(self, chain, refids, attrs): + def ref_id_fn(res_name): + return refids[int(res_name)] + + def attr_fn(args): + res_name = args[0] + return attrs[int(res_name)] + + def get_output(output_name): + outputs = chain._nested_output_defns(chain._resource_names(), + attr_fn, ref_id_fn) + op_defns = {od.name: od for od in outputs} + if output_name not in op_defns: + raise exception.NotFound('Specified output key %s not found.' % + output_name) + return op_defns[output_name].get_value() + + orig_get_attr = chain.FnGetAtt + + def get_attr(attr_name, *path): + if not path: + attr = attr_name + else: + attr = (attr_name,) + path + # Mock referenced_attrs() so that _nested_output_definitions() + # will include the output required for this attribute + chain.referenced_attrs = mock.Mock(return_value=[attr]) + + # Pass through to actual function under test + return orig_get_attr(attr_name, *path) + + chain.FnGetAtt = mock.Mock(side_effect=get_attr) + chain.get_output = mock.Mock(side_effect=get_output) + + +class ResourceChainAttrFallbackTest(ResourceChainAttrTest): + def _stub_get_attr(self, chain, refids, attrs): + # Raise NotFound when getting output, to force fallback to old-school + # grouputils functions + chain.get_output = mock.Mock(side_effect=exception.NotFound) + def make_fake_res(idx): fr = mock.Mock() fr.stack = chain.stack diff --git a/heat/tests/openstack/heat/test_resource_group.py b/heat/tests/openstack/heat/test_resource_group.py index 95853ad3f7..180a47d622 100644 --- a/heat/tests/openstack/heat/test_resource_group.py +++ b/heat/tests/openstack/heat/test_resource_group.py @@ -985,13 +985,6 @@ class ResourceGroupAttrTest(common.HeatTestCase): self.assertIn("Member '2' not found in group resource 'group1'.", six.text_type(ex)) - @mock.patch.object(grouputils, 'get_rsrc_id') - def test_get_attribute(self, mock_get_rsrc_id): - stack = utils.parse_stack(template) - mock_get_rsrc_id.side_effect = ['0', '1'] - rsrc = stack['group1'] - self.assertEqual(['0', '1'], rsrc.FnGetAtt(rsrc.REFS)) - def test_get_attribute_convg(self): cache_data = {'group1': node_data.NodeData.from_dict({ 'uuid': mock.ANY, @@ -1030,6 +1023,44 @@ class ResourceGroupAttrTest(common.HeatTestCase): return resg def _stub_get_attr(self, resg, refids, attrs): + def ref_id_fn(res_name): + return refids[int(res_name)] + + def attr_fn(args): + res_name = args[0] + return attrs[int(res_name)] + + def get_output(output_name): + outputs = resg._nested_output_defns(resg._resource_names(), + attr_fn, ref_id_fn) + op_defns = {od.name: od for od in outputs} + self.assertIn(output_name, op_defns) + return op_defns[output_name].get_value() + + orig_get_attr = resg.FnGetAtt + + def get_attr(attr_name, *path): + if not path: + attr = attr_name + else: + attr = (attr_name,) + path + # Mock referenced_attrs() so that _nested_output_definitions() + # will include the output required for this attribute + resg.referenced_attrs = mock.Mock(return_value=[attr]) + + # Pass through to actual function under test + return orig_get_attr(attr_name, *path) + + resg.FnGetAtt = mock.Mock(side_effect=get_attr) + resg.get_output = mock.Mock(side_effect=get_output) + + +class ResourceGroupAttrFallbackTest(ResourceGroupAttrTest): + def _stub_get_attr(self, resg, refids, attrs): + # Raise NotFound when getting output, to force fallback to old-school + # grouputils functions + resg.get_output = mock.Mock(side_effect=exception.NotFound) + def make_fake_res(idx): fr = mock.Mock() fr.stack = resg.stack @@ -1040,6 +1071,14 @@ class ResourceGroupAttrTest(common.HeatTestCase): fake_res = {str(i): make_fake_res(i) for i in refids} resg.nested = mock.Mock(return_value=fake_res) + @mock.patch.object(grouputils, 'get_rsrc_id') + def test_get_attribute(self, mock_get_rsrc_id): + stack = utils.parse_stack(template) + mock_get_rsrc_id.side_effect = ['0', '1'] + rsrc = stack['group1'] + rsrc.get_output = mock.Mock(side_effect=exception.NotFound) + self.assertEqual(['0', '1'], rsrc.FnGetAtt(rsrc.REFS)) + class ReplaceTest(common.HeatTestCase): # 1. no min_in_service diff --git a/heat/tests/openstack/heat/test_software_deployment.py b/heat/tests/openstack/heat/test_software_deployment.py index 77fa4ff40d..5042de2499 100644 --- a/heat/tests/openstack/heat/test_software_deployment.py +++ b/heat/tests/openstack/heat/test_software_deployment.py @@ -1602,6 +1602,47 @@ class SoftwareDeploymentGroupAttrTest(common.HeatTestCase): return resg def _stub_get_attr(self, resg): + def ref_id_fn(args): + self.fail('Getting member reference ID for some reason') + + def attr_fn(args): + res_name = args[0] + return self.values[self.server_names.index(res_name)] + + def get_output(output_name): + outputs = resg._nested_output_defns(resg._resource_names(), + attr_fn, ref_id_fn) + op_defns = {od.name: od for od in outputs} + self.assertIn(output_name, op_defns) + return op_defns[output_name].get_value() + + orig_get_attr = resg.FnGetAtt + + def get_attr(attr_name, *path): + if not path: + attr = attr_name + else: + attr = (attr_name,) + path + # Mock referenced_attrs() so that _nested_output_definitions() + # will include the output required for this attribute + resg.referenced_attrs = mock.Mock(return_value=[attr]) + + # Pass through to actual function under test + return orig_get_attr(attr_name, *path) + + resg.FnGetAtt = mock.Mock(side_effect=get_attr) + resg.get_output = mock.Mock(side_effect=get_output) + + def check_calls(self, count=1): + pass + + +class SoftwareDeploymentGroupAttrFallbackTest(SoftwareDeploymentGroupAttrTest): + def _stub_get_attr(self, resg): + # Raise NotFound when getting output, to force fallback to old-school + # grouputils functions + resg.get_output = mock.Mock(side_effect=exc.NotFound) + for server, value in zip(self.servers, self.values): server.FnGetAtt.return_value = value