diff --git a/cinder/tests/unit/backup/test_backup.py b/cinder/tests/unit/backup/test_backup.py index 2cc786ac41c..7213591397f 100644 --- a/cinder/tests/unit/backup/test_backup.py +++ b/cinder/tests/unit/backup/test_backup.py @@ -1019,9 +1019,9 @@ class BackupTestCase(BaseBackupTest): 'fake_provider_id'} temp_vol = volume_manager.driver._create_temp_cloned_volume( - self.ctxt, vol) + self.ctxt, vol, status=fields.VolumeStatus.BACKING_UP) - self.assertEqual('available', temp_vol['status']) + self.assertEqual(fields.VolumeStatus.BACKING_UP, temp_vol['status']) self.assertEqual('fake_provider_id', temp_vol['provider_id']) @mock.patch.object(fake_driver.FakeLoggingVolumeDriver, @@ -1038,9 +1038,9 @@ class BackupTestCase(BaseBackupTest): 'fake_provider_id'} temp_vol = volume_manager.driver._create_temp_volume_from_snapshot( - self.ctxt, vol, snap) + self.ctxt, vol, snap, status=fields.VolumeStatus.BACKING_UP) - self.assertEqual('available', temp_vol['status']) + self.assertEqual(fields.VolumeStatus.BACKING_UP, temp_vol['status']) self.assertEqual('fake_provider_id', temp_vol['provider_id']) @mock.patch('cinder.volume.volume_utils.notify_about_backup_usage') diff --git a/cinder/tests/unit/volume/test_driver.py b/cinder/tests/unit/volume/test_driver.py index c0ee59b47f4..8afb5428193 100644 --- a/cinder/tests/unit/volume/test_driver.py +++ b/cinder/tests/unit/volume/test_driver.py @@ -209,8 +209,8 @@ class GenericVolumeDriverTestCase(BaseDriverTestCase): vol = tests_utils.create_volume(self.context, status='backing-up') cloned_vol = self.volume.driver._create_temp_cloned_volume( - self.context, vol) - self.assertEqual('available', cloned_vol.status) + self.context, vol, status=fields.VolumeStatus.BACKING_UP) + self.assertEqual(fields.VolumeStatus.BACKING_UP, cloned_vol.status) def test_get_backup_device_available(self): vol = tests_utils.create_volume(self.context) @@ -232,8 +232,9 @@ class GenericVolumeDriverTestCase(BaseDriverTestCase): status='backing-up', previous_status='in-use') admin_meta = {'temporary': 'True'} - temp_vol = tests_utils.create_volume(self.context, - admin_metadata=admin_meta) + temp_vol = tests_utils.create_volume( + self.context, status=fields.VolumeStatus.BACKING_UP, + admin_metadata=admin_meta) self.context.user_id = fake.USER_ID self.context.project_id = fake.PROJECT_ID backup_obj = tests_utils.create_backup(self.context, @@ -249,6 +250,8 @@ class GenericVolumeDriverTestCase(BaseDriverTestCase): self.assertFalse(is_snapshot) backup_obj.refresh() self.assertEqual(temp_vol.id, backup_obj.temp_volume_id) + mock_create_temp.assert_called_once_with( + self.context, vol, status=fields.VolumeStatus.BACKING_UP) def test_create_temp_volume_from_snapshot(self): volume_dict = {'id': fake.SNAPSHOT_ID, diff --git a/cinder/tests/unit/volume/test_volume.py b/cinder/tests/unit/volume/test_volume.py index 8d774b76d84..976079216a6 100644 --- a/cinder/tests/unit/volume/test_volume.py +++ b/cinder/tests/unit/volume/test_volume.py @@ -2247,8 +2247,8 @@ class VolumeTestCase(base.BaseVolumeTestCase): 'delete_volume') as mock_driver_delete, \ mock.patch.object( self.volume, '_copy_volume_data') as mock_copy: - temp_volume = tests_utils.create_volume(self.context, - status='available') + temp_volume = tests_utils.create_volume( + self.context, status=fields.VolumeStatus.IN_USE) mock_copy.side_effect = [exception.VolumeDriverException('error')] mock_temp.return_value = temp_volume diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index 69d6b22d01c..15a6f1429a3 100644 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -1219,7 +1219,8 @@ class BaseVD(object, metaclass=abc.ABCMeta): # then clean up the temp volume. if snapshot: temp_vol_ref = self._create_temp_volume_from_snapshot( - context, volume, snapshot) + context, volume, snapshot, + status=fields.VolumeStatus.BACKING_UP) backup.temp_volume_id = temp_vol_ref.id backup.save() device_to_backup = temp_vol_ref @@ -1232,7 +1233,7 @@ class BaseVD(object, metaclass=abc.ABCMeta): previous_status = volume.get('previous_status') if previous_status == "in-use": temp_vol_ref = self._create_temp_cloned_volume( - context, volume) + context, volume, status=fields.VolumeStatus.BACKING_UP) backup.temp_volume_id = temp_vol_ref.id backup.save() device_to_backup = temp_vol_ref @@ -1334,7 +1335,8 @@ class BaseVD(object, metaclass=abc.ABCMeta): temp_vol_ref.create() return temp_vol_ref - def _create_temp_cloned_volume(self, context, volume): + def _create_temp_cloned_volume(self, context, volume, + status=fields.VolumeStatus.AVAILABLE): temp_vol_ref = self._create_temp_volume(context, volume) try: model_update = self.create_cloned_volume(temp_vol_ref, volume) @@ -1344,12 +1346,13 @@ class BaseVD(object, metaclass=abc.ABCMeta): with excutils.save_and_reraise_exception(): temp_vol_ref.destroy() - temp_vol_ref.status = 'available' + temp_vol_ref.status = status temp_vol_ref.save() return temp_vol_ref - def _create_temp_volume_from_snapshot(self, context, volume, snapshot, - volume_options=None): + def _create_temp_volume_from_snapshot( + self, context, volume, snapshot, volume_options=None, + status=fields.VolumeStatus.AVAILABLE): temp_vol_ref = self._create_temp_volume(context, volume, volume_options=volume_options) try: @@ -1361,7 +1364,7 @@ class BaseVD(object, metaclass=abc.ABCMeta): with excutils.save_and_reraise_exception(): temp_vol_ref.destroy() - temp_vol_ref.status = 'available' + temp_vol_ref.status = status temp_vol_ref.save() return temp_vol_ref diff --git a/cinder/volume/manager.py b/cinder/volume/manager.py index 49f430c7006..194018a756d 100644 --- a/cinder/volume/manager.py +++ b/cinder/volume/manager.py @@ -1068,7 +1068,8 @@ class VolumeManager(manager.CleanableManager, 'from snapshot %s' % snapshot.id} ctxt = context.get_internal_tenant_context() or ctxt temp_vol = self.driver._create_temp_volume_from_snapshot( - ctxt, volume, snapshot, volume_options=v_options) + ctxt, volume, snapshot, volume_options=v_options, + status=fields.VolumeStatus.IN_USE) self._copy_volume_data(ctxt, temp_vol, volume) self.driver_delete_volume(temp_vol) temp_vol.destroy() @@ -1080,7 +1081,7 @@ class VolumeManager(manager.CleanableManager, " %(volume)s.", {'snapshot': snapshot.id, 'volume': volume.id}) - if temp_vol and temp_vol.status == 'available': + if temp_vol and temp_vol.status == fields.VolumeStatus.IN_USE: self.driver_delete_volume(temp_vol) temp_vol.destroy() diff --git a/releasenotes/notes/bug-1970768-temp-vol-delete-6586a13f08d7a5c1.yaml b/releasenotes/notes/bug-1970768-temp-vol-delete-6586a13f08d7a5c1.yaml new file mode 100644 index 00000000000..f7e94609990 --- /dev/null +++ b/releasenotes/notes/bug-1970768-temp-vol-delete-6586a13f08d7a5c1.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + `Bug #1970768 `_: Fixed + status of temporary volumes when creating backups and reverting to a + snapshot, preventing accidental manual deletion of those resources.