PureISCSIDriver needs to disconnect hosts before deleting volumes.

Some error conditions can cause a situation where the volume is
“connected” to a Purity host, but the volume failed to attach on the
initiator side. If we then try to delete this volume it will cause
errors and leave orphaned volumes behind on the Flash Array.

The solution is to check for any connections and then remove them prior
to making a call to delete the volume.

Closes-Bug: 1388260
Change-Id: I8f5b3b28900d79228fd91f2ad53535224f263657

diff --git a/cinder/tests/test_pure.py b/cinder/tests/test_pure.py
index 27f36b5a10e2fc66115facf5d57526ec4bd5eafa..41a9b2ed96c0bc6e196a7fe15734739ac181fdd5 100644 (file)
--- a/cinder/tests/test_pure.py
+++ b/cinder/tests/test_pure.py
@@ -211,6 +211,29 @@ class PureISCSIDriverTestCase(test.TestCase):
         self.assert_error_propagates([self.array.destroy_volume],
                                      self.driver.delete_volume, VOLUME)
 
+    def test_delete_connected_volume(self):
+        vol_name = VOLUME["name"] + "-cinder"
+        host_name_a = "ha"
+        host_name_b = "hb"
+        self.array.list_volume_hosts.return_value = [{
+            "host": host_name_a,
+            "lun": 7,
+            "name": vol_name,
+            "size": 3221225472
+        }, {
+            "host": host_name_b,
+            "lun": 2,
+            "name": vol_name,
+            "size": 3221225472
+        }]
+
+        self.driver.delete_volume(VOLUME)
+        expected = [mock.call.list_volume_hosts(vol_name),
+                    mock.call.disconnect_host(host_name_a, vol_name),
+                    mock.call.disconnect_host(host_name_b, vol_name),
+                    mock.call.destroy_volume(vol_name)]
+        self.array.assert_has_calls(expected)
+
     def test_create_snapshot(self):
         vol_name = SRC_VOL["name"] + "-cinder"
         self.driver.create_snapshot(SNAPSHOT)
@@ -337,12 +360,13 @@ class PureISCSIDriverTestCase(test.TestCase):
         self.assertFalse(self.array.delete_host.called)
         # Branch with host added to host group
         self.array.reset_mock()
+        self.array.list_host_connections.return_value = []
         mock_host.return_value = PURE_HOST.copy()
         mock_host.return_value.update(hgroup="some-group")
         self.driver.terminate_connection(VOLUME, CONNECTOR)
         self.array.disconnect_host.assert_called_with(PURE_HOST_NAME, vol_name)
-        self.assertFalse(self.array.list_host_connections.called)
-        self.assertFalse(self.array.delete_host.called)
+        self.assertTrue(self.array.list_host_connections.called)
+        self.assertTrue(self.array.delete_host.called)
         # Branch with host still having connected volumes
         self.array.reset_mock()
         self.array.list_host_connections.return_value = [
@@ -370,6 +394,15 @@ class PureISCSIDriverTestCase(test.TestCase):
         self.array.list_host_connections.assert_called_with(PURE_HOST_NAME,
                                                             private=True)
         self.array.delete_host.assert_called_with(PURE_HOST_NAME)
+        # Branch where an unexpected exception occurs
+        self.array.reset_mock()
+        self.array.disconnect_host.side_effect = exception.PureAPIException(
+            code=500, reason="unexpected exception")
+        self.assertRaises(exception.PureAPIException,
+                          self.driver.terminate_connection, VOLUME, CONNECTOR)
+        self.array.disconnect_host.assert_called_with(PURE_HOST_NAME, vol_name)
+        self.assertFalse(self.array.list_host_connections.called)
+        self.assertFalse(self.array.delete_host.called)
 
     def test_get_volume_stats(self):
         self.assertEqual(self.driver.get_volume_stats(), {})
@@ -686,6 +719,14 @@ class FlashArrayRESTTestCase(FlashArrayBaseTestCase):
         self.assert_error_propagates([mock_req], self.array.list_ports,
                                      **self.kwargs)
 
+    def test_list_volume_hosts(self, mock_req):
+        mock_req.return_value = self.result
+        result = self.array.list_volume_hosts("vol-name")
+        self.assertEqual(result, self.result)
+        mock_req.assert_called_with(self.array, "GET", "volume/vol-name/host")
+        self.assert_error_propagates([mock_req], self.array.list_volume_hosts,
+                                     "vol-name")
+
 
 class FakeFlashArray(pure.FlashArray):
 

diff --git a/cinder/volume/drivers/pure.py b/cinder/volume/drivers/pure.py
index 683634b2ecc28b61af812dc61159e521b80b573d..d8180f37fae164b5a113fad298bd46a67fd8a343 100644 (file)
--- a/cinder/volume/drivers/pure.py
+++ b/cinder/volume/drivers/pure.py
@@ -130,9 +130,15 @@ class PureISCSIDriver(san.SanISCSIDriver):
             self._array.extend_volume(vol_name, vol_size)
 
     def delete_volume(self, volume):
-        """Deletes a volume."""
+        """Disconnect all hosts and delete the volume"""
         LOG.debug("Enter PureISCSIDriver.delete_volume.")
         vol_name = _get_vol_name(volume)
+
+        connected_hosts = self._array.list_volume_hosts(vol_name)
+        for host_info in connected_hosts:
+            host_name = host_info["host"]
+            self._disconnect_host(host_name, vol_name)
+
         try:
             self._array.destroy_volume(vol_name)
         except exception.PureAPIException as err:
@@ -252,27 +258,31 @@ class PureISCSIDriver(san.SanISCSIDriver):
         host = self._get_host(connector)
         if host:
             host_name = host["name"]
-            try:
-                self._array.disconnect_host(host_name, vol_name)
-            except exception.PureAPIException as err:
-                with excutils.save_and_reraise_exception() as ctxt:
-                    if err.kwargs["code"] == 400:
-                        # Happens if the host and volume are not connected.
-                        ctxt.reraise = False
-                        LOG.error(_LE("Disconnection failed "
-                                      "with message: {msg}."
-                                      ).format(msg=err.msg))
-            if (GENERATED_NAME.match(host_name) and not host["hgroup"] and
-                not self._array.list_host_connections(host_name,
-                                                      private=True)):
-                LOG.info(_LI("Deleting unneeded host %(host_name)r.") %
-                         {"host_name": host_name})
-                self._array.delete_host(host_name)
+            self._disconnect_host(host_name, vol_name)
         else:
             LOG.error(_LE("Unable to find host object in Purity with IQN: "
                           "{iqn}.").format(iqn=connector["initiator"]))
         LOG.debug("Leave PureISCSIDriver.terminate_connection.")
 
+    def _disconnect_host(self, host_name, vol_name):
+        LOG.debug("Enter PureISCSIDriver._disconnect_host.")
+        try:
+            self._array.disconnect_host(host_name, vol_name)
+        except exception.PureAPIException as err:
+            with excutils.save_and_reraise_exception() as ctxt:
+                if err.kwargs["code"] == 400:
+                    # Happens if the host and volume are not connected.
+                    ctxt.reraise = False
+                    LOG.error(_LE("Disconnection failed with message: "
+                                  "%(msg)s.") % {"msg": err.msg})
+        if (GENERATED_NAME.match(host_name) and
+            not self._array.list_host_connections(host_name,
+                                                  private=True)):
+            LOG.info(_LI("Deleting unneeded host %(host_name)r.") %
+                     {"host_name": host_name})
+            self._array.delete_host(host_name)
+        LOG.debug("Leave PureISCSIDriver._disconnect_host.")
+
     def get_volume_stats(self, refresh=False):
         """Return the current state of the volume service.
 
@@ -448,3 +458,7 @@ class FlashArray(object):
     def list_ports(self, **kwargs):
         """Return a list of dictionaries describing ports."""
         return self._http_request("GET", "port", kwargs)
+
+    def list_volume_hosts(self, volume):
+        """Return a list of dictionaries describing connected hosts."""
+        return self._http_request("GET", "volume/%s/host" % volume)