From 6e412852f2b756c83ab986fb382f5c249fa3d3ce Mon Sep 17 00:00:00 2001 From: wanghao Date: Fri, 16 Jun 2017 16:13:44 +0800 Subject: [PATCH] Fix driver exception when cascade deleting volume after transferring After transferring a volume without snapshots from one user project to another user project, if the receiving user uses cascade deleting, it will cause some exceptions in driver and volume will be error_deleting. APIImpact Change-Id: I9a870118c9c68799a4dab4bfa1f08b1e65a4796a Closes-Bug: #1698310 --- cinder/db/api.py | 4 ++++ cinder/db/sqlalchemy/api.py | 6 +++++ cinder/tests/unit/volume/test_volume.py | 22 +++++++++++++++++++ cinder/volume/api.py | 7 +++++- ...g-transferred-volume-575ef0b76bd7f334.yaml | 7 ++++++ 5 files changed, 45 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/check-snapshots-when-cascade-deleting-transferred-volume-575ef0b76bd7f334.yaml diff --git a/cinder/db/api.py b/cinder/db/api.py index 8fab078a1c4..e001fccb1e9 100644 --- a/cinder/db/api.py +++ b/cinder/db/api.py @@ -409,6 +409,10 @@ def volume_qos_allows_retype(new_vol_type): return IMPL.volume_qos_allows_retype(new_vol_type) +def volume_has_other_project_snp_filter(): + return IMPL.volume_has_other_project_snp_filter() + + #################### diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index 7a08b750695..3d117063e87 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -2618,6 +2618,12 @@ def volume_qos_allows_retype(new_vol_type): models.QualityOfServiceSpecs.value != 'back-end')))) +def volume_has_other_project_snp_filter(): + return sql.exists().where( + and_(models.Volume.id == models.Snapshot.volume_id, + models.Volume.project_id != models.Snapshot.project_id)) + + #################### diff --git a/cinder/tests/unit/volume/test_volume.py b/cinder/tests/unit/volume/test_volume.py index a1efa5fd018..7002d92e280 100644 --- a/cinder/tests/unit/volume/test_volume.py +++ b/cinder/tests/unit/volume/test_volume.py @@ -2479,6 +2479,28 @@ class VolumeTestCase(base.BaseVolumeTestCase): volume = objects.Volume.get_by_id(self.context, volume.id) self.assertEqual('deleting', volume.status) + def test_cascade_delete_volume_with_snapshots_in_other_project(self): + """Test volume deletion with dependent snapshots in other project.""" + volume = tests_utils.create_volume(self.user_context, + **self.volume_params) + snapshot = create_snapshot(volume['id'], size=volume['size'], + project_id=fake.PROJECT2_ID) + self.volume.create_snapshot(self.context, snapshot) + self.assertEqual( + snapshot.id, objects.Snapshot.get_by_id(self.context, + snapshot.id).id) + + volume['status'] = 'available' + volume['host'] = 'fakehost' + + volume_api = cinder.volume.api.API() + + self.assertRaises(exception.InvalidVolume, + volume_api.delete, + self.user_context, + volume, + cascade=True) + @mock.patch.object(driver.BaseVD, 'get_backup_device') @mock.patch.object(driver.BaseVD, 'secure_file_operations_enabled') def test_get_backup_device(self, mock_secure, mock_get_backup): diff --git a/cinder/volume/api.py b/cinder/volume/api.py index dac9795936e..c00778c8a65 100644 --- a/cinder/volume/api.py +++ b/cinder/volume/api.py @@ -418,6 +418,10 @@ class API(base.Base): else: # Allow deletion if all snapshots are in an expected state filters = [~db.volume_has_undeletable_snapshots_filter()] + # Check if the volume has snapshots which are existing in + # other project now. + if not context.is_admin: + filters.append(~db.volume_has_other_project_snp_filter()) else: # Don't allow deletion of volume with snapshots filters = [~db.volume_has_snapshots_filter()] @@ -433,7 +437,8 @@ class API(base.Base): status = utils.build_or_str(expected.get('status'), _('status must be %s and')) msg = _('Volume %s must not be migrating, attached, belong to a ' - 'group or have snapshots.') % status + 'group, have snapshots or be disassociated from ' + 'snapshots after volume transfer.') % status LOG.info(msg) raise exception.InvalidVolume(reason=msg) diff --git a/releasenotes/notes/check-snapshots-when-cascade-deleting-transferred-volume-575ef0b76bd7f334.yaml b/releasenotes/notes/check-snapshots-when-cascade-deleting-transferred-volume-575ef0b76bd7f334.yaml new file mode 100644 index 00000000000..e31ff59f123 --- /dev/null +++ b/releasenotes/notes/check-snapshots-when-cascade-deleting-transferred-volume-575ef0b76bd7f334.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - After transferring a volume without snapshots from one user project + to another user project, if the receiving user uses cascade deleting, + it will cause some exceptions in driver and volume will be error_deleting. + Adding additional check to ensure there are no snapshots left in other + project when cascade deleting a tranferred volume.