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

@@ -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

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.")

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
 
 

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

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