From 1dc445945670ac510c845f248f7b58ff96c25021 Mon Sep 17 00:00:00 2001 From: Felipe Rodrigues Date: Wed, 14 Sep 2022 22:27:56 -0300 Subject: [PATCH] [NetApp]: Fix issues with managed snapshot Managed snapshot has its name changed. When the volume is reverted to the snapshot, the name is reverted to the old too. As result, the Manila loses the snapshot reference, causing some issues. This patch is removing the code that renames the snapshot during management, so the snapshot name is kept as informed, saving it to the provider location field. Closes-bug: #1936648 Change-Id: Ib1a928d453de80d6841524cdb86c4708a5b0dbdf --- .../netapp/dataontap/cluster_mode/lib_base.py | 34 +++++-------------- .../dataontap/cluster_mode/test_lib_base.py | 34 ------------------- ...tapp-manage-snapshot-f6ed571bd4f9a2ac.yaml | 6 ++++ 3 files changed, 14 insertions(+), 60 deletions(-) create mode 100644 releasenotes/notes/fix-netapp-manage-snapshot-f6ed571bd4f9a2ac.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 543aaba65c..9c7dc2f5a4 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py @@ -2038,7 +2038,13 @@ class NetAppCmodeFileStorageLibrary(object): @na_utils.trace def manage_existing_snapshot( self, snapshot, driver_options, share_server=None): - """Brings an existing snapshot under Manila management.""" + """Brings an existing snapshot under Manila management. + + The managed snapshot keeps with its name, not renaming to the driver + snapshot name pattern (share_snapshot_), because the rename is lost + when reverting to the snapshot, causing some issues (bug #1936648). + """ + vserver, vserver_client = self._get_vserver(share_server=share_server) share_name = self._get_backend_share_name(snapshot['share_id']) existing_snapshot_name = snapshot.get('provider_location') @@ -2074,32 +2080,8 @@ class NetAppCmodeFileStorageLibrary(object): # When calculating the size, round up to the next GB. size = int(math.ceil(float(volume['size']) / units.Gi)) - update_info = {'size': size} - # NOTE(felipe_rodrigues): ONTAP does not support rename FlexGroup - # snapshots, so those managed snapshots will keep the previous name - # being referenced by its provider_location field. - is_flexgroup = na_utils.is_style_extended_flexgroup( - volume['style-extended']) - if not is_flexgroup: - new_snapshot_name = self._get_backend_snapshot_name(snapshot['id']) - update_info['provider_location'] = new_snapshot_name - - try: - vserver_client.rename_snapshot(share_name, - existing_snapshot_name, - new_snapshot_name) - except netapp_api.NaApiError: - msg = _('Could not rename snapshot %(snap)s in share %(vol)s.') - msg_args = {'snap': existing_snapshot_name, 'vol': share_name} - raise exception.ManageInvalidShareSnapshot( - reason=msg % msg_args) - - # Save original snapshot info to private storage. - original_data = {'original_name': existing_snapshot_name} - self.private_storage.update(snapshot['id'], original_data) - - return update_info + return {'size': size} @na_utils.trace def unmanage_snapshot(self, snapshot, share_server=None): 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 f8949c4071..077af53550 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 @@ -3005,9 +3005,6 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): vserver_client.get_volume.return_value = fake.FLEXVOL_TO_MANAGE vserver_client.snapshot_exists.return_value = True vserver_client.volume_has_snapmirror_relationships.return_value = False - self.mock_object( - na_utils, 'is_style_extended_flexgroup', - mock.Mock(return_value=is_flexgroup)) result = self.library.manage_existing_snapshot( fake.SNAPSHOT_TO_MANAGE, {}, share_server=fake_vserver) @@ -3019,20 +3016,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): fake.SNAPSHOT_NAME, share_name) (vserver_client.volume_has_snapmirror_relationships. assert_called_once_with(fake.FLEXVOL_TO_MANAGE)) - na_utils.is_style_extended_flexgroup.assert_called_once_with( - fake.FLEXGROUP_STYLE_EXTENDED) expected_result = {'size': 2} - if is_flexgroup: - vserver_client.rename_snapshot.assert_not_called() - self.library.private_storage.update.assert_not_called() - else: - new_snapshot_name = self.library._get_backend_snapshot_name( - fake.SNAPSHOT['id']) - vserver_client.rename_snapshot.assert_called_once_with( - share_name, fake.SNAPSHOT_NAME, new_snapshot_name) - self.library.private_storage.update.assert_called_once_with( - fake.SNAPSHOT['id'], {'original_name': fake.SNAPSHOT_NAME}) - expected_result['provider_location'] = new_snapshot_name self.assertEqual(expected_result, result) def test_manage_existing_snapshot_no_snapshot_name(self): @@ -3099,24 +3083,6 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.library.manage_existing_snapshot, fake.SNAPSHOT_TO_MANAGE, {}) - def test_manage_existing_snapshot_rename_snapshot_error(self): - - vserver_client = mock.Mock() - self.mock_object(self.library, - '_get_vserver', - mock.Mock(return_value=(fake.VSERVER1, - vserver_client))) - vserver_client.get_volume.return_value = fake.FLEXVOL_TO_MANAGE - vserver_client.volume_has_snapmirror_relationships.return_value = False - self.mock_object( - na_utils, 'is_style_extended_flexgroup', - mock.Mock(return_value=False)) - vserver_client.rename_snapshot.side_effect = netapp_api.NaApiError - - self.assertRaises(exception.ManageInvalidShareSnapshot, - self.library.manage_existing_snapshot, - fake.SNAPSHOT_TO_MANAGE, {}) - @ddt.data(None, fake.VSERVER1) def test_unmanage_snapshot(self, fake_vserver): diff --git a/releasenotes/notes/fix-netapp-manage-snapshot-f6ed571bd4f9a2ac.yaml b/releasenotes/notes/fix-netapp-manage-snapshot-f6ed571bd4f9a2ac.yaml new file mode 100644 index 0000000000..af4034b03e --- /dev/null +++ b/releasenotes/notes/fix-netapp-manage-snapshot-f6ed571bd4f9a2ac.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + NetApp driver: fix some issues caused with managed snapshot, it is no more + renaming the managed snapshot. For more details, please refer to + `launchpad bug #1936648 `_