From a745e9c7c48f0bfdd830f13094f588fe085b4845 Mon Sep 17 00:00:00 2001 From: Nahim Alves de Souza Date: Thu, 24 Jun 2021 14:13:13 -0300 Subject: [PATCH] [NetApp] Fixed scoped account replica delete In the ONTAP 9.8 and older, there is a bug in the zapi call `snapmirror-release-iter` when using the scoped account configuration. The operation is returning success, but the release is not occurring. To avoid this issue, the Netapp driver was changed to release with the `snapmirror-release` zapi call, instead of the one that has a bug. This new zapi call requires the field `relationship-id` to be specified and it is being retrieved from ONTAP before calling it. A small fix, not related to the bug, was made in the zapi call to `snapmirror-release-iter` because the `relationship-info-only` field was in the wrong place according to the documentation. Closes-Bug: #1934889 Change-Id: I21633d4ffe14983365b0b6239216ed5d0bbfaace --- .../netapp/dataontap/client/client_cmode.py | 57 +++++++++++------- .../dataontap/client/test_client_cmode.py | 60 +++++++++++++------ ...e-for-scoped-account-8fa193c0424af9b1.yaml | 7 +++ 3 files changed, 83 insertions(+), 41 deletions(-) create mode 100644 releasenotes/notes/bug-1934889-netapp-fix-replica-delete-for-scoped-account-8fa193c0424af9b1.yaml diff --git a/manila/share/drivers/netapp/dataontap/client/client_cmode.py b/manila/share/drivers/netapp/dataontap/client/client_cmode.py index 969493af19..af6fe63ff9 100644 --- a/manila/share/drivers/netapp/dataontap/client/client_cmode.py +++ b/manila/share/drivers/netapp/dataontap/client/client_cmode.py @@ -4098,11 +4098,36 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): dest_vserver, dest_volume, relationship_info_only=False): """Removes a SnapMirror relationship on the source endpoint.""" - self._release_snapmirror(source_vserver=source_vserver, - dest_vserver=dest_vserver, - source_volume=source_volume, - dest_volume=dest_volume, - relationship_info_only=relationship_info_only) + + self._ensure_snapmirror_v2() + snapmirror_destinations_list = self.get_snapmirror_destinations( + source_vserver=source_vserver, + dest_vserver=dest_vserver, + source_volume=source_volume, + dest_volume=dest_volume, + desired_attributes=['relationship-id']) + + if len(snapmirror_destinations_list) > 1: + msg = ("Expected snapmirror relationship to be unique. " + "List returned: %s." % snapmirror_destinations_list) + raise exception.NetAppException(msg) + + api_args = self._build_snapmirror_request( + source_vserver=source_vserver, dest_vserver=dest_vserver, + source_volume=source_volume, dest_volume=dest_volume) + api_args['relationship-info-only'] = ( + 'true' if relationship_info_only else 'false') + + # NOTE(nahimsouza): This verification is needed because an empty list + # is returned in snapmirror_destinations_list when a single share is + # created with only one replica and this replica is deleted, thus there + # will be no relationship-id in that case. + if len(snapmirror_destinations_list) == 1: + api_args['relationship-id'] = ( + snapmirror_destinations_list[0]['relationship-id']) + + self.send_request('snapmirror-release', api_args, + enable_tunneling=True) @na_utils.trace def release_snapmirror_svm(self, source_vserver, dest_vserver, @@ -4110,30 +4135,18 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): """Removes a SnapMirror relationship on the source endpoint.""" source_path = source_vserver + ':' dest_path = dest_vserver + ':' - self._release_snapmirror(source_path=source_path, dest_path=dest_path, - relationship_info_only=relationship_info_only, - enable_tunneling=False) - - @na_utils.trace - def _release_snapmirror(self, source_path=None, dest_path=None, - source_vserver=None, dest_vserver=None, - source_volume=None, dest_volume=None, - relationship_info_only=False, - enable_tunneling=True): - """Removes a SnapMirror relationship on the source endpoint.""" dest_info = self._build_snapmirror_request( - source_path, dest_path, source_vserver, - dest_vserver, source_volume, dest_volume) + source_path=source_path, dest_path=dest_path) self._ensure_snapmirror_v2() - dest_info['relationship-info-only'] = ( - 'true' if relationship_info_only else 'false') api_args = { 'query': { 'snapmirror-destination-info': dest_info - } + }, + 'relationship-info-only': ( + 'true' if relationship_info_only else 'false'), } self.send_request('snapmirror-release-iter', api_args, - enable_tunneling=enable_tunneling) + enable_tunneling=False) @na_utils.trace def quiesce_snapmirror_vol(self, source_vserver, source_volume, diff --git a/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py b/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py index 55cbb222d7..301d70e6cf 100644 --- a/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py +++ b/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py @@ -6233,10 +6233,22 @@ class NetAppClientCmodeTestCase(test.TestCase): } self.assertEqual(expected, result) - @ddt.data(True, False) - def test_release_snapmirror(self, relationship_info_only): - + @ddt.data({'snapmirror_destinations_list': [], + 'relationship_info_only': True}, + {'snapmirror_destinations_list': [], + 'relationship_info_only': False}, + {'snapmirror_destinations_list': + [{'relationship-id': 'fake_relationship_id'}], + 'relationship_info_only': True}, + {'snapmirror_destinations_list': + [{'relationship-id': 'fake_relationship_id'}], + 'relationship_info_only': False}) + @ddt.unpack + def test_release_snapmirror_vol(self, relationship_info_only, + snapmirror_destinations_list): self.mock_object(self.client, 'send_request') + self.mock_object(self.client, 'get_snapmirror_destinations', + mock.Mock(return_value=snapmirror_destinations_list)) self.client.release_snapmirror_vol( fake.SM_SOURCE_VSERVER, fake.SM_SOURCE_VOLUME, @@ -6244,21 +6256,31 @@ class NetAppClientCmodeTestCase(test.TestCase): relationship_info_only=relationship_info_only) snapmirror_release_args = { - 'query': { - 'snapmirror-destination-info': { - 'source-vserver': fake.SM_SOURCE_VSERVER, - 'source-volume': fake.SM_SOURCE_VOLUME, - 'destination-vserver': fake.SM_DEST_VSERVER, - 'destination-volume': fake.SM_DEST_VOLUME, - 'relationship-info-only': ('true' if relationship_info_only - else 'false'), - } - } + 'source-vserver': fake.SM_SOURCE_VSERVER, + 'source-volume': fake.SM_SOURCE_VOLUME, + 'destination-vserver': fake.SM_DEST_VSERVER, + 'destination-volume': fake.SM_DEST_VOLUME, + 'relationship-info-only': ('true' if relationship_info_only + else 'false'), } - self.client.send_request.assert_has_calls([ - mock.call('snapmirror-release-iter', snapmirror_release_args, - enable_tunneling=True)]) + if len(snapmirror_destinations_list) == 1: + snapmirror_release_args['relationship-id'] = 'fake_relationship_id' + + self.client.send_request.assert_called_once_with( + 'snapmirror-release', snapmirror_release_args, + enable_tunneling=True) + + def test_release_snapmirror_vol_error_not_unique_relationship(self): + self.mock_object(self.client, 'send_request') + self.mock_object(self.client, 'get_snapmirror_destinations', + mock.Mock(return_value=[{'relationship-id': 'fake'}, + {'relationship-id': 'fake'}])) + + self.assertRaises(exception.NetAppException, + self.client.release_snapmirror_vol, + fake.SM_SOURCE_VSERVER, fake.SM_SOURCE_VOLUME, + fake.SM_DEST_VSERVER, fake.SM_DEST_VOLUME) def test_release_snapmirror_svm(self): self.mock_object(self.client, 'send_request') @@ -6271,9 +6293,9 @@ class NetAppClientCmodeTestCase(test.TestCase): 'snapmirror-destination-info': { 'source-location': fake.SM_SOURCE_VSERVER + ':', 'destination-location': fake.SM_DEST_VSERVER + ':', - 'relationship-info-only': 'false' - } - } + }, + }, + 'relationship-info-only': 'false', } self.client.send_request.assert_has_calls([ mock.call('snapmirror-release-iter', snapmirror_release_args, diff --git a/releasenotes/notes/bug-1934889-netapp-fix-replica-delete-for-scoped-account-8fa193c0424af9b1.yaml b/releasenotes/notes/bug-1934889-netapp-fix-replica-delete-for-scoped-account-8fa193c0424af9b1.yaml new file mode 100644 index 0000000000..26c697c5a8 --- /dev/null +++ b/releasenotes/notes/bug-1934889-netapp-fix-replica-delete-for-scoped-account-8fa193c0424af9b1.yaml @@ -0,0 +1,7 @@ +--- +fixes: + - | + NetApp driver: fixed an issue with the ONTAP 9.8 and older, for scoped + account users, where the operation of deleting a replica was not working, + but returned a message of success. For more details, please refer to + `launchpad bug #1934889 `_ \ No newline at end of file