From 3e766e5bd429c6119c077b8be02360ae1e1ec8bd Mon Sep 17 00:00:00 2001 From: Wangpan Date: Tue, 28 Nov 2017 19:48:21 +0800 Subject: [PATCH] Handle glance exception during rotating instance backup Glance image backends may return HTTPConflict in many cases during deleting image, for example, an rbd image is used for booting a new instance, or rbd image has snapshot(s). If user have already backed up instance to few numbers and then execute backup api with rotation 1, then nova will delete the previously created images exceeding rotation limit. During deleting these images, if the first one of the backup images are deleted failed with HTTPConflict or other exceptions, all images exceeding rotation limit will be left over. This patch handles ImageDeleteConflict and all other exceptions during deleting backup images, logs a message and continues deleting all of the remaining images. Closes-Bug: #1734838 Change-Id: Ie8091fe3e0e4275717ddc50166345f1c9df4b889 --- nova/compute/manager.py | 6 +++ nova/exception.py | 4 ++ nova/image/glance.py | 3 ++ nova/tests/unit/compute/test_compute.py | 68 +++++++++++++++++++++++++ nova/tests/unit/image/test_glance.py | 10 ++++ 5 files changed, 91 insertions(+) diff --git a/nova/compute/manager.py b/nova/compute/manager.py index ed254293b5..970138c2f6 100644 --- a/nova/compute/manager.py +++ b/nova/compute/manager.py @@ -3401,6 +3401,12 @@ class ComputeManager(manager.Manager): LOG.info("Failed to find image %(image_id)s to " "delete", {'image_id': image_id}, instance=instance) + except (exception.ImageDeleteConflict, Exception) as exc: + LOG.info("Failed to delete image %(image_id)s during " + "deleting excess backups. " + "Continuing for next image.. %(exc)s", + {'image_id': image_id, 'exc': exc}, + instance=instance) @wrap_exception() @reverts_task_state diff --git a/nova/exception.py b/nova/exception.py index bf8a8e4852..4f81e6db23 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -652,6 +652,10 @@ class ImageNotFound(NotFound): msg_fmt = _("Image %(image_id)s could not be found.") +class ImageDeleteConflict(NovaException): + msg_fmt = _("Conflict deleting image. Reason: %(reason)s.") + + class PreserveEphemeralNotSupported(Invalid): msg_fmt = _("The current driver does not support " "preserving ephemeral partitions.") diff --git a/nova/image/glance.py b/nova/image/glance.py index d663e4c25c..d6e49cab51 100644 --- a/nova/image/glance.py +++ b/nova/image/glance.py @@ -547,6 +547,7 @@ class GlanceImageServiceV2(object): :raises: ImageNotFound if the image does not exist. :raises: NotAuthorized if the user is not an owner. :raises: ImageNotAuthorized if the user is not authorized. + :raises: ImageDeleteConflict if the image is conflicted to delete. """ try: @@ -555,6 +556,8 @@ class GlanceImageServiceV2(object): raise exception.ImageNotFound(image_id=image_id) except glanceclient.exc.HTTPForbidden: raise exception.ImageNotAuthorized(image_id=image_id) + except glanceclient.exc.HTTPConflict as exc: + raise exception.ImageDeleteConflict(reason=six.text_type(exc)) return True diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index 44b795c826..d12dbb7af4 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -3610,6 +3610,74 @@ class ComputeTestCase(BaseTestCase, rotation=1) self.assertEqual(2, mock_delete.call_count) + @mock.patch('nova.image.api.API.get_all') + def test_rotate_backups_with_image_delete_failed(self, + mock_get_all_images): + instance = self._create_fake_instance_obj() + instance_uuid = instance['uuid'] + fake_images = [{ + 'id': uuids.image_id_1, + 'created_at': timeutils.parse_strtime('2017-01-04T00:00:00.00'), + 'name': 'fake_name_1', + 'status': 'active', + 'properties': {'kernel_id': uuids.kernel_id_1, + 'ramdisk_id': uuids.ramdisk_id_1, + 'image_type': 'backup', + 'backup_type': 'daily', + 'instance_uuid': instance_uuid}, + }, + { + 'id': uuids.image_id_2, + 'created_at': timeutils.parse_strtime('2017-01-03T00:00:00.00'), + 'name': 'fake_name_2', + 'status': 'active', + 'properties': {'kernel_id': uuids.kernel_id_2, + 'ramdisk_id': uuids.ramdisk_id_2, + 'image_type': 'backup', + 'backup_type': 'daily', + 'instance_uuid': instance_uuid}, + }, + { + 'id': uuids.image_id_3, + 'created_at': timeutils.parse_strtime('2017-01-02T00:00:00.00'), + 'name': 'fake_name_3', + 'status': 'active', + 'properties': {'kernel_id': uuids.kernel_id_3, + 'ramdisk_id': uuids.ramdisk_id_3, + 'image_type': 'backup', + 'backup_type': 'daily', + 'instance_uuid': instance_uuid}, + }, + { + 'id': uuids.image_id_4, + 'created_at': timeutils.parse_strtime('2017-01-01T00:00:00.00'), + 'name': 'fake_name_4', + 'status': 'active', + 'properties': {'kernel_id': uuids.kernel_id_4, + 'ramdisk_id': uuids.ramdisk_id_4, + 'image_type': 'backup', + 'backup_type': 'daily', + 'instance_uuid': instance_uuid}, + }] + + mock_get_all_images.return_value = fake_images + + def _check_image_id(context, image_id): + self.assertIn(image_id, [uuids.image_id_2, uuids.image_id_3, + uuids.image_id_4]) + if image_id == uuids.image_id_3: + raise Exception('fake %s delete exception' % image_id) + if image_id == uuids.image_id_4: + raise exception.ImageDeleteConflict(reason='image is in use') + + with mock.patch.object(nova.image.api.API, 'delete', + side_effect=_check_image_id) as mock_delete: + # Fake images 4,3,2 should be rotated in sequence + self.compute._rotate_backups(self.context, instance=instance, + backup_type='daily', + rotation=1) + self.assertEqual(3, mock_delete.call_count) + def test_console_output(self): # Make sure we can get console output from instance. instance = self._create_fake_instance_obj() diff --git a/nova/tests/unit/image/test_glance.py b/nova/tests/unit/image/test_glance.py index 32a431a2cb..fd69e0d19e 100644 --- a/nova/tests/unit/image/test_glance.py +++ b/nova/tests/unit/image/test_glance.py @@ -1570,6 +1570,16 @@ class TestDelete(test.NoDBTestCase): self.assertRaises(exception.ImageNotFound, service.delete, ctx, mock.sentinel.image_id) + def test_delete_client_conflict_failure_v2(self): + client = mock.MagicMock() + fake_details = 'Image %s is in use' % mock.sentinel.image_id + client.call.side_effect = glanceclient.exc.HTTPConflict( + details=fake_details) + ctx = mock.sentinel.ctx + service = glance.GlanceImageServiceV2(client) + self.assertRaises(exception.ImageDeleteConflict, service.delete, ctx, + mock.sentinel.image_id) + class TestGlanceApiServers(test.NoDBTestCase):