From b9c6d4a7f7ac4bab4f30dc6a323e7eed9863a930 Mon Sep 17 00:00:00 2001 From: Lucio Seki Date: Tue, 17 Mar 2020 17:25:24 +0000 Subject: [PATCH] NetApp ONTAP: Fix iSCSI multiattach volume terminates connection When detaching a volume from an instance, NetApp ONTAP driver terminates the iSCSI connection, even if there're other connections to the LUN from the same initiator. This terminates connections to the LUN from other instances running on the same host. This patch fixes this bug by leaving the connection untouched if there are other connections from the same initiator. Now the connection is terminated only if it's the last one from the same initiator. Change-Id: I0b8b5930c9e8f3c049b82e92c5fb7b05d6ae9291 Closes-Bug: #1839384 (cherry picked from commit e27d83f4d03a89d982aebbb3fb8a8d1adafbdfc5) (cherry picked from commit 87b933db778ce5751a75e1dd10ce3a442f9e6902) --- .../volume/drivers/netapp/dataontap/fakes.py | 9 +++ .../netapp/dataontap/test_block_base.py | 71 +++++++++++++++++++ .../drivers/netapp/dataontap/block_base.py | 18 +++++ ...x-detach-multiattach-d99d33dff2fefb4c.yaml | 7 ++ 4 files changed, 105 insertions(+) create mode 100644 releasenotes/notes/netapp-ontap-fix-detach-multiattach-d99d33dff2fefb4c.yaml diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py index bad0135c2a6..e9ece0633c5 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/fakes.py @@ -538,6 +538,15 @@ test_snapshot.volume_id = 'fake_volume_id' test_snapshot.provider_location = PROVIDER_LOCATION +class test_iscsi_attachment(object): + def __getattr__(self, key): + return getattr(self, key) + + +test_iscsi_attachment = test_iscsi_attachment() +test_iscsi_attachment.connector = ISCSI_CONNECTOR + + def get_fake_net_interface_get_iter_response(): return etree.XML(""" 1 diff --git a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py index 91dc5b0cf5a..72f4d355b54 100644 --- a/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py +++ b/cinder/tests/unit/volume/drivers/netapp/dataontap/test_block_base.py @@ -260,6 +260,33 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase): 'fake_volume', fake.FC_FORMATTED_INITIATORS, protocol, None) + def test__is_multiattached_true(self): + volume = copy.deepcopy(fake.test_volume) + volume.multiattach = True + volume.volume_attachment = [ + fake.test_iscsi_attachment, + fake.test_iscsi_attachment, + ] + + self.assertTrue(self.library._is_multiattached( + volume, + fake.ISCSI_CONNECTOR)) + + def test__is_multiattached_false(self): + volume1 = copy.deepcopy(fake.test_volume) + volume1.multiattach = True + volume1.volume_attachment = [] + volume2 = copy.deepcopy(fake.test_volume) + volume2.multiattach = False + volume2.volume_attachment = [] + + self.assertFalse(self.library._is_multiattached( + volume1, + fake.ISCSI_CONNECTOR)) + self.assertFalse(self.library._is_multiattached( + volume2, + fake.ISCSI_CONNECTOR)) + @mock.patch.object(block_base.NetAppBlockStorageLibrary, '_find_mapped_lun_igroup') def test_unmap_lun_empty(self, mock_find_mapped_lun_igroup): @@ -302,6 +329,50 @@ class NetAppBlockStorageLibraryTestCase(test.TestCase): fake.ISCSI_MULTI_MAP_LIST[1]['initiator-group'])] self.zapi_client.unmap_lun.assert_has_calls(calls) + @mock.patch.object(block_base.NetAppBlockStorageLibrary, '_unmap_lun') + def test_terminate_connection_iscsi_multiattach(self, mock_unmap_lun): + volume = copy.deepcopy(fake.test_volume) + volume.multiattach = True + volume.volume_attachment = [ + fake.test_iscsi_attachment, + fake.test_iscsi_attachment, + ] + + self.library.terminate_connection_iscsi(volume, fake.ISCSI_CONNECTOR) + + mock_unmap_lun.assert_not_called() + + @mock.patch.object(block_base.NetAppBlockStorageLibrary, '_unmap_lun') + @mock.patch.object(block_base.NetAppBlockStorageLibrary, '_get_lun_attr') + def test_terminate_connection_iscsi_last_attachment(self, + mock_get_lun_attr, + mock_unmap_lun): + mock_get_lun_attr.return_value = {'Path': fake.PATH} + volume = copy.deepcopy(fake.test_volume) + volume.multiattach = True + volume.volume_attachment = [fake.test_iscsi_attachment] + + self.library.terminate_connection_iscsi(volume, fake.ISCSI_CONNECTOR) + + mock_unmap_lun.assert_called_once_with( + fake.PATH, [fake.ISCSI_CONNECTOR['initiator']]) + + @mock.patch.object(block_base.NetAppBlockStorageLibrary, '_unmap_lun') + @mock.patch.object(block_base.NetAppBlockStorageLibrary, '_get_lun_attr') + def test_terminate_connection_iscsi_all_initiators(self, mock_get_lun_attr, + mock_unmap_lun): + mock_get_lun_attr.return_value = {'Path': fake.PATH} + volume = copy.deepcopy(fake.test_volume) + volume.multiattach = True + volume.volume_attachment = [ + fake.test_iscsi_attachment, + fake.test_iscsi_attachment, + ] + + self.library.terminate_connection_iscsi(volume, None) + + mock_unmap_lun.assert_called_once_with(fake.PATH, []) + def test_find_mapped_lun_igroup(self): self.assertRaises(NotImplementedError, self.library._find_mapped_lun_igroup, diff --git a/cinder/volume/drivers/netapp/dataontap/block_base.py b/cinder/volume/drivers/netapp/dataontap/block_base.py index 1bc52643a68..832a4dc2aa1 100644 --- a/cinder/volume/drivers/netapp/dataontap/block_base.py +++ b/cinder/volume/drivers/netapp/dataontap/block_base.py @@ -888,12 +888,30 @@ class NetAppBlockStorageLibrary(object): targets = target_details_list return targets + def _is_multiattached(self, volume, connector): + """Returns whether the volume is multiattached. + + Returns True if the volume is attached to multiple instances using the + same initiator as the given one. + Returns False otherwise. + """ + if not volume.multiattach or not volume.volume_attachment: + return False + + same_connector = (True for at in volume.volume_attachment + if at.connector and + at.connector['initiator'] == connector['initiator']) + next(same_connector, False) + return next(same_connector, False) + def terminate_connection_iscsi(self, volume, connector, **kwargs): """Driver entry point to unattach a volume from an instance. Unmask the LUN on the storage system so the given initiator can no longer access it. """ + if connector and self._is_multiattached(volume, connector): + return name = volume['name'] if connector is None: diff --git a/releasenotes/notes/netapp-ontap-fix-detach-multiattach-d99d33dff2fefb4c.yaml b/releasenotes/notes/netapp-ontap-fix-detach-multiattach-d99d33dff2fefb4c.yaml new file mode 100644 index 00000000000..4d2d892c2f3 --- /dev/null +++ b/releasenotes/notes/netapp-ontap-fix-detach-multiattach-d99d33dff2fefb4c.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + NetApp ONTAP: Fixes `bug 1839384 + `__ Detaching any instance + from multiattached volume terminates connection. Now the connection is + terminated only if there're no other instances using the same initiator.