]> review.fuel-infra Code Review - openstack-build/cinder-build.git/commitdiff
Prevent missing Purity hosts from raising errors
authorPatrick East <patrick.east@purestorage.com>
Mon, 13 Jul 2015 22:49:40 +0000 (15:49 -0700)
committerPatrick East <patrick.east@purestorage.com>
Mon, 20 Jul 2015 17:58:37 +0000 (17:58 +0000)
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

cinder/tests/unit/test_pure.py
cinder/volume/drivers/pure.py

index 002449306dd84bd7b0c23d4c10e3007703ef2292..e9ec65261ba01aae83194ccc7ab83b9c8d230e27 100644 (file)
@@ -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):
index a3319a23ea60208953a5c0fd965f546ada55f63c..7aad3dd3f84b52b38102783c5c510bb7b0d55cb7 100644 (file)
@@ -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"]