diff --git a/cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py b/cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py index 1d1e440cbdb..7916792fa02 100644 --- a/cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py +++ b/cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py @@ -3398,6 +3398,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): } mock_client = self.setup_driver(mock_conf=conf) + mock_client.getVolumeSnapshots.return_value = [] with mock.patch.object(hpecommon.HPE3PARCommon, '_create_client') as mock_create_client: mock_create_client.return_value = mock_client @@ -3426,6 +3427,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): { 'comment': comment, 'readOnly': False}), + mock.call.getVolumeSnapshots(self.VOLUME_3PAR_NAME), mock.call.copyVolume( osv_matcher, omv_matcher, HPE3PAR_CPG, mock.ANY), mock.call.getTask(mock.ANY), @@ -3449,6 +3451,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): } mock_client = self.setup_driver(mock_conf=conf) + mock_client.getVolumeSnapshots.return_value = [] _mock_volume_types.return_value = { 'name': 'gold', 'extra_specs': { @@ -3490,6 +3493,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): 'comment': comment, 'readOnly': False}), mock.call.getCPG(HPE3PAR_CPG), + mock.call.getVolumeSnapshots(self.VOLUME_3PAR_NAME), mock.call.copyVolume( osv_matcher, omv_matcher, HPE3PAR_CPG, mock.ANY), mock.call.getTask(mock.ANY), @@ -3606,6 +3610,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): 'getVolume.return_value': {} } mock_client = self.setup_driver(mock_conf=conf) + mock_client.getVolumeSnapshots.return_value = [] volume_type_hos = copy.deepcopy(self.volume_type_hos) volume_type_hos['extra_specs']['convert_to_base'] = True _mock_volume_types.return_value = volume_type_hos @@ -3635,6 +3640,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): { 'comment': comment, 'readOnly': False}), + mock.call.getVolumeSnapshots(self.VOLUME_3PAR_NAME), mock.call.copyVolume( osv_matcher, omv_matcher, HPE3PAR_CPG, mock.ANY), mock.call.getTask(mock.ANY), @@ -3656,6 +3662,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): 'getVolume.return_value': {} } mock_client = self.setup_driver(mock_conf=conf) + mock_client.getVolumeSnapshots.return_value = [] _mock_volume_types.return_value = self.volume_type_hos with mock.patch.object(hpecommon.HPE3PARCommon, '_create_client') as mock_create_client: @@ -3684,6 +3691,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): { 'comment': comment, 'readOnly': False}), + mock.call.getVolumeSnapshots(self.VOLUME_3PAR_NAME), mock.call.copyVolume( osv_matcher, omv_matcher, HPE3PAR_CPG, mock.ANY), mock.call.getTask(mock.ANY), @@ -3706,6 +3714,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): 'getVolume.return_value': {} } mock_client = self.setup_driver(mock_conf=conf) + mock_client.getVolumeSnapshots.return_value = [] volume_type_hos = copy.deepcopy(self.volume_type_hos) volume_type_hos['extra_specs']['convert_to_base'] = True _mock_volume_types.return_value = volume_type_hos @@ -3736,6 +3745,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): { 'comment': comment, 'readOnly': False}), + mock.call.getVolumeSnapshots(self.VOLUME_3PAR_NAME), mock.call.copyVolume( osv_matcher, omv_matcher, HPE3PAR_CPG, mock.ANY), mock.call.getTask(mock.ANY), @@ -3834,6 +3844,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): } mock_client = self.setup_driver(mock_conf=conf) + mock_client.getVolumeSnapshots.return_value = [] with mock.patch.object(hpecommon.HPE3PARCommon, '_create_client') as mock_create_client: mock_create_client.return_value = mock_client @@ -3857,6 +3868,7 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): } mock_client = self.setup_driver(mock_conf=conf) + mock_client.getVolumeSnapshots.return_value = [] with mock.patch.object(hpecommon.HPE3PARCommon, '_create_client') as mock_create_client: mock_create_client.return_value = mock_client @@ -3868,6 +3880,18 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver): self.volume, str(new_size)) + def test__convert_to_base_volume_failure(self): + mock_client = self.setup_driver() + mock_client.getVolumeSnapshots.return_value = ( + ['oss-nwJVbXaEQMi0w.xPutFRQw']) + with mock.patch.object(hpecommon.HPE3PARCommon, + '_create_client') as mock_create_client: + mock_create_client.return_value = mock_client + common = self.driver._login() + self.assertRaises(exception.VolumeIsBusy, + common._convert_to_base_volume, + self.volume) + @mock.patch.object(volume_types, 'get_volume_type') def test_extend_volume_replicated(self, _mock_volume_types): # Managed vs. unmanaged and periodic vs. sync are not relevant when diff --git a/cinder/volume/drivers/hpe/hpe_3par_common.py b/cinder/volume/drivers/hpe/hpe_3par_common.py index 971fac3e859..94c346b3776 100644 --- a/cinder/volume/drivers/hpe/hpe_3par_common.py +++ b/cinder/volume/drivers/hpe/hpe_3par_common.py @@ -300,11 +300,13 @@ class HPE3PARCommon(object): 4.0.16 - In multi host env, fix multi-detach operation. Bug #1958122 4.0.17 - Added get_manageable_volumes and get_manageable_snapshots. Bug #1819903 + 4.0.18 - During conversion of volume to base volume, + error out if it has child snapshot(s). Bug #1994521 """ - VERSION = "4.0.17" + VERSION = "4.0.18" stats = {} @@ -3139,6 +3141,21 @@ class HPE3PARCommon(object): compression = self.get_compression_policy( type_info['hpe3par_keys']) + + # If volume (osv-) has snapshot, while converting the volume + # to base volume (omv-), snapshot cannot be transferred to + # new base volume (omv-) i.e it remain with volume (osv-). + # So error out for such volume. + snap_list = self.client.getVolumeSnapshots(volume_name) + if snap_list: + snap_str = ",".join(snap_list) + msg = (_("Volume %(name)s has dependent snapshots: %(snap)s." + " Either flatten or remove the dependent snapshots:" + " %(snap)s for the conversion of volume %(name)s to" + " succeed." % {'name': volume_name, + 'snap': snap_str})) + raise exception.VolumeIsBusy(message=msg) + # Create a physical copy of the volume task_id = self._copy_volume(volume_name, temp_vol_name, cpg, cpg, type_info['tpvv'], @@ -3162,16 +3179,18 @@ class HPE3PARCommon(object): comment = self._get_3par_vol_comment(volume_name) if comment: self.client.modifyVolume(temp_vol_name, {'comment': comment}) - LOG.debug('Volume rename completed: convert_to_base_volume: ' - 'id=%s.', volume['id']) + LOG.debug('Assigned the comment: convert_to_base_volume: ' + 'id=%s.', volume['id']) - # Delete source volume after the copy is complete + # Delete source volume (osv-) after the copy is complete self.client.deleteVolume(volume_name) LOG.debug('Delete src volume completed: convert_to_base_volume: ' 'id=%s.', volume['id']) - # Rename the new volume to the original name + # Rename the new volume (omv-) to the original name (osv-) self.client.modifyVolume(temp_vol_name, {'newName': volume_name}) + LOG.debug('Volume rename completed: convert_to_base_volume: ' + 'id=%s.', volume['id']) LOG.info('Completed: convert_to_base_volume: ' 'id=%s.', volume['id']) diff --git a/releasenotes/notes/hpe-3par-convert-to-base-vol-delete-snap-a460a4b1c419804a.yaml b/releasenotes/notes/hpe-3par-convert-to-base-vol-delete-snap-a460a4b1c419804a.yaml new file mode 100644 index 00000000000..e087e335373 --- /dev/null +++ b/releasenotes/notes/hpe-3par-convert-to-base-vol-delete-snap-a460a4b1c419804a.yaml @@ -0,0 +1,11 @@ +--- +fixes: + - | + HPE 3PAR driver `Bug #1994521 `_: + Fixed: While performing a delete snapshot (s1) operation, the volumes (v2) + dependent on the snapshot (s1) are converted to base volumes. This + operation fails if these dependent volumes (v2) have their own dependent + snapshots (s2). The errors during the failure were vague and not helpful. + With this release, we added conditions to fail this operation early and + also added useful error message. +