From f8f3d82c5fb8a494309e5fc07ca8f92458266d42 Mon Sep 17 00:00:00 2001 From: silvacarloss Date: Fri, 29 May 2020 23:21:44 +0000 Subject: [PATCH] [NetApp] Fix CIFS promote back issue This change fixes the NetApp promote back issue when using CIFS protocol. When promoting a replica, the NetApp ONTAP driver attempts to create a new CIFS share entity (an access point as defined in [1]) for the new active replica. This behavior causes a failure since the storage identifies that a current backend CIFS share with the same name exists, considering that the reffered replica was once the active one. This issue is addressed by removing the related CIFS share entity when the replica gets promoted. [1] https://library.netapp.com/ecmdocs/ECMP1401220/html/GUID-1898D717-A510-4B3D-B2E3-CCDDD5BD0089.html Closes-Bug: #1879368 Change-Id: Id9bdd5df0ff05ea08881dd2c83397f0a367d9945 (cherry picked from commit c3aed22f9454ca684af45c929be19b8593d55609) (cherry picked from commit d2514735141bcf982e4bf043802591b3d017b526) (cherry picked from commit 226f7e758a0b704e023e6aec0dfd271d9c4d3716) (cherry picked from commit ef8d704c3b08d7de64926549a2f8f14cec3bfd0b) --- .../netapp/dataontap/cluster_mode/lib_base.py | 22 +++++++++- .../netapp/dataontap/protocols/base.py | 4 ++ .../netapp/dataontap/protocols/cifs_cmode.py | 10 +++++ .../netapp/dataontap/protocols/nfs_cmode.py | 4 ++ .../dataontap/cluster_mode/test_lib_base.py | 42 ++++++++++++++++++- .../dataontap/protocols/test_cifs_cmode.py | 6 +++ ...s-promote-back-issue-d8fe28466f9dde49.yaml | 7 ++++ 7 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 releasenotes/notes/bug-1879368-netapp-fix-cifs-promote-back-issue-d8fe28466f9dde49.yaml diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py index 9fc25e9498..8715e0101d 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py @@ -1699,9 +1699,29 @@ class NetAppCmodeFileStorageLibrary(object): replica_list) new_replica_list.append(r) - # Unmount the original active replica. orig_active_vserver = dm_session.get_vserver_from_share( orig_active_replica) + + # Cleanup the original active share if necessary + orig_active_replica_backend = ( + share_utils.extract_host(orig_active_replica['host'], + level='backend_name')) + orig_active_replica_name = self._get_backend_share_name( + orig_active_replica['id']) + orig_active_vserver_client = data_motion.get_client_for_backend( + orig_active_replica_backend, vserver_name=orig_active_vserver) + + orig_active_replica_helper = self._get_helper(orig_active_replica) + orig_active_replica_helper.set_client(orig_active_vserver_client) + + try: + orig_active_replica_helper.cleanup_demoted_replica( + orig_active_replica, orig_active_replica_name) + except exception.StorageCommunicationException: + LOG.exception("Could not cleanup the original active replica %s.", + orig_active_replica['id']) + + # Unmount the original active replica. self._unmount_orig_active_replica(orig_active_replica, orig_active_vserver) diff --git a/manila/share/drivers/netapp/dataontap/protocols/base.py b/manila/share/drivers/netapp/dataontap/protocols/base.py index f6b708c38e..5c0cc13f0b 100644 --- a/manila/share/drivers/netapp/dataontap/protocols/base.py +++ b/manila/share/drivers/netapp/dataontap/protocols/base.py @@ -71,3 +71,7 @@ class NetAppBaseHelper(object): @abc.abstractmethod def get_share_name_for_share(self, share): """Returns the flexvol name that hosts a share.""" + + @abc.abstractmethod + def cleanup_demoted_replica(self, share, share_name): + """Do some cleanup regarding the former active replica""" diff --git a/manila/share/drivers/netapp/dataontap/protocols/cifs_cmode.py b/manila/share/drivers/netapp/dataontap/protocols/cifs_cmode.py index 40beca06c4..d9e8e9e3ae 100644 --- a/manila/share/drivers/netapp/dataontap/protocols/cifs_cmode.py +++ b/manila/share/drivers/netapp/dataontap/protocols/cifs_cmode.py @@ -163,3 +163,13 @@ class NetAppCmodeCIFSHelper(base.NetAppBaseHelper): return match.group('host_ip'), match.group('share_name') else: return '', '' + + @na_utils.trace + def cleanup_demoted_replica(self, share, share_name): + """Cleans up some things regarding a demoted replica.""" + # NOTE(carloss): This is necessary due to bug 1879368. If we do not + # remove this CIFS share, in case the demoted replica is promoted + # back, the promotion will fail due to a duplicated entry for the + # share, since a create share request is sent to the backend every + # time a promotion occurs. + self._client.remove_cifs_share(share_name) diff --git a/manila/share/drivers/netapp/dataontap/protocols/nfs_cmode.py b/manila/share/drivers/netapp/dataontap/protocols/nfs_cmode.py index 5bb67ea644..6f1124294e 100644 --- a/manila/share/drivers/netapp/dataontap/protocols/nfs_cmode.py +++ b/manila/share/drivers/netapp/dataontap/protocols/nfs_cmode.py @@ -179,3 +179,7 @@ class NetAppCmodeNFSHelper(base.NetAppBaseHelper): else: self._client.rename_nfs_export_policy(actual_export_policy, expected_export_policy) + + @na_utils.trace + def cleanup_demoted_replica(self, share, share_name): + return diff --git a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py index c5574c92a9..4852cc4c75 100644 --- a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py +++ b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py @@ -3020,9 +3020,10 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): '_get_vserver', mock.Mock(return_value=(fake.VSERVER1, mock.Mock()))) + protocol_helper = mock.Mock() self.mock_object(self.library, '_get_helper', - mock.Mock(return_value=mock.Mock())) + mock.Mock(return_value=protocol_helper)) self.mock_object(self.library, '_create_export', mock.Mock(return_value='fake_export_location')) self.mock_object(self.library, '_unmount_orig_active_replica') @@ -3033,6 +3034,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): mock.Mock(return_value=mock_dm_session)) self.mock_object(mock_dm_session, 'get_vserver_from_share', mock.Mock(return_value=fake.VSERVER1)) + self.mock_object(self.client, 'cleanup_demoted_replica') replicas = self.library.promote_replica( None, [self.fake_replica, self.fake_replica_2], @@ -3058,6 +3060,44 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.library._unmount_orig_active_replica.assert_called_once_with( self.fake_replica, fake.VSERVER1) self.library._handle_qos_on_replication_change.assert_called_once() + protocol_helper.cleanup_demoted_replica.assert_called_once_with( + self.fake_replica, fake.SHARE['name']) + + def test_promote_replica_cleanup_demoted_storage_error(self): + self.mock_object(self.library, + '_get_vserver', + mock.Mock(return_value=(fake.VSERVER1, + mock.Mock()))) + protocol_helper = mock.Mock() + self.mock_object(self.library, + '_get_helper', + mock.Mock(return_value=protocol_helper)) + self.mock_object(self.library, '_create_export', + mock.Mock(return_value='fake_export_location')) + self.mock_object(self.library, '_unmount_orig_active_replica') + self.mock_object(self.library, '_handle_qos_on_replication_change') + + mock_dm_session = mock.Mock() + self.mock_object(data_motion, "DataMotionSession", + mock.Mock(return_value=mock_dm_session)) + self.mock_object(mock_dm_session, 'get_vserver_from_share', + mock.Mock(return_value=fake.VSERVER1)) + self.mock_object( + protocol_helper, 'cleanup_demoted_replica', + mock.Mock(side_effect=exception.StorageCommunicationException)) + mock_log = self.mock_object(lib_base.LOG, 'exception') + + self.library.promote_replica( + None, [self.fake_replica, self.fake_replica_2], + self.fake_replica_2, [], share_server=None) + + mock_dm_session.change_snapmirror_source.assert_called_once_with( + self.fake_replica, self.fake_replica, self.fake_replica_2, + mock.ANY + ) + protocol_helper.cleanup_demoted_replica.assert_called_once_with( + self.fake_replica, fake.SHARE['name']) + mock_log.assert_called_once() def test_promote_replica_destination_unreachable(self): self.mock_object(self.library, diff --git a/manila/tests/share/drivers/netapp/dataontap/protocols/test_cifs_cmode.py b/manila/tests/share/drivers/netapp/dataontap/protocols/test_cifs_cmode.py index ef18c4d688..5139462ace 100644 --- a/manila/tests/share/drivers/netapp/dataontap/protocols/test_cifs_cmode.py +++ b/manila/tests/share/drivers/netapp/dataontap/protocols/test_cifs_cmode.py @@ -214,3 +214,9 @@ class NetAppClusteredCIFSHelperTestCase(test.TestCase): self.assertEqual(ip, result_ip) self.assertEqual(share_name, result_share_name) + + def test_cleanup_demoted_replica(self): + self.helper.cleanup_demoted_replica(fake.CIFS_SHARE, fake.SHARE_NAME) + + self.mock_client.remove_cifs_share.assert_called_once_with( + fake.SHARE_NAME) diff --git a/releasenotes/notes/bug-1879368-netapp-fix-cifs-promote-back-issue-d8fe28466f9dde49.yaml b/releasenotes/notes/bug-1879368-netapp-fix-cifs-promote-back-issue-d8fe28466f9dde49.yaml new file mode 100644 index 0000000000..3424ff23e4 --- /dev/null +++ b/releasenotes/notes/bug-1879368-netapp-fix-cifs-promote-back-issue-d8fe28466f9dde49.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + Fixed an issue while promoting back share replicas created using CIFS + protocol. Please refer to the + `Launchpad bug #1879368 `_ + for more details.