From 8fdfac7177b2fd03d86c1bbe0513026492e46341 Mon Sep 17 00:00:00 2001 From: Vipin Balachandran Date: Tue, 16 Jun 2015 17:45:18 +0530 Subject: [PATCH] Get updated volume status in begin_detaching Heat stack deletion followed by an unsuccessful heat stack delete attempt resulted in calling begin_detaching on a volume which was already in detaching state. It was observed that the API returned success instead of failing the operation. There is a check which verifies the volume status, but it is not using the updated volume status. This patch fixes the problem by getting updated volume status from DB. But this fix won't eliminate the possibility of above mentioned inconsistent behavior if multiple begin_detaching happens concurrently. Closes-Bug: #1466060 Change-Id: I2a4cacef193e6cfa68026d97292370a895b291a9 --- cinder/tests/unit/test_volume.py | 12 +++++++++++- cinder/volume/api.py | 4 ++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/cinder/tests/unit/test_volume.py b/cinder/tests/unit/test_volume.py index b2fd36fc8..67bc182a2 100644 --- a/cinder/tests/unit/test_volume.py +++ b/cinder/tests/unit/test_volume.py @@ -3570,19 +3570,29 @@ class VolumeTestCase(BaseVolumeTestCase): 'name', 'description') - def test_begin_detaching_fails_available(self): + @mock.patch.object(db, 'volume_get') + def test_begin_detaching_fails_available(self, volume_get): volume_api = cinder.volume.api.API() volume = tests_utils.create_volume(self.context, **self.volume_params) + volume_get.return_value = volume + # Volume status is 'available'. self.assertRaises(exception.InvalidVolume, volume_api.begin_detaching, self.context, volume) + volume_get.assert_called_once_with(self.context, volume['id']) + + volume_get.reset_mock() volume['status'] = "in-use" volume['attach_status'] = "detached" # Should raise an error since not attached self.assertRaises(exception.InvalidVolume, volume_api.begin_detaching, self.context, volume) + volume_get.assert_called_once_with(self.context, volume['id']) + + volume_get.reset_mock() volume['attach_status'] = "attached" # Ensure when attached no exception raised volume_api.begin_detaching(self.context, volume) + volume_get.assert_called_once_with(self.context, volume['id']) def test_begin_roll_detaching_volume(self): """Test begin_detaching and roll_detaching functions.""" diff --git a/cinder/volume/api.py b/cinder/volume/api.py index 4afd9bbef..21431c613 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -563,6 +563,10 @@ class API(base.Base): @wrap_check_policy def begin_detaching(self, context, volume): + # NOTE(vbala): The volume status might be 'detaching' already due to + # a previous begin_detaching call. Get updated volume status so that + # we fail such cases. + volume = self.db.volume_get(context, volume['id']) # If we are in the middle of a volume migration, we don't want the user # to see that the volume is 'detaching'. Having 'migration_status' set # will have the same effect internally. -- 2.45.2