From 8d7e3e41e8f02726dca33b5ec2f6d5b6b6b07a31 Mon Sep 17 00:00:00 2001 From: Crag Wolfe Date: Thu, 23 Mar 2017 14:59:11 -0400 Subject: [PATCH] Don't set metadata for deleted resources Change-Id: I809dd844797dc0d9a8f31226a5b2da65a20a345d Partial-Bug: #1675286 --- heat/engine/resource.py | 11 ++++++++++- heat/engine/service_software_config.py | 3 ++- heat/tests/test_resource.py | 17 +++++++++++++++++ heat/tests/test_signal.py | 22 +++++++++++++--------- 4 files changed, 42 insertions(+), 11 deletions(-) diff --git a/heat/engine/resource.py b/heat/engine/resource.py index b20c50e83a..43b9815ef6 100644 --- a/heat/engine/resource.py +++ b/heat/engine/resource.py @@ -447,10 +447,16 @@ class Resource(object): """ if self.id is None or self.action == self.INIT: raise exception.ResourceNotAvailable(resource_name=self.name) - LOG.debug('Setting metadata for %s', six.text_type(self)) refresh = merge_metadata is not None db_res = resource_objects.Resource.get_obj(self.stack.context, self.id, refresh=refresh) + if db_res.action == self.DELETE: + self._db_res_is_deleted = True + LOG.debug("resource %(name)s, id: %(id)s is DELETE_%(st)s, " + "not setting metadata", + {'name': self.name, 'id': self.id, 'st': db_res.status}) + raise exception.ResourceNotAvailable(resource_name=self.name) + LOG.debug('Setting metadata for %s', six.text_type(self)) if refresh: metadata = merge_metadata(metadata, db_res.rsrc_metadata) db_res.update_metadata(metadata) @@ -2179,6 +2185,9 @@ class Resource(object): # Don't log an event as it just spams the user. pass except Exception as ex: + if hasattr(self, '_db_res_is_deleted'): + # No spam required + return LOG.info('signal %(name)s : %(msg)s', {'name': six.text_type(self), 'msg': six.text_type(ex)}, diff --git a/heat/engine/service_software_config.py b/heat/engine/service_software_config.py index 5e3be812fe..426232b3bf 100644 --- a/heat/engine/service_software_config.py +++ b/heat/engine/service_software_config.py @@ -25,6 +25,7 @@ from heat.common import exception from heat.common.i18n import _ from heat.db.sqlalchemy import api as db_api from heat.engine import api +from heat.engine import resource from heat.engine import scheduler from heat.engine import software_config_io as swc_io from heat.objects import resource as resource_objects @@ -99,7 +100,7 @@ class SoftwareConfigService(object): def _push_metadata_software_deployments( self, cnxt, server_id, stack_user_project_id): rs = db_api.resource_get_by_physical_resource_id(cnxt, server_id) - if not rs: + if not rs or rs.action == resource.Resource.DELETE: return deployments = self.metadata_software_deployments(cnxt, server_id) md = rs.rsrc_metadata or {} diff --git a/heat/tests/test_resource.py b/heat/tests/test_resource.py index 5331a75cc0..714bb0d58e 100644 --- a/heat/tests/test_resource.py +++ b/heat/tests/test_resource.py @@ -635,6 +635,23 @@ class ResourceTest(common.HeatTestCase): res = generic_rsrc.GenericResource('test_resource', tmpl, self.stack) self.assertEqual({}, res.metadata_get()) + def test_metadata_set_when_deleted(self): + tmpl = rsrc_defn.ResourceDefinition('test_resource', 'Foo') + res = generic_rsrc.GenericResource('test_resource', tmpl, self.stack) + res.store() + md = {"don't": "care"} + res.action = 'CREATE' + res.metadata_set(md) + self.assertFalse(hasattr(res, '_db_res_is_deleted')) + + res_obj = self.stack.context.session.query( + models.Resource).get(res.id) + res_obj.update({'action': 'DELETE'}) + + self.assertRaises(exception.ResourceNotAvailable, + res.metadata_set, md) + self.assertTrue(res._db_res_is_deleted) + def test_equals_different_stacks(self): tmpl1 = rsrc_defn.ResourceDefinition('test_resource', 'Foo') tmpl2 = rsrc_defn.ResourceDefinition('test_resource', 'Foo') diff --git a/heat/tests/test_signal.py b/heat/tests/test_signal.py index 116b9098f2..52c7f7fdc3 100644 --- a/heat/tests/test_signal.py +++ b/heat/tests/test_signal.py @@ -20,9 +20,9 @@ from six.moves.urllib import parse as urlparse from heat.common import exception from heat.common import template_format +from heat.db.sqlalchemy import models from heat.engine.clients.os import heat_plugin from heat.engine.clients.os import swift -from heat.engine import resource from heat.engine import scheduler from heat.engine import stack as stk from heat.engine import template @@ -528,26 +528,30 @@ class SignalTest(common.HeatTestCase): mock_handle.assert_called_once_with(test_d) self.assertTrue(result) - @mock.patch.object(generic_resource.SignalResource, '_add_event') @mock.patch.object(generic_resource.SignalResource, 'handle_signal') - def test_signal_no_action(self, mock_handle, mock_add): + def test_handle_signal_no_reraise_deleted(self, mock_handle): # Setup test_d = {'Data': 'foo', 'Reason': 'bar', 'Status': 'SUCCESS', 'UniqueId': '123'} stack = self._create_stack(TEMPLATE_CFN_SIGNAL) - mock_handle.side_effect = resource.NoActionRequired() + mock_handle.side_effect = exception.ResourceNotAvailable( + resource_name='test') rsrc = stack['signal_handler'] self.assertEqual((rsrc.CREATE, rsrc.COMPLETE), rsrc.state) - self.assertTrue(rsrc.requires_deferred_auth) - # Test - mock_add.reset_mock() # clean up existing calls before test - rsrc.signal(details=test_d) + # In the midst of handling a signal, an update happens on the + # db resource concurrently, deleting it + + # Test exception not re-raised in DELETE case + res_obj = stack.context.session.query( + models.Resource).get(rsrc.id) + res_obj.update({'action': 'DELETE'}) + rsrc._db_res_is_deleted = True + rsrc._handle_signal(details=test_d) mock_handle.assert_called_once_with(test_d) - mock_add.assert_not_called() @mock.patch.object(generic_resource.SignalResource, '_add_event') def test_signal_different_reason_types(self, mock_add):