Don't trace on ImageNotFound in delete_image_on_error

The point of the delete_image_on_error decorator is to
cleanup an image used during snapshot operations, so it
makes little sense to log an exception trace if the image
delete fails because the image no longer exists, which it
might not since _snapshot_instance method will proactively
delete non-active images in certain situations.

So let's just handle the ImageNotFound and ignore it.

Change-Id: I14e061a28678ad28e38bd185e3d0a35cae41a9cf
Closes-Bug: #1648574

diff --git a/nova/compute/manager.py b/nova/compute/manager.py
index 9f81ad472c06a6c9db598cc8d939ffe68db290f0..40d8fb23ba129f793455ae9693177be940e84ee9 100644 (file)
--- a/nova/compute/manager.py
+++ b/nova/compute/manager.py
@@ -236,6 +236,10 @@ def delete_image_on_error(function):
                           exc_info=True, instance=instance)
                 try:
                     self.image_api.delete(context, image_id)
+                except exception.ImageNotFound:
+                    # Since we're trying to cleanup an image, we don't care if
+                    # if it's already gone.
+                    pass
                 except Exception:
                     LOG.exception(_LE("Error while trying to clean up "
                                       "image %s"), image_id,

diff --git a/nova/tests/unit/compute/test_compute_mgr.py b/nova/tests/unit/compute/test_compute_mgr.py
index 46f73e59563167c77ece74bef868ac7aad0928ef..a675b594e7849bf47af01c8124f474480becadeb 100755 (executable)
--- a/nova/tests/unit/compute/test_compute_mgr.py
+++ b/nova/tests/unit/compute/test_compute_mgr.py
@@ -3321,6 +3321,31 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase):
                 mock.call(self.context, inst_obj, 'fake-mini',
                           action='restore', phase='end')])
 
+    def test_delete_image_on_error_image_not_found_ignored(self):
+        """Tests that we don't log an exception trace if we get a 404 when
+        trying to delete an image as part of the image cleanup decorator.
+        """
+        @manager.delete_image_on_error
+        def some_image_related_op(self, context, image_id, instance):
+            raise test.TestingException('oops!')
+
+        image_id = uuids.image_id
+        instance = objects.Instance(uuid=uuids.instance_uuid)
+
+        with mock.patch.object(manager.LOG, 'exception') as mock_log:
+            with mock.patch.object(
+                    self, 'image_api', create=True) as mock_image_api:
+                mock_image_api.delete.side_effect = (
+                    exception.ImageNotFound(image_id=image_id))
+                self.assertRaises(test.TestingException,
+                                  some_image_related_op,
+                                  self, self.context, image_id, instance)
+
+        mock_image_api.delete.assert_called_once_with(
+            self.context, image_id)
+        # make sure nothing was logged at exception level
+        mock_log.assert_not_called()
+
 
 class ComputeManagerBuildInstanceTestCase(test.NoDBTestCase):
     def setUp(self):