From a752012c92bb15cf264c617bb2071958a32a8ad1 Mon Sep 17 00:00:00 2001
From: whoami-rajat <rajatdhasmana@gmail.com>
Date: Thu, 21 Sep 2023 17:41:16 +0000
Subject: [PATCH] Fix: Roll back volume status during reimage failure

When we try to reimage a volume, we update the status of
volume to 'downloading'.
We later validate the image metadata (like image is 'active',
image size is less than volume size, etc), and in case the
validation fails, we currently don't revert the volume status
back to original ('available', 'in-use' etc) and volume stays
in 'downloading' state.

This patch fixes this by catching the failure exception and
doing a DB update to restore the volume status back to it's
previous state.

Closes-Bug: #2036994
Change-Id: I05bf29e2a089b06398414b542b655a8083c9a21f
---
 cinder/tests/unit/volume/test_volume_reimage.py | 16 ++++++++++++++++
 cinder/volume/api.py                            | 17 ++++++++++++++++-
 ...eimage-status-rollback-eb2aa8f82a8caabc.yaml |  6 ++++++
 3 files changed, 38 insertions(+), 1 deletion(-)
 create mode 100644 releasenotes/notes/fix-reimage-status-rollback-eb2aa8f82a8caabc.yaml

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 <https://bugs.launchpad.net/cinder/+bug/2036994>`_: Fixed
+    rollback of volume status if the reimage operation fails while
+    checking image metadata.