3PAR: Added retries on volume deletion

online copy can be completed so the condition
to find active task of online copy can be missed
and this results in deleteVolume failure

Added a retries to delete a volume

Change-Id: I902896d4127c219aeea62a4c0994c6d5bb9e82f3
Closes-Bug: #1783934

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

@@ -2257,6 +2257,37 @@ class TestHPE3PARDriverBase(HPE3PARBaseDriver):
                 expected +
                 self.standard_logout)
 
+    def _FakeRetrying(wait_func=None,
+                      original_retrying = hpecommon.utils.retrying.Retrying,
+                      *args, **kwargs):
+        return original_retrying(wait_func=lambda *a, **k: 0,
+                                 *args, **kwargs)
+
+    @mock.patch('retrying.Retrying', _FakeRetrying)
+    def test_delete_volume_online_active_done(self):
+        # setup_mock_client drive with default configuration
+        # and return the mock HTTP 3PAR client
+        mock_client = self.setup_driver()
+        ex = hpeexceptions.HTTPConflict("Online clone is active.")
+        ex._error_code = 151
+        mock_client.deleteVolume = mock.Mock(side_effect=[ex, 200])
+        mock_client.isOnlinePhysicalCopy.return_value = False
+
+        with mock.patch.object(hpecommon.HPE3PARCommon,
+                               '_create_client') as mock_create_client:
+            mock_create_client.return_value = mock_client
+            self.driver.delete_volume(self.volume)
+
+            expected = [
+                mock.call.deleteVolume(self.VOLUME_3PAR_NAME),
+                mock.call.isOnlinePhysicalCopy(self.VOLUME_3PAR_NAME),
+                mock.call.deleteVolume(self.VOLUME_3PAR_NAME)]
+
+            mock_client.assert_has_calls(
+                self.standard_login +
+                expected +
+                self.standard_logout)
+
     @mock.patch.object(volume_types, 'get_volume_type')
     def test_delete_volume_replicated(self, _mock_volume_types):
         # setup_mock_client drive with default configuration

cinder/volume/drivers/hpe/hpe_3par_common.py

@@ -42,17 +42,10 @@ import re
 import six
 import uuid
 
-from oslo_serialization import base64
-from oslo_utils import importutils
-
-hpe3parclient = importutils.try_import("hpe3parclient")
-if hpe3parclient:
-    from hpe3parclient import client
-    from hpe3parclient import exceptions as hpeexceptions
-
 from oslo_config import cfg
 from oslo_log import log as logging
 from oslo_log import versionutils
+from oslo_serialization import base64
 from oslo_service import loopingcall
 from oslo_utils import excutils
 from oslo_utils import units
@@ -62,6 +55,7 @@ from cinder import exception
 from cinder import flow_utils
 from cinder.i18n import _
 from cinder.objects import fields
+from cinder import utils
 from cinder.volume import configuration
 from cinder.volume import qos_specs
 from cinder.volume import utils as volume_utils
@@ -70,6 +64,15 @@ from cinder.volume import volume_types
 import taskflow.engines
 from taskflow.patterns import linear_flow
 
+try:
+    import hpe3parclient
+    from hpe3parclient import client
+    from hpe3parclient import exceptions as hpeexceptions
+except ImportError:
+    hpe3parclient = None
+    client = None
+    hpeexceptions = None
+
 LOG = logging.getLogger(__name__)
 
 MIN_CLIENT_VERSION = '4.2.0'
@@ -269,11 +272,12 @@ class HPE3PARCommon(object):
         4.0.8 - Added support for report backend state in service list.
         4.0.9 - Set proper backend on subsequent operation, after group
                 failover. bug #1773069
+        4.0.10 - Added retry in delete_volume. bug #1783934
 
 
     """
 
-    VERSION = "4.0.9"
+    VERSION = "4.0.10"
 
     stats = {}
 
@@ -499,6 +503,12 @@ class HPE3PARCommon(object):
             self.client.id = stats['array_id']
 
     def check_for_setup_error(self):
+        """Verify that requirements are in place to use HPE driver."""
+        if not all((hpe3parclient, client, hpeexceptions)):
+            msg = _('HPE driver setup error: some required '
+                    'libraries (hpe3parclient, client.*) not found.')
+            LOG.error(msg)
+            raise exception.VolumeDriverException(message=msg)
         if self.client:
             self.client_login()
             try:
@@ -2465,6 +2475,17 @@ class HPE3PARCommon(object):
             raise exception.CinderException(ex)
 
     def delete_volume(self, volume):
+
+        @utils.retry(exception.VolumeIsBusy, interval=2, retries=10)
+        def _try_remove_volume(volume_name):
+            try:
+                self.client.deleteVolume(volume_name)
+            except Exception:
+                msg = _("The volume is currently busy on the 3PAR "
+                        "and cannot be deleted at this time. "
+                        "You can try again later.")
+                raise exception.VolumeIsBusy(message=msg)
+
         # v2 replication check
         # If the volume type is replication enabled, we want to call our own
         # method of deconstructing the volume and its dependencies
@@ -2515,13 +2536,7 @@ class HPE3PARCommon(object):
                     else:
                         # the volume is being operated on in a background
                         # task on the 3PAR.
-                        # TODO(walter-boring) do a retry a few times.
-                        # for now lets log a better message
-                        msg = _("The volume is currently busy on the 3PAR"
-                                " and cannot be deleted at this time. "
-                                "You can try again later.")
-                        LOG.error(msg)
-                        raise exception.VolumeIsBusy(message=msg)
+                        _try_remove_volume(volume_name)
                 elif (ex.get_code() == 32):
                     # Error 32 means that the volume has children