diff --git a/cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py b/cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py index 1b57575f4f1..19466394bc2 100644 --- a/cinder/tests/unit/volume/drivers/hpe/test_hpe3par.py +++ b/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 diff --git a/cinder/volume/drivers/hpe/hpe_3par_common.py b/cinder/volume/drivers/hpe/hpe_3par_common.py index c17ac64f38e..41e1d6cef67 100644 --- a/cinder/volume/drivers/hpe/hpe_3par_common.py +++ b/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