From e9cd1bc89e5610de84b116d6c3687d4f796f527c Mon Sep 17 00:00:00 2001 From: Patrick East Date: Tue, 6 Jun 2017 13:51:01 -0700 Subject: [PATCH] Allow Pure drivers to handle detach with no host Apparently force-detach is ok with no connector object passed in. This changes the driver to assume that no connector means just detach the volume from anything its attached to. I updated the base driver to document this quirk too. This should only ever happen with force detach and "old" style connections, which means multi-attach shouldn't be in the picture so this is kind of safe... probably.. Change-Id: Ia0710d66bdbc85afffdb6e4f22f3ce6ca642ec75 Closes-Bug: #1696215 --- cinder/interface/volume_driver.py | 3 +- cinder/tests/unit/volume/drivers/test_pure.py | 21 +++++++++++ cinder/volume/driver.py | 14 ++++++-- cinder/volume/drivers/pure.py | 36 +++++++++++++------ 4 files changed, 61 insertions(+), 13 deletions(-) diff --git a/cinder/interface/volume_driver.py b/cinder/interface/volume_driver.py index ed54ed27f56..0473e726adb 100644 --- a/cinder/interface/volume_driver.py +++ b/cinder/interface/volume_driver.py @@ -206,7 +206,8 @@ class VolumeDriverCore(base.CinderInterface): :param volume: The volume to remove. :param connector: The Dictionary containing information about the - connection. + connection. This is optional when doing a + force-detach and can be None. """ def detach_volume(self, context, volume, attachment=None): diff --git a/cinder/tests/unit/volume/drivers/test_pure.py b/cinder/tests/unit/volume/drivers/test_pure.py index f74319565ac..f5619096a4b 100644 --- a/cinder/tests/unit/volume/drivers/test_pure.py +++ b/cinder/tests/unit/volume/drivers/test_pure.py @@ -967,6 +967,27 @@ class PureBaseVolumeDriverTestCase(PureBaseSharedDriverTestCase): 'Host cannot be deleted due to existing connections.' ) + def test_terminate_connection_no_connector_with_host(self): + # Show the volume having a connection + self.array.list_volume_private_connections.return_value = \ + [VOLUME_CONNECTIONS[0]] + + self.driver.terminate_connection(VOLUME, None) + self.array.disconnect_host.assert_called_with( + VOLUME_CONNECTIONS[0]["host"], + VOLUME_CONNECTIONS[0]["name"] + ) + + def test_terminate_connection_no_connector_no_host(self): + vol = fake_volume.fake_volume_obj(None, name=VOLUME["name"]) + + # Show the volume having a connection + self.array.list_volume_private_connections.return_value = [] + + # Make sure + self.driver.terminate_connection(vol, None) + self.array.disconnect_host.assert_not_called() + def test_extend_volume(self): vol_name = VOLUME["name"] + "-cinder" self.driver.extend_volume(VOLUME, 3) diff --git a/cinder/volume/driver.py b/cinder/volume/driver.py index 3ac07ffadb5..c8d6640848c 100644 --- a/cinder/volume/driver.py +++ b/cinder/volume/driver.py @@ -1361,7 +1361,12 @@ class BaseVD(object): @abc.abstractmethod def terminate_connection(self, volume, connector, **kwargs): - """Disallow connection from connector.""" + """Disallow connection from connector. + + :param volume: The volume to be disconnected. + :param connector: A dictionary describing the connection with details + about the initiator. Can be None. + """ return def terminate_connection_snapshot(self, snapshot, connector, **kwargs): @@ -2095,7 +2100,12 @@ class VolumeDriver(ManageableVD, CloneableImageVD, ManageableSnapshotsVD, """Allow connection from connector for a snapshot.""" def terminate_connection(self, volume, connector, **kwargs): - """Disallow connection from connector""" + """Disallow connection from connector + + :param volume: The volume to be disconnected. + :param connector: A dictionary describing the connection with details + about the initiator. Can be None. + """ def terminate_connection_snapshot(self, snapshot, connector, **kwargs): """Disallow connection from connector for a snapshot.""" diff --git a/cinder/volume/drivers/pure.py b/cinder/volume/drivers/pure.py index 09d38588578..ad10bf4c714 100644 --- a/cinder/volume/drivers/pure.py +++ b/cinder/volume/drivers/pure.py @@ -428,17 +428,33 @@ class PureBaseVolumeDriver(san.SanDriver): raise NotImplementedError def _disconnect(self, array, volume, connector, **kwargs): - vol_name = self._get_vol_name(volume) - host = self._get_host(array, connector) - if host: - host_name = host["name"] - result = self._disconnect_host(array, host_name, vol_name) - else: - LOG.error("Unable to disconnect host from volume, could not " - "determine Purity host") - result = False + """Disconnect the volume from the host described by the connector. - return result + If no connector is specified it will remove *all* attachments for + the volume. + + Returns True if it was the hosts last connection. + """ + vol_name = self._get_vol_name(volume) + if connector is None: + # If no connector was provided it is a force-detach, remove all + # host connections for the volume + LOG.warning("Removing ALL host connections for volume %s", + vol_name) + connections = array.list_volume_private_connections(vol_name) + for connection in connections: + self._disconnect_host(array, connection['host'], vol_name) + return False + else: + # Normal case with a specific initiator to detach it from + host = self._get_host(array, connector) + if host: + host_name = host["name"] + return self._disconnect_host(array, host_name, vol_name) + else: + LOG.error("Unable to disconnect host from volume, could not " + "determine Purity host") + return False @pure_driver_debug_trace def terminate_connection(self, volume, connector, **kwargs):