From 5d070f9c59b3218d45c23c2936e223552fe81d81 Mon Sep 17 00:00:00 2001 From: Rakesh H S Date: Thu, 7 Jan 2016 17:54:06 +0530 Subject: [PATCH] Convergence: Fix FnGetAtt to fetch value from cache_data Resource plugins will now override rsrc.get_attribute method to add extra logic to Fn::GetAtt specific to resource implementation. Also provides proper comment to rsrc.get_reference_id method. Change-Id: I7561afa1c824b2294dd6701be6a19723abbe0522 Closes-Bug: #1531840 --- heat/engine/cfn/functions.py | 2 +- heat/engine/resource.py | 28 +++++++++++++------ heat/engine/resources/aws/cfn/stack.py | 2 +- .../openstack/heat/autoscaling_group.py | 2 +- .../resources/openstack/heat/none_resource.py | 2 +- .../openstack/heat/resource_chain.py | 2 +- .../openstack/heat/resource_group.py | 2 +- .../openstack/heat/software_deployment.py | 6 ++-- heat/engine/resources/template_resource.py | 2 +- heat/tests/generic_resource.py | 4 +-- .../openstack/heat/test_resource_chain.py | 20 +++++++++++++ .../openstack/heat/test_resource_group.py | 20 +++++++++++++ .../heat/test_software_deployment.py | 16 +++++++++-- heat/tests/test_nested_stack.py | 17 +++++++++++ heat/tests/test_stack_resource.py | 22 +++++++++++++++ 15 files changed, 125 insertions(+), 22 deletions(-) diff --git a/heat/engine/cfn/functions.py b/heat/engine/cfn/functions.py index 5087f8c4c1..b2693a3b06 100644 --- a/heat/engine/cfn/functions.py +++ b/heat/engine/cfn/functions.py @@ -190,7 +190,7 @@ class GetAtt(function.Function): attr = function.resolve(self._attribute) from heat.engine import resource - if (type(res).FnGetAtt == resource.Resource.FnGetAtt and + if (type(res).get_attribute == resource.Resource.get_attribute and attr not in six.iterkeys(res.attributes_schema)): raise exception.InvalidTemplateAttribute( resource=self._resource_name, key=attr) diff --git a/heat/engine/resource.py b/heat/engine/resource.py index 867b5d4415..66222846cb 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -1566,6 +1566,11 @@ class Resource(object): return (self.action, self.status) def get_reference_id(self): + """Default implementation for function get_resource. + + This may be overridden by resource plugins to add extra + logic specific to the resource implementation. + """ if self.resource_id is not None: return six.text_type(self.resource_id) else: @@ -1587,6 +1592,20 @@ class Resource(object): else: return Resource.get_reference_id(self) + def get_attribute(self, key, *path): + """Default implementation for function get_attr and Fn::GetAtt. + + This may be overridden by resource plugins to add extra + logic specific to the resource implementation. + """ + try: + attribute = self.attributes[key] + except KeyError: + raise exception.InvalidTemplateAttribute(resource=self.name, + key=key) + + return attributes.select_from_attribute(attribute, path) + def FnGetAtt(self, key, *path): """For the intrinsic function Fn::GetAtt. @@ -1602,14 +1621,7 @@ class Resource(object): attribute = self.stack.cache_data_resource_attribute( self.name, complex_key) return attribute - else: - try: - attribute = self.attributes[key] - except KeyError: - raise exception.InvalidTemplateAttribute(resource=self.name, - key=key) - - return attributes.select_from_attribute(attribute, path) + return self.get_attribute(key, *path) def FnGetAtts(self): """For the intrinsic function get_attr which returns all attributes. diff --git a/heat/engine/resources/aws/cfn/stack.py b/heat/engine/resources/aws/cfn/stack.py index 1be93f5455..0f66de96d4 100644 --- a/heat/engine/resources/aws/cfn/stack.py +++ b/heat/engine/resources/aws/cfn/stack.py @@ -80,7 +80,7 @@ class NestedStack(stack_resource.StackResource): self.properties[self.TIMEOUT_IN_MINS], adopt_data=resource_adopt_data) - def FnGetAtt(self, key, *path): + def get_attribute(self, key, *path): if key and not key.startswith('Outputs.'): raise exception.InvalidTemplateAttribute(resource=self.name, key=key) diff --git a/heat/engine/resources/openstack/heat/autoscaling_group.py b/heat/engine/resources/openstack/heat/autoscaling_group.py index 3c0135dfe1..277f4fe629 100644 --- a/heat/engine/resources/openstack/heat/autoscaling_group.py +++ b/heat/engine/resources/openstack/heat/autoscaling_group.py @@ -162,7 +162,7 @@ class AutoScalingResourceGroup(aws_asg.AutoScalingGroup): self)._create_template(num_instances, num_replace, template_version=template_version) - def FnGetAtt(self, key, *path): + def get_attribute(self, key, *path): if key == self.CURRENT_SIZE: return grouputils.get_size(self) if path: diff --git a/heat/engine/resources/openstack/heat/none_resource.py b/heat/engine/resources/openstack/heat/none_resource.py index f889001554..07afc6cb12 100644 --- a/heat/engine/resources/openstack/heat/none_resource.py +++ b/heat/engine/resources/openstack/heat/none_resource.py @@ -45,7 +45,7 @@ class NoneResource(resource.Resource): def validate(self): pass - def FnGetAtt(self, key, *path): + def get_attribute(self, key, *path): return None diff --git a/heat/engine/resources/openstack/heat/resource_chain.py b/heat/engine/resources/openstack/heat/resource_chain.py index 2b47bf6c5f..bd1435fef3 100644 --- a/heat/engine/resources/openstack/heat/resource_chain.py +++ b/heat/engine/resources/openstack/heat/resource_chain.py @@ -151,7 +151,7 @@ class ResourceChain(stack_resource.StackResource): def child_params(self): return {} - def FnGetAtt(self, key, *path): + def get_attribute(self, key, *path): if key.startswith('resource.'): return grouputils.get_nested_attrs(self, key, False, *path) diff --git a/heat/engine/resources/openstack/heat/resource_group.py b/heat/engine/resources/openstack/heat/resource_group.py index 5f1a2fa56c..e532967f80 100644 --- a/heat/engine/resources/openstack/heat/resource_group.py +++ b/heat/engine/resources/openstack/heat/resource_group.py @@ -408,7 +408,7 @@ class ResourceGroup(stack_resource.StackResource): checkers[0].start() return checkers - def FnGetAtt(self, key, *path): + def get_attribute(self, key, *path): if key.startswith("resource."): return grouputils.get_nested_attrs(self, key, False, *path) diff --git a/heat/engine/resources/openstack/heat/software_deployment.py b/heat/engine/resources/openstack/heat/software_deployment.py index cdbb38af21..4de0612705 100644 --- a/heat/engine/resources/openstack/heat/software_deployment.py +++ b/heat/engine/resources/openstack/heat/software_deployment.py @@ -513,7 +513,7 @@ class SoftwareDeployment(signal_responder.SignalResponder): self.context, self.resource_id, details, timeutils.utcnow().isoformat()) - def FnGetAtt(self, key, *path): + def get_attribute(self, key, *path): """Resource attributes map to deployment outputs values.""" sd = self.rpc_client().show_software_deployment( self.context, self.resource_id) @@ -640,7 +640,7 @@ class SoftwareDeploymentGroup(resource_group.ResourceGroup): 'OS::Heat::SoftwareDeployment', props, None) - def FnGetAtt(self, key, *path): + def get_attribute(self, key, *path): rg = super(SoftwareDeploymentGroup, self) if key == self.STDOUTS: n_attr = SoftwareDeployment.STDOUT @@ -653,7 +653,7 @@ class SoftwareDeploymentGroup(resource_group.ResourceGroup): # including arbitrary outputs, so we can't validate here n_attr = key - rg_attr = rg.FnGetAtt(rg.ATTR_ATTRIBUTES, n_attr) + rg_attr = rg.get_attribute(rg.ATTR_ATTRIBUTES, n_attr) return attributes.select_from_attribute(rg_attr, path) diff --git a/heat/engine/resources/template_resource.py b/heat/engine/resources/template_resource.py index 7dea82d825..b2400bb830 100644 --- a/heat/engine/resources/template_resource.py +++ b/heat/engine/resources/template_resource.py @@ -313,7 +313,7 @@ class TemplateResource(stack_resource.StackResource): return self.nested().identifier().arn() - def FnGetAtt(self, key, *path): + def get_attribute(self, key, *path): stack = self.nested() if stack is None: return None diff --git a/heat/tests/generic_resource.py b/heat/tests/generic_resource.py index 94bced490a..3ef4ab8114 100644 --- a/heat/tests/generic_resource.py +++ b/heat/tests/generic_resource.py @@ -241,12 +241,12 @@ class ResourceWithDefaultClientNameExt(resource.Resource): class ResourceWithFnGetAttType(GenericResource): - def FnGetAtt(self, name): + def get_attribute(self, name): pass class ResourceWithFnGetRefIdType(ResourceWithProps): - def FnGetRefId(self): + def get_reference_id(self): return 'ID-%s' % self.name diff --git a/heat/tests/openstack/heat/test_resource_chain.py b/heat/tests/openstack/heat/test_resource_chain.py index b7739e9202..e67a1003c5 100644 --- a/heat/tests/openstack/heat/test_resource_chain.py +++ b/heat/tests/openstack/heat/test_resource_chain.py @@ -15,6 +15,7 @@ import copy import mock from heat.common.exception import StackValidationFailed +from heat.common import grouputils from heat.engine.resources.openstack.heat import resource_chain from heat.tests import common from heat.tests import utils @@ -205,3 +206,22 @@ class ResourceChainTests(common.HeatTestCase): snip = self.stack.t.resource_definitions(self.stack)['test-chain'] chain = resource_chain.ResourceChain('test', snip, self.stack) return chain + + @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['test-chain'] + self.assertEqual(['0', '1'], rsrc.FnGetAtt(rsrc.REFS)) + + def test_get_attribute_convg(self): + cache_data = {'test-chain': { + 'uuid': mock.ANY, + 'id': mock.ANY, + 'action': 'CREATE', + 'status': 'COMPLETE', + 'attrs': {'refs': ['rsrc1', 'rsrc2']} + }} + stack = utils.parse_stack(TEMPLATE, cache_data=cache_data) + rsrc = stack['test-chain'] + self.assertEqual(['rsrc1', 'rsrc2'], rsrc.FnGetAtt(rsrc.REFS)) diff --git a/heat/tests/openstack/heat/test_resource_group.py b/heat/tests/openstack/heat/test_resource_group.py index 65c2cfdc8a..9e7a7745e9 100644 --- a/heat/tests/openstack/heat/test_resource_group.py +++ b/heat/tests/openstack/heat/test_resource_group.py @@ -769,6 +769,25 @@ class ResourceGroupAttrTest(common.HeatTestCase): self.assertRaises(exception.InvalidTemplateAttribute, resg.FnGetAtt, 'resource.2') + @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': { + 'uuid': mock.ANY, + 'id': mock.ANY, + 'action': 'CREATE', + 'status': 'COMPLETE', + 'attrs': {'refs': ['rsrc1', 'rsrc2']} + }} + stack = utils.parse_stack(template, cache_data=cache_data) + rsrc = stack['group1'] + self.assertEqual(['rsrc1', 'rsrc2'], rsrc.FnGetAtt(rsrc.REFS)) + def _create_dummy_stack(self, template_data=template, expect_count=2, expect_attrs=None): stack = utils.parse_stack(template_data) @@ -779,6 +798,7 @@ class ResourceGroupAttrTest(common.HeatTestCase): for resc in range(expect_count): res = str(resc) fake_res[res] = mock.Mock() + fake_res[res].stack = stack fake_res[res].FnGetRefId.return_value = 'ID-%s' % res if res in expect_attrs: fake_res[res].FnGetAtt.return_value = expect_attrs[res] diff --git a/heat/tests/openstack/heat/test_software_deployment.py b/heat/tests/openstack/heat/test_software_deployment.py index 7f73532984..7f0cf561f5 100644 --- a/heat/tests/openstack/heat/test_software_deployment.py +++ b/heat/tests/openstack/heat/test_software_deployment.py @@ -160,12 +160,13 @@ class SoftwareDeploymentTest(common.HeatTestCase): super(SoftwareDeploymentTest, self).setUp() self.ctx = utils.dummy_context() - def _create_stack(self, tmpl): + def _create_stack(self, tmpl, cache_data=None): self.stack = parser.Stack( self.ctx, 'software_deployment_test_stack', template.Template(tmpl), stack_id='42f6f66b-631a-44e7-8d01-e22fb54574a9', - stack_user_project_id='65728b74-cfe7-4f17-9c15-11d4f686e591' + stack_user_project_id='65728b74-cfe7-4f17-9c15-11d4f686e591', + cache_data=cache_data ) self.patchobject(nova.NovaClientPlugin, 'get_server', @@ -917,6 +918,17 @@ class SoftwareDeploymentTest(common.HeatTestCase): self.deployment.FnGetAtt('deploy_stderr')) self.assertEqual(0, self.deployment.FnGetAtt('deploy_status_code')) + def test_fn_get_att_convg(self): + cache_data = {'deployment_mysql': { + 'uuid': mock.ANY, + 'id': mock.ANY, + 'action': 'CREATE', + 'status': 'COMPLETE', + 'attrs': {'foo': 'bar'} + }} + self._create_stack(self.template, cache_data=cache_data) + self.assertEqual('bar', self.deployment.FnGetAtt('foo')) + def test_fn_get_att_error(self): self._create_stack(self.template) diff --git a/heat/tests/test_nested_stack.py b/heat/tests/test_nested_stack.py index 0b5fa03543..e08add6238 100644 --- a/heat/tests/test_nested_stack.py +++ b/heat/tests/test_nested_stack.py @@ -286,6 +286,23 @@ Resources: nested_stack = stack['the_nested'] self.assertEqual('the_nested_convg_mock', nested_stack.FnGetRefId()) + def test_get_attribute(self): + tmpl = template_format.parse(self.test_template) + ctx = utils.dummy_context('test_username', 'aaaa', 'password') + stack = parser.Stack(ctx, 'test', + template.Template(tmpl)) + stack.store() + + stack_res = stack['the_nested'] + stack_res._store() + + nested_t = template_format.parse(self.nested_template) + nested_stack = parser.Stack(ctx, 'test', + template.Template(nested_t)) + nested_stack.store() + stack_res.nested = mock.Mock(return_value=nested_stack) + self.assertEqual('bar', stack_res.FnGetAtt('Outputs.Foo')) + class ResDataResource(generic_rsrc.GenericResource): def handle_create(self): diff --git a/heat/tests/test_stack_resource.py b/heat/tests/test_stack_resource.py index e1c8e85331..ff04987ce5 100644 --- a/heat/tests/test_stack_resource.py +++ b/heat/tests/test_stack_resource.py @@ -428,6 +428,28 @@ class StackResourceTest(StackResourceBaseTest): self._test_validate_unknown_resource_type(stack_name, tmpl, 'my_autoscaling_group') + def test_get_attribute_autoscaling(self): + t = template_format.parse(heat_autoscaling_group_template) + tmpl = templatem.Template(t) + stack = parser.Stack(utils.dummy_context(), 'test_att', tmpl) + rsrc = stack['my_autoscaling_group'] + self.assertEqual(0, rsrc.FnGetAtt(rsrc.CURRENT_SIZE)) + + def test_get_attribute_autoscaling_convg(self): + t = template_format.parse(heat_autoscaling_group_template) + tmpl = templatem.Template(t) + cache_data = {'my_autoscaling_group': { + 'uuid': mock.ANY, + 'id': mock.ANY, + 'action': 'CREATE', + 'status': 'COMPLETE', + 'attrs': {'current_size': 4} + }} + stack = parser.Stack(utils.dummy_context(), 'test_att', tmpl, + cache_data=cache_data) + rsrc = stack['my_autoscaling_group'] + self.assertEqual(4, rsrc.FnGetAtt(rsrc.CURRENT_SIZE)) + def test__validate_nested_resources_checks_num_of_resources(self): stack_resource.cfg.CONF.set_override('max_resources_per_stack', 2) tmpl = {'HeatTemplateFormatVersion': '2012-12-12',