From 02275f890196f74740d44cc9eeb6d5f456fe15e4 Mon Sep 17 00:00:00 2001 From: Sergey Kraynev Date: Thu, 25 Jun 2015 09:05:53 -0400 Subject: [PATCH] Add base resolve method for attributes Current patch adds two new methods to base class: * _resolve_all_attributes - for choosing path for resolving, which depends on type of attribute (base or custom). * _show_resource - method for resolving base 'show' attribute. This method should be re-defined for resources, which use clients with specific implementations of 'show' commands. New attribute for base class was added - entity, which is used for resolving 'show' attribute. Tests for Neutron resources and Server resource were updated corresponding changes mentioned above. Hook for stack_resource class is removed in this patch. Change-Id: I4ee0a53407739e5590c9a8d12bc7c1e84077f4c4 --- heat/engine/api.py | 25 ++++--- heat/engine/resource.py | 44 ++++++++++- .../resources/openstack/neutron/neutron.py | 16 ++-- .../engine/resources/openstack/nova/server.py | 4 +- heat/engine/resources/stack_resource.py | 13 ++-- heat/engine/resources/template_resource.py | 2 +- heat/tests/generic_resource.py | 16 +--- heat/tests/neutron/test_neutron.py | 15 +++- heat/tests/test_engine_api_utils.py | 8 +- heat/tests/test_resource.py | 74 +++++++++++++++++++ 10 files changed, 169 insertions(+), 48 deletions(-) diff --git a/heat/engine/api.py b/heat/engine/api.py index 67042ef8f..be923d902 100644 --- a/heat/engine/api.py +++ b/heat/engine/api.py @@ -11,6 +11,8 @@ # License for the specific language governing permissions and limitations # under the License. +import collections + from oslo_log import log as logging from oslo_utils import timeutils import six @@ -159,15 +161,20 @@ def format_resource_attributes(resource, with_attr=None): # 'console_urls' for nova server, user can view it by taking with_attr # parameter if 'show' in six.iterkeys(resolver): - show_attr = resolver['show'] - for a in with_attr: - if a not in show_attr: - show_attr[a] = resolve(a, resolver) - return show_attr - else: - attributes = set(list(six.iterkeys(resolver)) + with_attr) - return dict((attr, resolve(attr, resolver)) - for attr in attributes) + show_attr = resolve('show', resolver) + # check if 'show' resolved to dictionary. so it's not None + if isinstance(show_attr, collections.Mapping): + for a in with_attr: + if a not in show_attr: + show_attr[a] = resolve(a, resolver) + return show_attr + else: + # remove 'show' attribute if it's None or not a mapping + # then resolve all attributes manually + del resolver._attributes['show'] + attributes = set(list(six.iterkeys(resolver)) + with_attr) + return dict((attr, resolve(attr, resolver)) + for attr in attributes) def format_resource_properties(resource): diff --git a/heat/engine/resource.py b/heat/engine/resource.py index 8e440dd85..7fc2f18e8 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -122,6 +122,10 @@ class Resource(object): # Resource implementations set this to update policies update_policy_schema = {} + # Default entity of resource, which is used for during resolving + # show attribute + entity = None + # Description dictionary, that describes the common attributes for all # resources base_attributes_schema = { @@ -194,7 +198,7 @@ class Resource(object): self.attributes_schema.update(self.base_attributes_schema) self.attributes = attributes.Attributes(self.name, self.attributes_schema, - self._resolve_attribute) + self._resolve_all_attributes) self.abandon_in_progress = False @@ -1308,6 +1312,44 @@ class Resource(object): if not updated_ok: LOG.warn(_LW('Failed to unlock resource %s'), rsrc.name) + def _resolve_all_attributes(self, attr): + """Method for resolving all attributes. + + This method uses basic _resolve_attribute method for resolving + specific attributes. Base attributes will be resolved with + corresponding method, which should be defined in each resource + class. + + :param attr: attribute name, which will be resolved + :returns: method of resource class, which resolve base attribute + """ + if attr in self.base_attributes_schema: + # check resource_id, because usually it is required for getting + # information about resource + if not self.resource_id: + return None + try: + return getattr(self, '_{0}_resource'.format(attr))() + except Exception as ex: + self.client_plugin().ignore_not_found(ex) + return None + else: + return self._resolve_attribute(attr) + + def _show_resource(self): + """Default implementation; should be overridden by resources + + :returns: the map of resource information or None + """ + if self.entity: + try: + obj = getattr(self.client(), self.entity) + resource = obj.get(self.resource_id) + return resource.to_dict() + except AttributeError as ex: + LOG.warn(_LW("Resolving 'show' attribute has failed : %s"), ex) + return None + def _resolve_attribute(self, name): """ Default implementation; should be overridden by resources that expose diff --git a/heat/engine/resources/openstack/neutron/neutron.py b/heat/engine/resources/openstack/neutron/neutron.py index da381e49e..bfddd7ef6 100644 --- a/heat/engine/resources/openstack/neutron/neutron.py +++ b/heat/engine/resources/openstack/neutron/neutron.py @@ -119,16 +119,12 @@ class NeutronResource(resource.Resource): result=_('Resource is not built')) def _resolve_attribute(self, name): - if self.resource_id: - try: - attributes = self._show_resource() - except Exception as ex: - self.client_plugin().ignore_not_found(ex) - return None - if name == 'show': - return attributes - - return attributes[name] + try: + attributes = self._show_resource() + except Exception as ex: + self.client_plugin().ignore_not_found(ex) + return None + return attributes[name] def FnGetRefId(self): return six.text_type(self.resource_id) diff --git a/heat/engine/resources/openstack/nova/server.py b/heat/engine/resources/openstack/nova/server.py index 1a255385e..cf8de5290 100644 --- a/heat/engine/resources/openstack/nova/server.py +++ b/heat/engine/resources/openstack/nova/server.py @@ -516,6 +516,8 @@ class Server(stack_user.StackUser): default_client_name = 'nova' + entity = 'servers' + def translation_rules(self): return [properties.TranslationRule( self.properties, @@ -934,8 +936,6 @@ class Server(stack_user.StackUser): return server.accessIPv4 if name == self.ACCESSIPV6: return server.accessIPv6 - if name == self.SHOW: - return server._info if name == self.CONSOLE_URLS: return self.client_plugin('nova').get_console_urls(server) diff --git a/heat/engine/resources/stack_resource.py b/heat/engine/resources/stack_resource.py index 24ab0cb6f..d23a05a1f 100644 --- a/heat/engine/resources/stack_resource.py +++ b/heat/engine/resources/stack_resource.py @@ -79,9 +79,11 @@ class StackResource(resource.Resource): if not self.attributes and outputs: self.attributes_schema = ( attributes.Attributes.schema_from_outputs(outputs)) - self.attributes = attributes.Attributes(self.name, - self.attributes_schema, - self._resolve_attribute) + # Note: it can be updated too and for show return dictionary + # with all available outputs + self.attributes = attributes.Attributes( + self.name, self.attributes_schema, + self._resolve_all_attributes) def _needs_update(self, after, before, after_props, before_props, prev_resource): @@ -525,10 +527,7 @@ class StackResource(resource.Resource): return result def _resolve_attribute(self, name): - # NOTE(skraynev): should be removed in patch with methods, - # which resolve base attributes - if name != 'show': - return self.get_output(name) + return self.get_output(name) def implementation_signature(self): schema_names = ([prop for prop in self.properties_schema] + diff --git a/heat/engine/resources/template_resource.py b/heat/engine/resources/template_resource.py index e21414ebf..af0e23631 100644 --- a/heat/engine/resources/template_resource.py +++ b/heat/engine/resources/template_resource.py @@ -117,7 +117,7 @@ class TemplateResource(stack_resource.StackResource): self.attributes_schema.update(self.base_attributes_schema) self.attributes = attributes.Attributes(self.name, self.attributes_schema, - self._resolve_attribute) + self._resolve_all_attributes) def child_params(self): ''' diff --git a/heat/tests/generic_resource.py b/heat/tests/generic_resource.py index 56da97630..85d086ef5 100644 --- a/heat/tests/generic_resource.py +++ b/heat/tests/generic_resource.py @@ -63,18 +63,10 @@ class GenericResource(resource.Resource): class ResWithShowAttr(GenericResource): - attributes_schema = {'foo': attributes.Schema('A generic attribute'), - 'Foo': attributes.Schema('Another generic attribute'), - 'show': attributes.Schema('A dict of all details ' - 'of attributes')} - - def _resolve_attribute(self, name): - if name == 'show': - return {'foo': self.name, - 'Foo': self.name, - 'Another': self.name} - else: - return self.name + def _show_resource(self): + return {'foo': self.name, + 'Foo': self.name, + 'Another': self.name} class ResWithComplexPropsAndAttrs(GenericResource): diff --git a/heat/tests/neutron/test_neutron.py b/heat/tests/neutron/test_neutron.py index e5e0f95e8..b46dc8c99 100644 --- a/heat/tests/neutron/test_neutron.py +++ b/heat/tests/neutron/test_neutron.py @@ -120,19 +120,26 @@ class NeutronTest(common.HeatTestCase): mock_show_resource.side_effect = [{'attr1': 'val1', 'attr2': 'val2'}, {'attr1': 'val1', 'attr2': 'val2'}, {'attr1': 'val1', 'attr2': 'val2'}, - qe.NotFound, - qe.NeutronClientException] + qe.NotFound] res._show_resource = mock_show_resource nclientplugin = neutron.NeutronClientPlugin(mock.MagicMock()) res.client_plugin = mock.Mock(return_value=nclientplugin) self.assertEqual({'attr1': 'val1', 'attr2': 'val2'}, - res._resolve_attribute('show')) + res.FnGetAtt('show')) self.assertEqual('val2', res._resolve_attribute('attr2')) self.assertRaises(KeyError, res._resolve_attribute, 'attr3') self.assertIsNone(res._resolve_attribute('attr2')) + res.resource_id = None - self.assertIsNone(res._resolve_attribute('show')) + # use local cached object + self.assertEqual({'attr1': 'val1', 'attr2': 'val2'}, + res.FnGetAtt('show')) + # reset cache and call 'show' again, so resolver should be used again + # and return None due to resource_id is None + with mock.patch.object(res.attributes, '_resolved_values') as res_vals: + res_vals.return_value = {} + self.assertIsNone(res.FnGetAtt('show')) class GetSecGroupUuidTest(common.HeatTestCase): diff --git a/heat/tests/test_engine_api_utils.py b/heat/tests/test_engine_api_utils.py index 1b8624fbc..96cc732e8 100644 --- a/heat/tests/test_engine_api_utils.py +++ b/heat/tests/test_engine_api_utils.py @@ -110,12 +110,15 @@ class FormatTest(common.HeatTestCase): def test_format_resource_attributes(self): res = self.stack['generic1'] - # the _resolve_attribute method of 'generic1' return res.name + # the _resolve_attribute method of 'generic1' returns map with all + # attributes except 'show' (because it's None in this test) formatted_attributes = api.format_resource_attributes(res) - self.assertEqual(res.name, formatted_attributes) + expected = {'foo': 'generic1', 'Foo': 'generic1'} + self.assertEqual(expected, formatted_attributes) def test_format_resource_attributes_show_attribute(self): res = self.stack['generic3'] + res.resource_id = 'generic3_id' formatted_attributes = api.format_resource_attributes(res) self.assertEqual(3, len(formatted_attributes)) self.assertIn('foo', formatted_attributes) @@ -124,6 +127,7 @@ class FormatTest(common.HeatTestCase): def test_format_resource_attributes_show_attribute_with_attr(self): res = self.stack['generic3'] + res.resource_id = 'generic3_id' formatted_attributes = api.format_resource_attributes( res, with_attr=['c']) self.assertEqual(4, len(formatted_attributes)) diff --git a/heat/tests/test_resource.py b/heat/tests/test_resource.py index f8cf4d2eb..0abc17aed 100644 --- a/heat/tests/test_resource.py +++ b/heat/tests/test_resource.py @@ -1645,6 +1645,80 @@ class ResourceTest(common.HeatTestCase): self.assertTrue(mock_get_obj.called) self.assertFalse(db_res.select_and_update.called) + def create_resource_for_attributes_tests(self): + tmpl = template.Template({ + 'heat_template_version': '2013-05-23', + 'resources': { + 'res': { + 'type': 'GenericResourceType' + } + } + }) + stack = parser.Stack(utils.dummy_context(), 'test', tmpl) + return stack + + def test_resolve_attributes_stuff_base_attribute(self): + # check path with resolving base attributes (via 'show' attribute) + stack = self.create_resource_for_attributes_tests() + res = stack['res'] + + with mock.patch.object(res, '_show_resource') as show_attr: + # return None, if resource_id is None + self.assertIsNone(res.FnGetAtt('show')) + + # set resource_id and recheck with re-written _show_resource + res.resource_id = mock.Mock() + + show_attr.return_value = 'my attr' + self.assertEqual('my attr', res.FnGetAtt('show')) + self.assertEqual(1, show_attr.call_count) + + # clean resolved_values + with mock.patch.object(res.attributes, '_resolved_values') as r_v: + with mock.patch.object(res, 'client_plugin') as client_plug: + r_v.return_value = {} + # generate error during calling _show_resource + show_attr.side_effect = [Exception] + self.assertIsNone(res.FnGetAtt('show')) + self.assertEqual(2, show_attr.call_count) + self.assertEqual(1, client_plug.call_count) + + def test_resolve_attributes_stuff_custom_attribute(self): + # check path with resolve_attribute + stack = self.create_resource_for_attributes_tests() + res = stack['res'] + + with mock.patch.object(res, '_resolve_attribute') as res_attr: + res.FnGetAtt('Foo') + res_attr.assert_called_once_with('Foo') + + def test_show_resource(self): + # check default function _show_resource + stack = self.create_resource_for_attributes_tests() + res = stack['res'] + + # check default value of entity + self.assertIsNone(res.entity) + self.assertIsNone(res.FnGetAtt('show')) + + # set entity and recheck + res.resource_id = 'test_resource_id' + res.entity = 'test' + + # mock gettring resource info + res.client = mock.Mock() + test_obj = mock.Mock() + test_resource = mock.Mock() + test_resource.to_dict.return_value = {'test': 'info'} + test_obj.get.return_value = test_resource + res.client().test = test_obj + + self.assertEqual({'test': 'info'}, res._show_resource()) + + # check handling AttributeError exception + test_obj.get.side_effect = AttributeError + self.assertIsNone(res._show_resource()) + class ResourceAdoptTest(common.HeatTestCase):