From 31569827c749dca6e0affd4646360b839f321758 Mon Sep 17 00:00:00 2001 From: Vipin Balachandran Date: Wed, 30 Apr 2014 17:30:57 +0530 Subject: [PATCH] Delete image on upload-to-image failure On upload-to-image failure before initiating the data transfer or during data transfer, the source volume status is restored properly whereas the image created remains in queued or saving state. This change deletes the image during such failures. Change-Id: I0aa64798d2bc5bf19b79dd3b88dcd107ff369c42 Closes-Bug: #1298042 --- cinder/tests/test_volume.py | 70 ++++++++++++++++++++++++++++++++++++- cinder/volume/manager.py | 23 ++++++++++++ 2 files changed, 92 insertions(+), 1 deletion(-) diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py index e94d04cc2..96e13c625 100644 --- a/cinder/tests/test_volume.py +++ b/cinder/tests/test_volume.py @@ -2584,8 +2584,9 @@ class CopyVolumeToImageTestCase(BaseVolumeTestCase): os.close(self.dst_fd) self.stubs.Set(self.volume.driver, 'local_path', self.fake_local_path) + self.image_id = '70a599e0-31e7-49b7-b260-868f441e862b' self.image_meta = { - 'id': '70a599e0-31e7-49b7-b260-868f441e862b', + 'id': self.image_id, 'container_format': 'bare', 'disk_format': 'raw' } @@ -2662,6 +2663,73 @@ class CopyVolumeToImageTestCase(BaseVolumeTestCase): volume = db.volume_get(self.context, self.volume_id) self.assertEqual(volume.status, 'available') + def test_copy_volume_to_image_driver_exception(self): + self.image_meta['id'] = self.image_id + + image_service = fake_image.FakeImageService() + # create new image in queued state + queued_image_id = 'd5133f15-f753-41bd-920a-06b8c49275d9' + queued_image_meta = image_service.show(self.context, self.image_id) + queued_image_meta['id'] = queued_image_id + queued_image_meta['status'] = 'queued' + image_service.create(self.context, queued_image_meta) + + # create new image in saving state + saving_image_id = '5c6eec33-bab4-4e7d-b2c9-88e2d0a5f6f2' + saving_image_meta = image_service.show(self.context, self.image_id) + saving_image_meta['id'] = saving_image_id + saving_image_meta['status'] = 'saving' + image_service.create(self.context, saving_image_meta) + + # create volume + self.volume_attrs['status'] = 'available' + self.volume_attrs['instance_uuid'] = None + db.volume_create(self.context, self.volume_attrs) + + with mock.patch.object(self.volume.driver, + 'copy_volume_to_image') as driver_copy_mock: + driver_copy_mock.side_effect = exception.VolumeDriverException( + "Error") + + # test with image not in queued state + self.assertRaises(exception.VolumeDriverException, + self.volume.copy_volume_to_image, + self.context, + self.volume_id, + self.image_meta) + volume = db.volume_get(self.context, self.volume_id) + self.assertEqual(volume['status'], 'available') + # image shouldn't be deleted if it is not in queued state + image_service.show(self.context, self.image_id) + + # test with image in queued state + self.assertRaises(exception.VolumeDriverException, + self.volume.copy_volume_to_image, + self.context, + self.volume_id, + queued_image_meta) + volume = db.volume_get(self.context, self.volume_id) + self.assertEqual(volume['status'], 'available') + # queued image should be deleted + self.assertRaises(exception.ImageNotFound, + image_service.show, + self.context, + queued_image_id) + + # test with image in saving state + self.assertRaises(exception.VolumeDriverException, + self.volume.copy_volume_to_image, + self.context, + self.volume_id, + saving_image_meta) + volume = db.volume_get(self.context, self.volume_id) + self.assertEqual(volume['status'], 'available') + # image in saving state should be deleted + self.assertRaises(exception.ImageNotFound, + image_service.show, + self.context, + saving_image_id) + class GetActiveByWindowTestCase(BaseVolumeTestCase): def setUp(self): diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index 29ad0b2ba..5176f6740 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -676,6 +676,7 @@ class VolumeManager(manager.SchedulerDependentManager): """ payload = {'volume_id': volume_id, 'image_id': image_meta['id']} + image_service = None try: volume = self.db.volume_get(context, volume_id) @@ -692,6 +693,13 @@ class VolumeManager(manager.SchedulerDependentManager): "image (%(image_id)s) successfully"), {'volume_id': volume_id, 'image_id': image_id}) except Exception as error: + LOG.error(_("Error occurred while uploading volume %(volume_id)s " + "to image %(image_id)s."), + {'volume_id': volume_id, 'image_id': image_meta['id']}) + if image_service is not None: + # Deletes the image if it is in queued or saving state + self._delete_image(context, image_meta['id'], image_service) + with excutils.save_and_reraise_exception(): payload['message'] = unicode(error) finally: @@ -703,6 +711,21 @@ class VolumeManager(manager.SchedulerDependentManager): self.db.volume_update(context, volume_id, {'status': 'in-use'}) + def _delete_image(self, context, image_id, image_service): + """Deletes an image stuck in queued or saving state.""" + try: + image_meta = image_service.show(context, image_id) + image_status = image_meta.get('status') + if image_status == 'queued' or image_status == 'saving': + LOG.warn("Deleting image %(image_id)s in %(image_status)s " + "state.", + {'image_id': image_id, + 'image_status': image_status}) + image_service.delete(context, image_id) + except Exception: + LOG.warn(_("Error occurred while deleting image %s."), + image_id, exc_info=True) + def initialize_connection(self, context, volume_id, connector): """Prepare volume for connection from host represented by connector. -- 2.45.2