Propagate deletion errors with exceptions

Change-Id: Ib8380f6d985e328c56d53b326700d6c9df636f40

diff --git a/heat/common/exception.py b/heat/common/exception.py
index 1e2f85e51ae0ba8dda7a302ffe342cfc021d87dd..5fd56a192b13f45a6ebaf5cd0fc5738146e4eb63 100644 (file)
--- a/heat/common/exception.py
+++ b/heat/common/exception.py
@@ -256,7 +256,3 @@ class ResourceFailure(OpenstackException):
         exc_type = type(exception).__name__
         super(ResourceFailure, self).__init__(exc_type=exc_type,
                                               message=str(exception))
-
-
-class NestedResourceFailure(OpenstackException):
-    message = _("%(message)s")

diff --git a/heat/engine/parser.py b/heat/engine/parser.py
index 257f3d11d8f7311481311bcb255ebfd5d3eaebdb..992a74ee5ac70402b4f783dd5c03bc283af9929a 100644 (file)
--- a/heat/engine/parser.py
+++ b/heat/engine/parser.py
@@ -346,10 +346,11 @@ class Stack(object):
                     if not res.name in newstack.keys():
                         logger.debug("resource %s not found in updated stack"
                                      % res.name + " definition, deleting")
-                        result = res.destroy()
-                        if result:
+                        try:
+                            res.destroy()
+                        except exception.ResourceFailure as ex:
                             logger.error("Failed to remove %s : %s" %
-                                         (res.name, result))
+                                         (res.name, str(ex)))
                             raise exception.ResourceUpdateFailed(
                                 resource_name=res.name)
                         else:
@@ -408,10 +409,11 @@ class Stack(object):
                                         (res.name, self.name) +
                                         " update requires replacement")
                             # Resource requires replacement for update
-                            result = self[res.name].destroy()
-                            if result:
+                            try:
+                                self[res.name].destroy()
+                            except exception.ResourceFailure as ex:
                                 logger.error("Failed to delete %s : %s" %
-                                             (res.name, result))
+                                             (res.name, str(ex)))
                                 raise exception.ResourceUpdateFailed(
                                     resource_name=res.name)
                             else:
@@ -497,10 +499,11 @@ class Stack(object):
 
         failures = []
         for res in reversed(self):
-            result = res.destroy()
-            if result:
+            try:
+                res.destroy()
+            except exception.ResourceFailure as ex:
                 logger.error('Failed to delete %s error: %s' % (str(res),
-                                                                result))
+                                                                str(ex)))
                 failures.append(str(res))
 
         if failures:
@@ -535,7 +538,7 @@ class Stack(object):
         for res in reversed(deps):
             try:
                 res.destroy()
-            except Exception as ex:
+            except exception.ResourceFailure as ex:
                 failed = True
                 logger.error('delete: %s' % str(ex))
 

diff --git a/heat/engine/resource.py b/heat/engine/resource.py
index bef28a85f0e4d7c0139939a5032d9d3fd4676c2d..f5ff8531058f834783f851cec7c9de6d6b7dfb15 100644 (file)
--- a/heat/engine/resource.py
+++ b/heat/engine/resource.py
@@ -410,32 +410,37 @@ class Resource(object):
         if self.state == self.DELETE_COMPLETE:
             return
         if self.state == self.DELETE_IN_PROGRESS:
-            return 'Resource deletion already in progress'
+            raise exception.Error('Resource deletion already in progress')
         # No need to delete if the resource has never been created
         if self.state is None:
             return
 
-        logger.info('deleting %s (inst:%s db_id:%s)' %
-                    (str(self), self.resource_id, str(self.id)))
-        self.state_set(self.DELETE_IN_PROGRESS)
+        logger.info('deleting %s' % str(self))
 
         try:
+            self.state_set(self.DELETE_IN_PROGRESS)
+
             if callable(getattr(self, 'handle_delete', None)):
                 self.handle_delete()
         except Exception as ex:
             logger.exception('Delete %s', str(self))
-            self.state_set(self.DELETE_FAILED, str(ex))
-            return str(ex) or "Error : %s" % type(ex)
-
-        self.state_set(self.DELETE_COMPLETE)
+            failure = exception.ResourceFailure(ex)
+            self.state_set(self.DELETE_FAILED, str(failure))
+            raise failure
+        except:
+            with excutils.save_and_reraise_exception():
+                try:
+                    self.state_set(self.DELETE_FAILED, 'Deletion aborted')
+                except Exception:
+                    logger.exception('Error marking resource deletion failed')
+        else:
+            self.state_set(self.DELETE_COMPLETE)
 
     def destroy(self):
         '''
         Delete the resource and remove it from the database.
         '''
-        result = self.delete()
-        if result:
-            return result
+        self.delete()
 
         if self.id is None:
             return
@@ -446,9 +451,6 @@ class Resource(object):
             # Don't fail on delete if the db entry has
             # not been created yet.
             pass
-        except Exception as ex:
-            logger.exception('Delete %s from DB' % str(self))
-            return str(ex) or "Error : %s" % type(ex)
 
         self.id = None
 

diff --git a/heat/engine/resources/autoscaling.py b/heat/engine/resources/autoscaling.py
index ed1ca5c5dec5d1fa756de01d6044cae01a1119ec..a2233fd3e0068cd85523c4adf53fae9b96b93fad 100644 (file)
--- a/heat/engine/resources/autoscaling.py
+++ b/heat/engine/resources/autoscaling.py
@@ -167,11 +167,7 @@ class InstanceGroup(resource.Resource):
             for victim in inst_list:
                 logger.debug('handle_delete %s' % victim)
                 inst = self._make_instance(victim)
-                error_str = inst.destroy()
-                if error_str is not None:
-                    # try suck out the grouped resouces failure reason
-                    # and re-raise
-                    raise exception.NestedResourceFailure(message=error_str)
+                inst.destroy()
 
     def resize(self, new_capacity, raise_on_error=False):
         inst_list = []

diff --git a/heat/tests/test_parser.py b/heat/tests/test_parser.py
index 8a349edd1bafbc7067550e23e809e1dc0552b172..3e154c22222c0e7d9ecb7e48747881ae2029e36c 100644 (file)
--- a/heat/tests/test_parser.py
+++ b/heat/tests/test_parser.py
@@ -470,9 +470,12 @@ class StackTest(unittest.TestCase):
         self.assertEqual(resource, self.stack.resource_by_refid('aaaa'))
 
         resource.state = resource.DELETE_IN_PROGRESS
-        self.assertEqual(None, self.stack.resource_by_refid('aaaa'))
+        try:
+            self.assertEqual(None, self.stack.resource_by_refid('aaaa'))
 
-        self.assertEqual(None, self.stack.resource_by_refid('bbbb'))
+            self.assertEqual(None, self.stack.resource_by_refid('bbbb'))
+        finally:
+            resource.state = resource.CREATE_COMPLETE
 
     @stack_delete_after
     def test_update_add(self):
@@ -628,7 +631,8 @@ class StackTest(unittest.TestCase):
 
         # make the update fail deleting the existing resource
         self.m.StubOutWithMock(resource.Resource, 'destroy')
-        resource.Resource.destroy().AndReturn("Error")
+        exc = exception.ResourceFailure(Exception())
+        resource.Resource.destroy().AndRaise(exc)
         self.m.ReplayAll()
 
         self.stack.update(updated_stack)
@@ -839,16 +843,17 @@ class StackTest(unittest.TestCase):
         updated_stack = parser.Stack(self.ctx, 'updated_stack',
                                      template.Template(tmpl2))
 
-        # patch in a dummy destroy making the delete fail
-        self.m.StubOutWithMock(resource.Resource, 'destroy')
-        resource.Resource.destroy().AndReturn('Error')
+        # patch in a dummy delete making the destroy fail
+        self.m.StubOutWithMock(resource.Resource, 'delete')
+        exc = exception.ResourceFailure(Exception())
+        resource.Resource.delete().AndRaise(exc)
         self.m.ReplayAll()
 
         self.stack.update(updated_stack)
         self.assertEqual(self.stack.state, parser.Stack.ROLLBACK_COMPLETE)
         self.assertTrue('BResource' in self.stack)
         self.m.VerifyAll()
-        # Unset here so destroy() is not stubbed for stack.delete cleanup
+        # Unset here so delete() is not stubbed for stack.delete cleanup
         self.m.UnsetStubs()
 
     @stack_delete_after

diff --git a/heat/tests/test_volume.py b/heat/tests/test_volume.py
index 079d1e93ae9b1862f3374b9ac630d6c1c2df1ac3..1a69a515aeeab770300e7b74e50f63ef012f8c17 100644 (file)
--- a/heat/tests/test_volume.py
+++ b/heat/tests/test_volume.py
@@ -117,14 +117,13 @@ class VolumeTest(unittest.TestCase):
         self.assertEqual(resource.handle_update({}), vol.Volume.UPDATE_REPLACE)
 
         fv.status = 'in-use'
-        resource.state = 'CREATE_COMPLETE'
-        self.assertEqual(resource.delete(), 'Volume in use')
+        self.assertRaises(exception.ResourceFailure, resource.destroy)
         fv.status = 'available'
-        resource.state = 'CREATE_COMPLETE'
-        self.assertEqual(resource.delete(), None)
-        fv.status = 'available'
-        resource.state = 'CREATE_COMPLETE'
-        self.assertEqual(resource.delete(), None)
+        self.assertEqual(resource.destroy(), None)
+
+        # Test when volume already deleted
+        resource.state = resource.CREATE_COMPLETE
+        self.assertEqual(resource.destroy(), None)
 
         self.m.VerifyAll()