From 0c5511f84378e99afabb4c61f2c8330294310707 Mon Sep 17 00:00:00 2001 From: Simon Dodsley Date: Thu, 3 Jun 2021 10:48:40 -0400 Subject: [PATCH] [Pure Storage] Ensure multiattach volumes are not disconnected early Change-Id: I5135a48326f5f7adb0a380b55993660b8b38ce27 Closes-Bug: #1930748 --- cinder/tests/unit/volume/drivers/test_pure.py | 5 +- cinder/volume/drivers/pure.py | 50 ++++++++++++++++--- ..._storage_multiattach-f4aee3576757b2ff.yaml | 7 +++ 3 files changed, 52 insertions(+), 10 deletions(-) create mode 100644 releasenotes/notes/pure_storage_multiattach-f4aee3576757b2ff.yaml diff --git a/cinder/tests/unit/volume/drivers/test_pure.py b/cinder/tests/unit/volume/drivers/test_pure.py index a06bb94d430..0ce22689b40 100644 --- a/cinder/tests/unit/volume/drivers/test_pure.py +++ b/cinder/tests/unit/volume/drivers/test_pure.py @@ -657,6 +657,7 @@ class PureBaseSharedDriverTestCase(PureDriverTestCase): vol.volume_type = voltype vol.volume_type_id = voltype.id + vol.volume_attachment = None return vol, vol_name @@ -4060,9 +4061,9 @@ class PureFCDriverTestCase(PureBaseSharedDriverTestCase): self.driver.terminate_connection(vol, FC_CONNECTOR) mock_disconnect.assert_has_calls([ mock.call(mock_secondary, vol, FC_CONNECTOR, - remove_remote_hosts=False), + is_multiattach=False, remove_remote_hosts=False), mock.call(self.array, vol, FC_CONNECTOR, - remove_remote_hosts=False) + is_multiattach=False, remove_remote_hosts=False) ]) diff --git a/cinder/volume/drivers/pure.py b/cinder/volume/drivers/pure.py index 1cfbe0f7e49..5dfc374b27b 100644 --- a/cinder/volume/drivers/pure.py +++ b/cinder/volume/drivers/pure.py @@ -690,8 +690,23 @@ class PureBaseVolumeDriver(san.SanDriver): """ raise NotImplementedError + def _is_multiattach_to_host(self, volume_attachment, host_name): + # When multiattach is enabled a volume could be attached to multiple + # instances which are hosted on the same Nova compute. + # Because Purity cannot recognize the volume is attached more than + # one instance we should keep the volume attached to the Nova compute + # until the volume is detached from the last instance + if not volume_attachment: + return False + + attachment = [a for a in volume_attachment + if a.attach_status == "attached" and + a.attached_host == host_name] + return len(attachment) > 1 + @pure_driver_debug_trace - def _disconnect(self, array, volume, connector, remove_remote_hosts=False): + def _disconnect(self, array, volume, connector, remove_remote_hosts=False, + is_multiattach=False): """Disconnect the volume from the host described by the connector. If no connector is specified it will remove *all* attachments for @@ -722,11 +737,16 @@ class PureBaseVolumeDriver(san.SanDriver): remote=remove_remote_hosts) if hosts: any_in_use = False + host_in_use = False for host in hosts: host_name = host["name"] - host_in_use = self._disconnect_host(array, - host_name, - vol_name) + if not is_multiattach: + host_in_use = self._disconnect_host(array, + host_name, + vol_name) + else: + LOG.warning("Unable to disconnect host from volume. " + "Volume is multi-attached.") any_in_use = any_in_use or host_in_use return any_in_use else: @@ -739,20 +759,27 @@ class PureBaseVolumeDriver(san.SanDriver): def terminate_connection(self, volume, connector, **kwargs): """Terminate connection.""" vol_name = self._get_vol_name(volume) + # None `connector` indicates force detach, then delete all even + # if the volume is multi-attached. + multiattach = (connector is not None and + self._is_multiattach_to_host(volume.volume_attachment, + connector["host"])) if self._is_vol_in_pod(vol_name): # Try to disconnect from each host, they may not be online though # so if they fail don't cause a problem. for array in self._uniform_active_cluster_target_arrays: try: self._disconnect(array, volume, connector, - remove_remote_hosts=False) + remove_remote_hosts=False, + is_multiattach=multiattach) except purestorage.PureError as err: # Swallow any exception, just warn and continue LOG.warning("Disconnect on secondary array failed with" " message: %(msg)s", {"msg": err.text}) # Now disconnect from the current array self._disconnect(self._get_current_array(), volume, - connector, remove_remote_hosts=False) + connector, remove_remote_hosts=False, + is_multiattach=multiattach) @pure_driver_debug_trace def _disconnect_host(self, array, host_name, vol_name): @@ -2901,6 +2928,11 @@ class PureFCDriver(PureBaseVolumeDriver, driver.FibreChannelDriver): def terminate_connection(self, volume, connector, **kwargs): """Terminate connection.""" vol_name = self._get_vol_name(volume) + # None `connector` indicates force detach, then delete all even + # if the volume is multi-attached. + multiattach = (connector is not None and + self._is_multiattach_to_host(volume.volume_attachment, + connector["host"])) unused_wwns = [] if self._is_vol_in_pod(vol_name): @@ -2909,7 +2941,8 @@ class PureFCDriver(PureBaseVolumeDriver, driver.FibreChannelDriver): for array in self._uniform_active_cluster_target_arrays: try: no_more_connections = self._disconnect( - array, volume, connector, remove_remote_hosts=False) + array, volume, connector, remove_remote_hosts=False, + is_multiattach=multiattach) if no_more_connections: unused_wwns += self._get_array_wwns(array) except purestorage.PureError as err: @@ -2922,7 +2955,8 @@ class PureFCDriver(PureBaseVolumeDriver, driver.FibreChannelDriver): current_array = self._get_current_array() no_more_connections = self._disconnect(current_array, volume, connector, - remove_remote_hosts=False) + remove_remote_hosts=False, + is_multiattach=multiattach) if no_more_connections: unused_wwns += self._get_array_wwns(current_array) diff --git a/releasenotes/notes/pure_storage_multiattach-f4aee3576757b2ff.yaml b/releasenotes/notes/pure_storage_multiattach-f4aee3576757b2ff.yaml new file mode 100644 index 00000000000..f55603f309a --- /dev/null +++ b/releasenotes/notes/pure_storage_multiattach-f4aee3576757b2ff.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Pure Storage `bug #1930748 + `_: Fixed issues + with multiattched volumes being diconnected from a backend when + still listed as an attachment to an instance.