[NetApp] Fix NetApp driver create from snapshot

Changes were done in create_share_from_snapshot
method so that a scoped account does not need to get the
source and the destination cluster names and does not have the
permissions.

Closes-Bug: #1922512
Change-Id: Ib36c81c213a374a918378854ce0a89ce70acf1d0

manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py

@@ -31,6 +31,7 @@ from oslo_service import loopingcall
 from oslo_utils import excutils
 from oslo_utils import timeutils
 from oslo_utils import units
+from oslo_utils import uuidutils
 
 from manila.common import constants
 from manila import coordination
@@ -752,11 +753,15 @@ class NetAppCmodeFileStorageLibrary(object):
             dm_session.get_backend_info_for_share(parent_share))
         src_vserver_client = data_motion.get_client_for_backend(
             src_backend, vserver_name=src_vserver)
-        src_cluster_name = src_vserver_client.get_cluster_name()
 
         # Destination host info
         dest_vserver, dest_vserver_client = self._get_vserver(share_server)
-        dest_cluster_name = dest_vserver_client.get_cluster_name()
+
+        src_cluster_name = None
+        dest_cluster_name = None
+        if self._have_cluster_creds:
+            src_cluster_name = src_vserver_client.get_cluster_name()
+            dest_cluster_name = dest_vserver_client.get_cluster_name()
 
         # the parent share and new share must reside on same pool type: either
         # FlexGroup or FlexVol.
@@ -805,13 +810,17 @@ class NetAppCmodeFileStorageLibrary(object):
             # NOTE(felipe_rodrigues): no support to move volumes that are
             # FlexGroup or without the cluster credential. So, performs the
             # workaround using snapmirror even being in the same cluster.
+
             if (src_cluster_name != dest_cluster_name or dest_is_flexgroup or
                     not self._have_cluster_creds):
                 # 1. Create a clone on source (temporary volume). We don't need
                 # to split from clone in order to replicate data. We don't need
                 # to create fpolicies since this copy will be deleted.
+                temp_share = copy.deepcopy(dest_share)
+                temp_uuid = uuidutils.generate_uuid()
+                temp_share['id'] = str(temp_uuid)
                 self._allocate_container_from_snapshot(
-                    dest_share, snapshot, src_vserver, src_vserver_client,
+                    temp_share, snapshot, src_vserver, src_vserver_client,
                     split=False, create_fpolicy=False)
                 # 2. Create a replica in destination host.
                 self._allocate_container(
@@ -822,6 +831,7 @@ class NetAppCmodeFileStorageLibrary(object):
                     constants.REPLICA_STATE_ACTIVE)
                 relationship_type = na_utils.get_relationship_type(
                     dest_is_flexgroup)
+                src_share_instance["id"] = temp_share['id']
                 dm_session.create_snapmirror(
                     src_share_instance,
                     dest_share,

manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_base.py

@@ -899,8 +899,8 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
             self.mock_dm_session, 'create_snapmirror')
         self.mock_storage_update = self.mock_object(
             self.library.private_storage, 'update')
-        self.mock_object(self.library, '_have_cluster_creds',
+        self.mock_generate_uuid = self.mock_object(
-                         mock.Mock(return_value=True))
+            uuidutils, 'generate_uuid', mock.Mock(return_value=fake.SHARE_ID5))
 
         # Parent share on MANILA_HOST_2
         self.parent_share = copy.copy(fake.SHARE)
@@ -917,13 +917,22 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
             'share_server': self.parent_share_server or None
         }
 
-    @ddt.data({'dest_cluster': fake.CLUSTER_NAME, 'is_flexgroup': False},
+    @ddt.data({'dest_cluster': fake.CLUSTER_NAME, 'is_flexgroup': False,
-              {'dest_cluster': fake.CLUSTER_NAME, 'is_flexgroup': True},
+              'have_cluster_creds': False},
-              {'dest_cluster': fake.CLUSTER_NAME_2, 'is_flexgroup': False},
+              {'dest_cluster': fake.CLUSTER_NAME, 'is_flexgroup': False,
-              {'dest_cluster': fake.CLUSTER_NAME_2, 'is_flexgroup': True})
+              'have_cluster_creds': True},
+              {'dest_cluster': fake.CLUSTER_NAME, 'is_flexgroup': True,
+              'have_cluster_creds': False},
+              {'dest_cluster': fake.CLUSTER_NAME_2, 'is_flexgroup': False,
+              'have_cluster_creds': False},
+              {'dest_cluster': fake.CLUSTER_NAME_2, 'is_flexgroup': False,
+              'have_cluster_creds': True},
+              )
     @ddt.unpack
     def test_create_share_from_snapshot_another_host(self, dest_cluster,
-                                                     is_flexgroup):
+                                                     is_flexgroup,
+                                                     have_cluster_creds):
+        self.library._have_cluster_creds = have_cluster_creds
         self._setup_mocks_for_create_share_from_snapshot(
             dest_cluster=dest_cluster, is_flexgroup=is_flexgroup)
         mock_get_backend_shr_name = self.mock_object(
@@ -942,9 +951,14 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
         self.mock_dm_backend.assert_called_once_with(self.parent_share)
         self.mock_dm_get_src_client.assert_called_once_with(
             fake.BACKEND_NAME, vserver_name=fake.VSERVER1)
-        self.mock_get_src_cluster.assert_called_once()
+
         self.mock_get_vserver.assert_called_once_with(self.fake_share_server)
-        self.mock_get_dest_cluster.assert_called_once()
+        if have_cluster_creds:
+            self.mock_get_dest_cluster.assert_called_once()
+            self.mock_get_src_cluster.assert_called_once()
+        else:
+            self.mock_get_dest_cluster.assert_not_called()
+            self.mock_get_src_cluster.assert_not_called()
         mock_get_backend_shr_name.assert_called_once()
         self.mock_is_flexgroup_share.assert_called_once()
         self.mock_is_flexgroup_pool.assert_called_once()
@@ -953,9 +967,12 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
         self.assertEqual(is_flexgroup,
                          self.mock_get_flexgroup_aggregate_list.called)
 
-        if dest_cluster != fake.CLUSTER_NAME or is_flexgroup:
+        if (dest_cluster != fake.CLUSTER_NAME
+                or is_flexgroup or not have_cluster_creds):
+            temp_share = copy.deepcopy(self.fake_share)
+            temp_share["id"] = fake.SHARE_ID5
             self.mock_allocate_container_from_snapshot.assert_called_once_with(
-                self.fake_share, fake.SNAPSHOT, fake.VSERVER1,
+                temp_share, fake.SNAPSHOT, fake.VSERVER1,
                 self.src_vserver_client, split=False, create_fpolicy=False)
             self.mock_allocate_container.assert_called_once_with(
                 self.fake_share, fake.VSERVER2,
@@ -970,19 +987,24 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
                 self.src_vserver_client, split=True)
             state = self.library.STATE_SPLITTING_VOLUME_CLONE
 
-        self.temp_src_share['aggregate'] = ([fake.POOL_NAME] if is_flexgroup
+            self.temp_src_share['aggregate'] = ([fake.POOL_NAME]
-                                            else fake.POOL_NAME)
+                                                if is_flexgroup
-        self.temp_src_share['internal_state'] = state
+                                                else fake.POOL_NAME)
-        self.temp_src_share['status'] = constants.STATUS_ACTIVE
+            self.temp_src_share['internal_state'] = state
-        str_temp_src_share = json.dumps(self.temp_src_share)
+            self.temp_src_share['status'] = constants.STATUS_ACTIVE
-        self.mock_storage_update.assert_called_once_with(
+            str_temp_src_share = json.dumps(self.temp_src_share)
-            self.fake_share['id'], {
+            self.mock_storage_update.assert_called_once_with(
-                'source_share': str_temp_src_share
+                self.fake_share['id'], {
-            })
+                    'source_share': str_temp_src_share
-        expected_return = {'status': constants.STATUS_CREATING_FROM_SNAPSHOT}
+                })
-        self.assertEqual(expected_return, result)
+            expected_return = {'status':
+                               constants.STATUS_CREATING_FROM_SNAPSHOT}
+            self.assertEqual(expected_return, result)
 
-    def test_create_share_from_snapshot_another_host_driver_error(self):
+    @ddt.data(True, False)
+    def test_create_share_from_snapshot_another_host_driver_error(
+            self, have_cluster_creds):
+        self.library._have_cluster_creds = have_cluster_creds
         self._setup_mocks_for_create_share_from_snapshot(
             allocate_attr=mock.Mock(side_effect=exception.NetAppException))
         mock_delete_snapmirror = self.mock_object(
@@ -1005,14 +1027,29 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
         self.mock_dm_backend.assert_called_once_with(self.parent_share)
         self.mock_dm_get_src_client.assert_called_once_with(
             fake.BACKEND_NAME, vserver_name=fake.VSERVER1)
-        self.mock_get_src_cluster.assert_called_once()
         self.mock_get_vserver.assert_called_once_with(self.fake_share_server)
-        self.mock_get_dest_cluster.assert_called_once()
+        if have_cluster_creds:
-        self.mock_allocate_container_from_snapshot.assert_called_once_with(
+            self.mock_get_dest_cluster.assert_called_once()
-            self.fake_share, fake.SNAPSHOT, fake.VSERVER1,
+            self.mock_get_src_cluster.assert_called_once()
-            self.src_vserver_client, split=True)
+        else:
-        mock_delete_snapmirror.assert_called_once_with(self.temp_src_share,
+            self.mock_get_dest_cluster.assert_not_called()
-                                                       self.fake_share)
+            self.mock_get_src_cluster.assert_not_called()
+
+        if have_cluster_creds:
+            self.mock_allocate_container_from_snapshot.assert_called_once_with(
+                self.fake_share, fake.SNAPSHOT, fake.VSERVER1,
+                self.src_vserver_client, split=True)
+        else:
+            self.mock_generate_uuid.assert_called_once()
+            temp_share = copy.deepcopy(self.fake_share)
+            temp_share["id"] = fake.SHARE_ID5
+            self.mock_allocate_container_from_snapshot.assert_called_once_with(
+                temp_share, fake.SNAPSHOT, fake.VSERVER1,
+                self.src_vserver_client, split=False, create_fpolicy=False)
+
+        mock_delete_snapmirror.assert_called_once_with(
+            self.temp_src_share, self.fake_share)
+
         mock_delete_share.assert_called_once_with(
             self.temp_src_share, fake.VSERVER1, self.src_vserver_client,
             remove_export=False)

manila/tests/share/drivers/netapp/dataontap/fakes.py

@@ -51,6 +51,7 @@ SHARE_ID = '7cf7c200-d3af-4e05-b87e-9167c95dfcad'
 SHARE_ID2 = 'b51c5a31-aa5b-4254-9ee8-7d39fa4c8c38'
 SHARE_ID3 = '1379991d-037b-4897-bf3a-81b4aac72eff'
 SHARE_ID4 = '1cb41aad-fd9b-4964-8059-646f69de925e'
+SHARE_ID5 = '2cb41aad-fd9b-4964-8059-646f69de925f'
 SHARE_INSTANCE_ID = 'd24e7257-124e-4fb6-b05b-d384f660bc85'
 PARENT_SHARE_ID = '585c3935-2aa9-437c-8bad-5abae1076555'
 SNAPSHOT_ID = 'de4c9050-e2f9-4ce1-ade4-5ed0c9f26451'

releasenotes/notes/fix_create_from_snapshot-5d8e470b46aac67d.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Netapp driver: Fix Netapp driver create from snapshot accross pools using
+    SVM scoped account.  For more details please refer to
+    `launchpad bug #1922512 <https://bugs.launchpad.net/manila/+bug/1922512>`_