From b9279e767671d7b306b765aca12978f01701dfa7 Mon Sep 17 00:00:00 2001 From: Sergey Kraynev Date: Fri, 14 Aug 2015 04:09:20 -0400 Subject: [PATCH] Handle NotFound error for all _resolve_attributes Some of resources have try/except logic in their _resolve_attribute methods. This patch move this logic to one level up for handling it resolve_all_attributes. It should allow avoid overhead coding in resource with custom _resolve_attribute method. Change-Id: Id564c8ccfb2bc42eb522fcac98e81054502cdc65 --- heat/engine/resource.py | 6 ++++- .../resources/aws/ec2/network_interface.py | 11 ++++----- .../resources/openstack/heat/remote_stack.py | 7 +----- .../openstack/mistral/cron_trigger.py | 6 +---- .../resources/openstack/neutron/neutron.py | 6 +---- heat/tests/neutron/test_neutron.py | 13 +++++----- heat/tests/nova/test_server.py | 6 ++--- heat/tests/test_resource.py | 24 ++++++++++++------- 8 files changed, 36 insertions(+), 43 deletions(-) diff --git a/heat/engine/resource.py b/heat/engine/resource.py index e98e595ccd..82fd7f1c3a 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -1381,7 +1381,11 @@ class Resource(object): self.client_plugin().ignore_not_found(ex) return None else: - return self._resolve_attribute(attr) + try: + return self._resolve_attribute(attr) + except Exception as ex: + self.client_plugin().ignore_not_found(ex) + return None def _show_resource(self): """Default implementation; should be overridden by resources diff --git a/heat/engine/resources/aws/ec2/network_interface.py b/heat/engine/resources/aws/ec2/network_interface.py index a4d7caa5de..27f9013fa9 100644 --- a/heat/engine/resources/aws/ec2/network_interface.py +++ b/heat/engine/resources/aws/ec2/network_interface.py @@ -158,14 +158,11 @@ class NetworkInterface(resource.Resource): self.client().update_port(self.resource_id, {'port': update_props}) - def _get_fixed_ip_address(self, ): + def _get_fixed_ip_address(self): if self.fixed_ip_address is None: - try: - port = self.client().show_port(self.resource_id)['port'] - if port['fixed_ips'] and len(port['fixed_ips']) > 0: - self.fixed_ip_address = port['fixed_ips'][0]['ip_address'] - except Exception as ex: - self.client_plugin().ignore_not_found(ex) + port = self.client().show_port(self.resource_id)['port'] + if port['fixed_ips'] and len(port['fixed_ips']) > 0: + self.fixed_ip_address = port['fixed_ips'][0]['ip_address'] return self.fixed_ip_address diff --git a/heat/engine/resources/openstack/heat/remote_stack.py b/heat/engine/resources/openstack/heat/remote_stack.py index 375404e0be..5c1a81de3f 100644 --- a/heat/engine/resources/openstack/heat/remote_stack.py +++ b/heat/engine/resources/openstack/heat/remote_stack.py @@ -290,12 +290,7 @@ class RemoteStack(resource.Resource): return self._check_action_complete(action=self.CHECK) def _resolve_attribute(self, name): - try: - stack = self.heat().stacks.get(stack_id=self.resource_id) - except Exception as e: - self.client_plugin().ignore_not_found(e) - return None - + stack = self.heat().stacks.get(stack_id=self.resource_id) if name == self.NAME_ATTR: value = getattr(stack, name, None) return value or self.physical_resource_name_or_FnGetRefId() diff --git a/heat/engine/resources/openstack/mistral/cron_trigger.py b/heat/engine/resources/openstack/mistral/cron_trigger.py index 28da634fbb..77f048a4db 100644 --- a/heat/engine/resources/openstack/mistral/cron_trigger.py +++ b/heat/engine/resources/openstack/mistral/cron_trigger.py @@ -111,11 +111,7 @@ class CronTrigger(resource.Resource): self.resource_id_set(cron_trigger.name) def _resolve_attribute(self, name): - try: - trigger = self.client().cron_triggers.get(self.resource_id) - except Exception as ex: - self.client_plugin().ignore_not_found(ex) - return '' + trigger = self.client().cron_triggers.get(self.resource_id) if name == self.NEXT_EXECUTION_TIME: return trigger.next_execution_time elif name == self.REMAINING_EXECUTIONS: diff --git a/heat/engine/resources/openstack/neutron/neutron.py b/heat/engine/resources/openstack/neutron/neutron.py index bfddd7ef67..7c3824386d 100644 --- a/heat/engine/resources/openstack/neutron/neutron.py +++ b/heat/engine/resources/openstack/neutron/neutron.py @@ -119,11 +119,7 @@ class NeutronResource(resource.Resource): result=_('Resource is not built')) def _resolve_attribute(self, name): - try: - attributes = self._show_resource() - except Exception as ex: - self.client_plugin().ignore_not_found(ex) - return None + attributes = self._show_resource() return attributes[name] def FnGetRefId(self): diff --git a/heat/tests/neutron/test_neutron.py b/heat/tests/neutron/test_neutron.py index b46dc8c99e..047e2577aa 100644 --- a/heat/tests/neutron/test_neutron.py +++ b/heat/tests/neutron/test_neutron.py @@ -127,19 +127,18 @@ class NeutronTest(common.HeatTestCase): self.assertEqual({'attr1': 'val1', 'attr2': 'val2'}, res.FnGetAtt('show')) - self.assertEqual('val2', res._resolve_attribute('attr2')) - self.assertRaises(KeyError, res._resolve_attribute, 'attr3') - self.assertIsNone(res._resolve_attribute('attr2')) + self.assertEqual('val2', res._resolve_all_attributes('attr2')) + self.assertRaises(KeyError, res._resolve_all_attributes, 'attr3') + self.assertIsNone(res._resolve_all_attributes('attr2')) res.resource_id = None # 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 + # reset cache, 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')) + res.attributes.reset_resolved_values() + self.assertIsNone(res.FnGetAtt('show')) class GetSecGroupUuidTest(common.HeatTestCase): diff --git a/heat/tests/nova/test_server.py b/heat/tests/nova/test_server.py index f1dace1410..c6930be3a3 100644 --- a/heat/tests/nova/test_server.py +++ b/heat/tests/nova/test_server.py @@ -2730,7 +2730,7 @@ class ServersTest(common.HeatTestCase): fakes_nova.fake_exception()) self.m.ReplayAll() - self.assertEqual('', server._resolve_attribute("accessIPv4")) + self.assertEqual('', server._resolve_all_attributes("accessIPv4")) self.m.VerifyAll() def test_resolve_attribute_console_url(self): @@ -2745,7 +2745,7 @@ class ServersTest(common.HeatTestCase): self.fc.servers.get(server.id).AndReturn(server) self.m.ReplayAll() - console_urls = ws._resolve_attribute('console_urls') + console_urls = ws._resolve_all_attributes('console_urls') self.assertIsInstance(console_urls, collections.Mapping) supported_consoles = ('novnc', 'xvpvnc', 'spice-html5', 'rdp-html5', 'serial') @@ -2769,7 +2769,7 @@ class ServersTest(common.HeatTestCase): expect_networks = {"fake_uuid": ["10.0.0.3"], "fake_net": ["10.0.0.3"]} self.assertEqual(expect_networks, - server._resolve_attribute("networks")) + server._resolve_all_attributes("networks")) self.m.VerifyAll() def test_empty_instance_user(self): diff --git a/heat/tests/test_resource.py b/heat/tests/test_resource.py index e624eb03d9..3108dccb3b 100644 --- a/heat/tests/test_resource.py +++ b/heat/tests/test_resource.py @@ -1788,14 +1788,13 @@ class ResourceTest(common.HeatTestCase): 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) + res.attributes.reset_resolved_values() + with mock.patch.object(res, 'client_plugin') as client_plugin: + # 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_plugin.call_count) def test_resolve_attributes_stuff_custom_attribute(self): # check path with resolve_attribute @@ -1803,9 +1802,16 @@ class ResourceTest(common.HeatTestCase): res = stack['res'] with mock.patch.object(res, '_resolve_attribute') as res_attr: - res.FnGetAtt('Foo') + res_attr.side_effect = ['Works', Exception] + self.assertEqual('Works', res.FnGetAtt('Foo')) res_attr.assert_called_once_with('Foo') + # clean resolved_values + res.attributes.reset_resolved_values() + with mock.patch.object(res, 'client_plugin') as client_plugin: + self.assertIsNone(res.FnGetAtt('Foo')) + self.assertEqual(1, client_plugin.call_count) + def test_show_resource(self): # check default function _show_resource stack = self.create_resource_for_attributes_tests()