From 89bc79a49911fefd5d0dfc6bcb3085dbcfd06750 Mon Sep 17 00:00:00 2001 From: Matt Riedemann Date: Thu, 8 Dec 2016 13:44:54 -0500 Subject: [PATCH] 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 (cherry picked from commit 2bb70e7b15e6cfef4652e2e49c4e02d151d2dbdf) --- nova/compute/manager.py | 4 ++++ nova/tests/unit/compute/test_compute_mgr.py | 25 +++++++++++++++++++++ 2 files changed, 29 insertions(+) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index 4cc03e5de830..fb58012300fc 100644 --- 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 37c44040e258..235c46ddae28 100755 --- a/nova/tests/unit/compute/test_compute_mgr.py +++ b/nova/tests/unit/compute/test_compute_mgr.py @@ -3236,6 +3236,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):