Check volume status in detach db api

If a volume is detached while uploading volume to image is in progress then
it sets volume status to available and then it is allowed to delete it
immediately while uploading is still running in background.

Set the status to available in the detach db api only when the volume
is not migrating and status is not uploading.

Closes-Bug: 1413887
Change-Id: I6df9c6918825fba7c8d7cd6521752b399dd61610

diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py
index 432f59c02f3c97548449d61988758e9110fd8101..9ae7304dfa323efee27d76cb59146452936b9e27 100644 (file)
--- a/cinder/db/sqlalchemy/api.py
+++ b/cinder/db/sqlalchemy/api.py
@@ -1212,7 +1212,9 @@ def volume_detached(context, volume_id, attachment_id):
         volume_ref = _volume_get(context, volume_id, session=session)
         if not remain_attachment:
             # Hide status update from user if we're performing volume migration
-            if not volume_ref['migration_status']:
+            # or uploading it to image
+            if (not volume_ref['migration_status'] and
+                    not (volume_ref['status'] == 'uploading')):
                 volume_ref['status'] = 'available'
 
             volume_ref['attach_status'] = 'detached'

diff --git a/cinder/tests/test_volume.py b/cinder/tests/test_volume.py
index 7a0e5e90e14c86ca40c9559ceb46e59e8b2b1a66..511bfc5d25c3d6974a700462e7709e3e5d0f7a97 100644 (file)
--- a/cinder/tests/test_volume.py
+++ b/cinder/tests/test_volume.py
@@ -2353,6 +2353,31 @@ class VolumeTestCase(BaseVolumeTestCase):
         self.assertEqual('readonly', admin_metadata[0]['key'])
         self.assertEqual('True', admin_metadata[0]['value'])
 
+    def test_detach_volume_while_uploading_to_image_is_in_progress(self):
+        # If instance is booted from volume with 'Terminate on Delete' flag
+        # set, and when we delete instance then it tries to delete volume
+        # even it is in 'uploading' state.
+        # It is happening because detach call is setting volume status to
+        # 'available'.
+        mountpoint = "/dev/sdf"
+        # Attach volume to the instance
+        instance_uuid = '12345678-1234-5678-1234-567812345678'
+        volume = tests_utils.create_volume(self.context,
+                                           admin_metadata={'readonly': 'True'},
+                                           **self.volume_params)
+        volume_id = volume['id']
+        self.volume.create_volume(self.context, volume_id)
+        self.volume.attach_volume(self.context, volume_id, instance_uuid,
+                                  None, mountpoint, 'ro')
+        # Change volume status to 'uploading'
+        db.volume_update(self.context, volume_id, {'status': 'uploading'})
+        # Call detach api
+        self.volume.detach_volume(self.context, volume_id)
+        vol = db.volume_get(self.context, volume_id)
+        # Check that volume status is 'uploading'
+        self.assertEqual("uploading", vol['status'])
+        self.assertEqual("detached", vol['attach_status'])
+
     @mock.patch.object(cinder.volume.api.API, 'update')
     @mock.patch.object(db, 'volume_get')
     def test_reserve_volume_success(self, volume_get, volume_update):