From 84378681d89c70185065df6958540f872c63bd3f Mon Sep 17 00:00:00 2001 From: Patrick East Date: Fri, 12 Sep 2014 17:09:42 -0700 Subject: [PATCH] PureISCSIDriver needs to disconnect hosts before deleting volumes. MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 --- cinder/tests/test_pure.py | 45 ++++++++++++++++++++++++++++++-- cinder/volume/drivers/pure.py | 48 ++++++++++++++++++++++------------- 2 files changed, 74 insertions(+), 19 deletions(-) diff --git a/cinder/tests/test_pure.py b/cinder/tests/test_pure.py index 27f36b5a1..41a9b2ed9 100644 --- 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 683634b2e..d8180f37f 100644 --- 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) -- 2.45.2