diff --git a/cinder/tests/unit/volume/test_volume_reimage.py b/cinder/tests/unit/volume/test_volume_reimage.py index e03c59bd612..d490f4935c4 100644 --- a/cinder/tests/unit/volume/test_volume_reimage.py +++ b/cinder/tests/unit/volume/test_volume_reimage.py @@ -108,6 +108,22 @@ class VolumeReimageTestCase(base.BaseVolumeTestCase): mock_reimage.assert_called_once_with(self.context, volume, self.image_meta) + @mock.patch('cinder.volume.volume_utils.check_image_metadata') + @mock.patch('cinder.volume.rpcapi.VolumeAPI.reimage') + @ddt.data('available', 'error') + def test_volume_reimage_check_meta_exception(self, status, mock_reimage, + mock_check): + volume = tests_utils.create_volume(self.context) + volume.status = status + volume.save() + self.assertEqual(volume.status, status) + mock_check.side_effect = exception.ImageUnacceptable( + image_id=self.image_meta['id'], reason='') + # The available or error volume can be reimaged directly + self.assertRaises(exception.ImageUnacceptable, self.volume_api.reimage, + self.context, volume, self.image_meta['id']) + self.assertEqual(status, volume.status) + @mock.patch('cinder.volume.volume_utils.check_image_metadata') @mock.patch('cinder.volume.rpcapi.VolumeAPI.reimage') def test_volume_reimage_api_with_reimage_reserved(self, mock_reimage, diff --git a/cinder/volume/api.py b/cinder/volume/api.py index ccf58523c4c..d9dba25e18b 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -2675,7 +2675,22 @@ class API(base.Base): 'status': volume.status}) raise exception.InvalidVolume(reason=msg) image_meta = self.image_service.show(context, image_id) - volume_utils.check_image_metadata(image_meta, volume['size']) + try: + volume_utils.check_image_metadata(image_meta, volume['size']) + # Currently we only raise InvalidInput and ImageUnacceptable + # exceptions in the check_image_metadata call but having Exception + # here makes it more generic since we want to roll back to original + # state in any case and we re-raise anyway. + # Also this helps makes adding new exceptions easier in the future. + except Exception: + with excutils.save_and_reraise_exception(): + LOG.exception("Failed to reimage volume %(volume_id)s with " + "image %(image_id)s", + {'volume_id': volume.id, 'image_id': image_id}) + volume.conditional_update( + {'status': volume.model.previous_status, + 'previous_status': None}, + {'status': 'downloading'}) self.volume_rpcapi.reimage(context, volume, image_meta) diff --git a/releasenotes/notes/fix-reimage-status-rollback-eb2aa8f82a8caabc.yaml b/releasenotes/notes/fix-reimage-status-rollback-eb2aa8f82a8caabc.yaml new file mode 100644 index 00000000000..6e4b1dbfcaa --- /dev/null +++ b/releasenotes/notes/fix-reimage-status-rollback-eb2aa8f82a8caabc.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + `Bug #2036994 `_: Fixed + rollback of volume status if the reimage operation fails while + checking image metadata.