Don't set metadata for deleted resources

Change-Id: I809dd844797dc0d9a8f31226a5b2da65a20a345d
Partial-Bug: #1675286

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)},

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 {}

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')

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):