From: Patrick East <patrick.east@purestorage.com>
Date: Mon, 13 Jul 2015 22:49:40 +0000 (-0700)
Subject: Prevent missing Purity hosts from raising errors
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=c18e1c7cad6f0209a322a0db1ec653bc7940a0d8;p=openstack-build%2Fcinder-build.git

Prevent missing Purity hosts from raising errors

Occasionally a Purity host can be deleted out from underneath Cinder.
When this happens if cinder is attempting to disconnect a volume from
that host we should treat it as a ‘success’ since if the host is gone,
it is disconnected already (as far as Purity is concerned).

To prevent one cause of this, the disconnect method will now be locked
with the connect ones so that can’t delete a host we intend to re-use.

Change-Id: Iba82c93f9e101709cf5532225068474688187b29
Closes-bug: #1472710
---

diff --git a/cinder/tests/unit/test_pure.py b/cinder/tests/unit/test_pure.py
index 002449306..e9ec65261 100644
--- a/cinder/tests/unit/test_pure.py
+++ b/cinder/tests/unit/test_pure.py
@@ -482,6 +482,21 @@ class PureBaseVolumeDriverTestCase(PureDriverTestCase):
         self.assertFalse(self.array.list_host_connections.called)
         self.assertFalse(self.array.delete_host.called)
 
+    @mock.patch(BASE_DRIVER_OBJ + "._get_host", autospec=True)
+    def test_terminate_connection_host_deleted(self, mock_host):
+        vol_name = VOLUME["name"] + "-cinder"
+        mock_host.return_value = PURE_HOST.copy()
+        self.array.reset_mock()
+        self.array.list_host_connections.return_value = []
+        self.array.delete_host.side_effect = \
+            self.purestorage_module.PureHTTPError(code=400,
+                                                  text='Host does not exist.')
+        self.driver.terminate_connection(VOLUME, ISCSI_CONNECTOR)
+        self.array.disconnect_host.assert_called_with(PURE_HOST_NAME, vol_name)
+        self.array.list_host_connections.assert_called_with(PURE_HOST_NAME,
+                                                            private=True)
+        self.array.delete_host.assert_called_once_with(PURE_HOST_NAME)
+
     @mock.patch(BASE_DRIVER_OBJ + ".get_filter_function", autospec=True)
     @mock.patch(BASE_DRIVER_OBJ + "._get_provisioned_space", autospec=True)
     def test_get_volume_stats(self, mock_space, mock_filter):
diff --git a/cinder/volume/drivers/pure.py b/cinder/volume/drivers/pure.py
index a3319a23e..7aad3dd3f 100644
--- a/cinder/volume/drivers/pure.py
+++ b/cinder/volume/drivers/pure.py
@@ -60,6 +60,8 @@ CHAP_SECRET_KEY = "PURE_TARGET_CHAP_SECRET"
 ERR_MSG_NOT_EXIST = "does not exist"
 ERR_MSG_PENDING_ERADICATION = "has been destroyed"
 
+CONNECT_LOCK_NAME = 'PureVolumeDriver_connect'
+
 
 def log_debug_trace(f):
     def wrapper(*args, **kwargs):
@@ -213,6 +215,7 @@ class PureBaseVolumeDriver(san.SanDriver):
         """
         raise NotImplementedError
 
+    @utils.synchronized(CONNECT_LOCK_NAME, external=True)
     def _disconnect(self, volume, connector, **kwargs):
         vol_name = self._get_vol_name(volume)
         host = self._get_host(connector)
@@ -247,7 +250,16 @@ class PureBaseVolumeDriver(san.SanDriver):
                                                   private=True)):
             LOG.info(_LI("Deleting unneeded host %(host_name)r."),
                      {"host_name": host_name})
-            self._array.delete_host(host_name)
+            try:
+                self._array.delete_host(host_name)
+            except purestorage.PureHTTPError as err:
+                with excutils.save_and_reraise_exception() as ctxt:
+                    if err.code == 400 and ERR_MSG_NOT_EXIST in err.text:
+                        # Happens if the host is already deleted.
+                        # This is fine though, just treat it as a warning.
+                        ctxt.reraise = False
+                        LOG.warning(_LW("Purity host deletion failed: "
+                                        "%(msg)s."), {"msg": err.text})
             return True
 
         return False
@@ -704,7 +716,7 @@ class PureISCSIDriver(PureBaseVolumeDriver, san.SanISCSIDriver):
             }
         return username, password, initiator_updates
 
-    @utils.synchronized('PureISCSIDriver._connect', external=True)
+    @utils.synchronized(CONNECT_LOCK_NAME, external=True)
     def _connect(self, volume, connector, initiator_data):
         """Connect the host and volume; return dict describing connection."""
         iqn = connector["initiator"]
@@ -810,7 +822,7 @@ class PureFCDriver(PureBaseVolumeDriver, driver.FibreChannelDriver):
 
         return properties
 
-    @utils.synchronized('PureFCDriver._connect', external=True)
+    @utils.synchronized(CONNECT_LOCK_NAME, external=True)
     def _connect(self, volume, connector):
         """Connect the host and volume; return dict describing connection."""
         wwns = connector["wwpns"]