PowerMax Driver - Miscellaneous improvements to delete

1.  Remove rollback code to re-add volume to storage group as this operation
is really not necessary on a delete operation.
2.  Change certain error messages to debugs as these can be misleading
when debugging the logs.
3.  Check if source volume is still on the array, if not a sync check is
unnecessary.

Change-Id: Id261176018c81704d7551a661a0270407f00658a

cinder/tests/unit/volume/drivers/dell_emc/powermax/test_powermax_common.py

@@ -1023,8 +1023,7 @@ class PowerMaxCommonTest(test.TestCase):
             self.assertRaises(exception.VolumeBackendAPIException,
                               self.common._delete_from_srp, array,
                               device_id, volume_name, extra_specs)
-            mock_add.assert_called_once_with(
+            mock_add.assert_not_called()
-                array, device_id, volume_name, extra_specs)
 
     @mock.patch.object(utils.PowerMaxUtils, 'is_replication_enabled',
                        side_effect=[False, True])
@@ -2793,3 +2792,16 @@ class PowerMaxCommonTest(test.TestCase):
     def test_get_replication_info(self, mock_config):
         self.common._get_replication_info()
         mock_config.assert_not_called()
+
+    @mock.patch.object(common.PowerMaxCommon,
+                       '_do_sync_check')
+    def test_sync_check_no_source_device_on_array(self, mock_check):
+        with mock.patch.object(self.rest, 'get_volume',
+                               side_effect=exception.VolumeBackendAPIException(
+                                   "404 00123 does not exist")):
+            array = self.data.array
+            device_id = self.data.device_id
+            extra_specs = self.data.extra_specs
+            self.common._sync_check(array, device_id, extra_specs,
+                                    source_device_id='00123')
+            mock_check.assert_not_called()

cinder/volume/drivers/dell_emc/powermax/common.py

@@ -1731,11 +1731,9 @@ class PowerMaxCommon(object):
                 foundsnap_name = None
 
         if foundsnap_name is None or sourcedevice_id is None:
-            exception_message = (_("Error retrieving snapshot details. "
+            LOG.debug("Error retrieving snapshot details. "
-                                   "Snapshot name: %(snap)s") %
+                      "Snapshot name: %(snap)s",
-                                 {'snap': volume_name})
+                      {'snap': volume_name})
-            LOG.error(exception_message)
-
         else:
             LOG.debug("Source volume: %(volume_name)s  Snap name: "
                       "%(foundsnap_name)s.",
@@ -1801,6 +1799,12 @@ class PowerMaxCommon(object):
         # Remove from any storage groups and cleanup replication
         self._remove_vol_and_cleanup_replication(
             array, device_id, volume_name, extra_specs, volume)
+        # Check if volume is in any storage group
+        sg_list = self.rest.get_storage_groups_from_volume(array, device_id)
+        if sg_list:
+            LOG.error("Device %(device_id)s is in storage group(s) "
+                      "%(sg_list)s prior to delete. Delete will fail.",
+                      {'device_id': device_id, 'sg_list': sg_list})
         self._delete_from_srp(
             array, device_id, volume_name, extra_specs)
         return volume_name
@@ -1997,12 +2001,6 @@ class PowerMaxCommon(object):
             self.provision.delete_volume_from_srp(
                 array, device_id, volume_name)
         except Exception as e:
-            # If we cannot successfully delete the volume, then we want to
-            # return the volume to the default storage group,
-            # which should be the SG it previously belonged to.
-            self.masking.add_volume_to_default_storage_group(
-                array, device_id, volume_name, extra_specs)
-
             error_message = (_("Failed to delete volume %(volume_name)s. "
                                "Exception received: %(e)s") %
                              {'volume_name': volume_name,
@@ -2233,6 +2231,14 @@ class PowerMaxCommon(object):
         if source_device_id:
             @coordination.synchronized("emc-source-{source_device_id}")
             def do_unlink_and_delete_snap(source_device_id):
+                # Check if source device exists on the array
+                try:
+                    self.rest.get_volume(array, source_device_id)
+                except exception.VolumeBackendAPIException:
+                    LOG.debug("Device %(device_id)s not found on array, no "
+                              "sync check required.",
+                              {'device_id': source_device_id})
+                    return
                 self._do_sync_check(
                     array, device_id, extra_specs, tgt_only)
 

cinder/volume/drivers/dell_emc/powermax/masking.py

@@ -1338,15 +1338,18 @@ class PowerMaxMasking(object):
             self.add_volume_to_default_storage_group(
                 serial_number, device_id, volume_name,
                 extra_specs, src_sg=storagegroup_name)
+            LOG.debug(
+                "Volume %(volume_name)s successfully added to "
+                "storage group %(sg)s.",
+                {'volume_name': volume_name, 'sg': storagegroup_name})
         else:
             self.remove_vol_from_storage_group(
                 serial_number, device_id, storagegroup_name,
                 volume_name, extra_specs)
-
+            LOG.debug(
-        LOG.debug(
+                "Volume %(volume_name)s successfully removed from "
-            "Volume %(volume_name)s successfully moved/ removed from "
+                "storage group %(sg)s.",
-            "storage group %(sg)s.",
+                {'volume_name': volume_name, 'sg': storagegroup_name})
-            {'volume_name': volume_name, 'sg': storagegroup_name})
 
         num_vol_in_sg = self.rest.get_num_vols_in_sg(
             serial_number, storagegroup_name)

cinder/volume/drivers/dell_emc/powermax/rest.py

@@ -517,6 +517,7 @@ class PowerMaxRest(object):
         self.check_status_code_success(operation, status_code, message)
         return status_code, message
 
+    @retry(retry_exc_tuple, interval=2, retries=3)
     def delete_resource(
             self, array, category, resource_type, resource_name,
             payload=None, private='', params=None):