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 `_