Merge "3PAR: Error out if vol cannot be converted to base" into stable/2023.1

cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py

@@ -3499,6 +3499,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
@@ -3527,6 +3528,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),
@@ -3550,6 +3552,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': {
@@ -3591,6 +3594,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),
@@ -3707,6 +3711,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 +3741,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),
@@ -3757,6 +3763,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:
@@ -3785,6 +3792,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),
@@ -3807,6 +3815,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
@@ -3837,6 +3846,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),
@@ -3935,6 +3945,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
@@ -3958,6 +3969,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
@@ -3969,6 +3981,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

cinder/volume/drivers/hpe/hpe_3par_common.py

@@ -301,6 +301,8 @@ 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
         4.0.19 - Update code to work with new WSAPI (of 2023). Bug #2015746
 
 
@@ -3149,6 +3151,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'],
@@ -3172,16 +3189,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: '
+                LOG.debug('Assigned the comment: convert_to_base_volume: '
-                      'id=%s.', volume['id'])
+                          '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'])

releasenotes/notes/hpe-3par-convert-to-base-vol-delete-snap-a460a4b1c419804a.yaml

@@ -0,0 +1,11 @@
+---
+fixes:
+  - |
+    HPE 3PAR driver `Bug #1994521 <https://bugs.launchpad.net/cinder/+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.
+