From 038eb7be4fa75fd7b8c2ad2a69f831e7186dbb9f Mon Sep 17 00:00:00 2001 From: Clinton Knight Date: Wed, 31 May 2017 13:52:02 -0400 Subject: [PATCH] NetApp ONTAP: Fix revert-to-snapshot In course of normal operation, the backend snapshot name is associated with the the ID of the snapshot instance in manila. However, when a share is replicated or is being migrated, the backend snapshot is not renamed. Instead, we simply store the original name in the "provider_location" field of the snapshot model. Fix revert-to-snapshot methods to identify snapshots by "provider_location"s rather than the instance IDs. Co-Authored-By: Goutham Pacha Ravi Change-Id: I4900c7aecc3da6640ea9c0d4d08012ff5b68cc58 Closes-Bug: #1694768 (cherry picked from commit bd47f93007227719e8b9eebbef9767d27025a236) --- .../netapp/dataontap/cluster_mode/lib_base.py | 9 ++++--- .../dataontap/cluster_mode/test_lib_base.py | 26 ++++++++++++++----- ...t-revert-to-snapshot-5e1be65260454988.yaml | 4 +++ 3 files changed, 30 insertions(+), 9 deletions(-) create mode 100644 releasenotes/notes/bug-1694768-fix-netapp-cdot-revert-to-snapshot-5e1be65260454988.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 e07c2346fa..47baa3a3e3 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py @@ -879,7 +879,8 @@ class NetAppCmodeFileStorageLibrary(object): """Reverts a share (in place) to the specified snapshot.""" vserver, vserver_client = self._get_vserver(share_server=share_server) share_name = self._get_backend_share_name(snapshot['share_id']) - snapshot_name = self._get_backend_snapshot_name(snapshot['id']) + snapshot_name = (snapshot.get('provider_location') or + self._get_backend_snapshot_name(snapshot['id'])) LOG.debug('Restoring snapshot %s', snapshot_name) vserver_client.restore_snapshot(share_name, snapshot_name) @@ -1824,8 +1825,10 @@ class NetAppCmodeFileStorageLibrary(object): vserver, vserver_client = self._get_vserver(share_server=share_server) share_name = self._get_backend_share_name( active_replica_snapshot['share_id']) - snapshot_name = self._get_backend_snapshot_name( - active_replica_snapshot['id']) + snapshot_name = ( + active_replica_snapshot.get('provider_location') or + self._get_backend_snapshot_name(active_replica_snapshot['id'])) + LOG.debug('Restoring snapshot %s', snapshot_name) dm_session = data_motion.DataMotionSession() 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 d56e870fca..d705dbea75 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 @@ -1309,22 +1309,29 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): snapshot_name) self.assertEqual(snapshot_name, model_update['provider_location']) - def test_revert_to_snapshot(self): + @ddt.data(True, False) + def test_revert_to_snapshot(self, use_snap_provider_location): vserver_client = mock.Mock() self.mock_object(self.library, '_get_vserver', mock.Mock(return_value=(fake.VSERVER1, vserver_client))) + fake_snapshot = copy.deepcopy(fake.SNAPSHOT) + if use_snap_provider_location: + fake_snapshot['provider_location'] = 'fake-provider-location' + else: + del fake_snapshot['provider_location'] result = self.library.revert_to_snapshot( - self.context, fake.SNAPSHOT, share_server=fake.SHARE_SERVER) + self.context, fake_snapshot, share_server=fake.SHARE_SERVER) self.assertIsNone(result) share_name = self.library._get_backend_share_name( - fake.SNAPSHOT['share_id']) - snapshot_name = self.library._get_backend_snapshot_name( - fake.SNAPSHOT['id']) + fake_snapshot['share_id']) + snapshot_name = (self.library._get_backend_snapshot_name( + fake_snapshot['id']) if not use_snap_provider_location + else 'fake-provider-location') vserver_client.restore_snapshot.assert_called_once_with(share_name, snapshot_name) @@ -3919,11 +3926,18 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): snapshot_list = [fake_snapshot, fake_snapshot_2, fake_snapshot_3] return replica_list, snapshot_list - def test_revert_to_replicated_snapshot(self): + @ddt.data(True, False) + def test_revert_to_replicated_snapshot(self, use_snap_provider_location): replica_list, snapshot_list = self._get_fake_replicas_and_snapshots() fake_replica, fake_replica_2, fake_replica_3 = replica_list fake_snapshot, fake_snapshot_2, fake_snapshot_3 = snapshot_list + + if not use_snap_provider_location: + del fake_snapshot['provider_location'] + del fake_snapshot_2['provider_location'] + del fake_snapshot_3['provider_location'] + share_name = self.library._get_backend_share_name( fake_snapshot['share_id']) snapshot_name = self.library._get_backend_snapshot_name( diff --git a/releasenotes/notes/bug-1694768-fix-netapp-cdot-revert-to-snapshot-5e1be65260454988.yaml b/releasenotes/notes/bug-1694768-fix-netapp-cdot-revert-to-snapshot-5e1be65260454988.yaml new file mode 100644 index 0000000000..08b975045f --- /dev/null +++ b/releasenotes/notes/bug-1694768-fix-netapp-cdot-revert-to-snapshot-5e1be65260454988.yaml @@ -0,0 +1,4 @@ +--- +fixes: + - Fixed the NetApp ONTAP driver to handle reverting to replicated + and migrated snapshots.