NetApp: fix delay clone split when creating share from snapshot (#124)

When creating share from snapshot, the clone-split job is triggered
right after volume is created on NetApp. The subsequent tasks on the
volume might fail because of "Volume busy" error. To prevent this issue,
we would like to delay the clone split job to the end. This problem was
addressed before, but the previos fix does not work as expected. Both
split variable in arguments and split field in provisioning_options
should be considered, and split argument should take precedence.

Change-Id: Ic401a919ab96b07866a883b5967962fecc892ff0
Closes-bug: #2029467

manila/share/drivers/netapp/dataontap/client/client_cmode.py

@@ -3141,7 +3141,7 @@ class NetAppCmodeClient(client_base.NetAppBaseClient):
 
     @na_utils.trace
     def create_volume_clone(self, volume_name, parent_volume_name,
-                            parent_snapshot_name=None, split=False,
+                            parent_snapshot_name=None,
                             qos_policy_group=None,
                             adaptive_qos_policy_group=None, **options):
         """Clones a volume."""
@@ -3157,9 +3157,6 @@ class NetAppCmodeClient(client_base.NetAppBaseClient):
 
         self.send_request('volume-clone-create', api_args)
 
-        if split:
-            self.volume_clone_split_start(volume_name)
-
         if adaptive_qos_policy_group is not None:
             self.set_qos_adaptive_policy_group_for_volume(
                 volume_name, adaptive_qos_policy_group)

manila/share/drivers/netapp/dataontap/client/client_cmode_rest.py

@@ -3200,7 +3200,7 @@ class NetAppRestClient(object):
 
     @na_utils.trace
     def create_volume_clone(self, volume_name, parent_volume_name,
-                            parent_snapshot_name=None, split=False,
+                            parent_snapshot_name=None,
                             qos_policy_group=None,
                             adaptive_qos_policy_group=None,
                             **options):
@@ -3227,9 +3227,6 @@ class NetAppRestClient(object):
             }
             self.send_request(f'/storage/volumes/{uuid}', 'patch', body=body)
 
-        if split:
-            self.volume_clone_split_start(volume_name)
-
         if adaptive_qos_policy_group is not None:
             self.set_qos_adaptive_policy_group_for_volume(
                 volume_name, adaptive_qos_policy_group)

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

@@ -1615,6 +1615,10 @@ class NetAppCmodeFileStorageLibrary(object):
 
         hide_snapdir = provisioning_options.pop('hide_snapdir')
 
+        # split in args takes precedence over split in provisioning_options
+        if split is None:
+            split = provisioning_options.pop('split')
+
         LOG.debug('Creating share from snapshot %s', snapshot['id'])
         vserver_client.create_volume_clone(
             share_name, parent_share_name, parent_snapshot_name,
@@ -1637,7 +1641,7 @@ class NetAppCmodeFileStorageLibrary(object):
                                                **provisioning_options)
 
         # split at the end: not be blocked by a busy volume
-        if split is not None:
+        if split:
             vserver_client.volume_clone_split_start(share_name)
 
     @na_utils.trace

manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py

@@ -4630,7 +4630,6 @@ class NetAppClientCmodeTestCase(test.TestCase):
                                  adaptive_qos_policy_group_name):
         self.client.features.add_feature('ADAPTIVE_QOS')
         self.mock_object(self.client, 'send_request')
-        self.mock_object(self.client, 'volume_clone_split_start')
         set_qos_adapt_mock = self.mock_object(
             self.client,
             'set_qos_adaptive_policy_group_for_volume')
@@ -4656,35 +4655,6 @@ class NetAppClientCmodeTestCase(test.TestCase):
             set_qos_adapt_mock.assert_called_once_with(
                 fake.SHARE_NAME, fake.ADAPTIVE_QOS_POLICY_GROUP_NAME
             )
-        self.client.send_request.assert_has_calls([
-            mock.call('volume-clone-create', volume_clone_create_args)])
-        self.assertFalse(self.client.volume_clone_split_start.called)
-
-    @ddt.data(True, False)
-    def test_create_volume_clone_split(self, split):
-
-        self.mock_object(self.client, 'send_request')
-        self.mock_object(self.client, 'volume_clone_split_start')
-
-        self.client.create_volume_clone(fake.SHARE_NAME,
-                                        fake.PARENT_SHARE_NAME,
-                                        fake.PARENT_SNAPSHOT_NAME,
-                                        split=split)
-
-        volume_clone_create_args = {
-            'volume': fake.SHARE_NAME,
-            'parent-volume': fake.PARENT_SHARE_NAME,
-            'parent-snapshot': fake.PARENT_SNAPSHOT_NAME,
-            'junction-path': '/%s' % fake.SHARE_NAME
-        }
-
-        self.client.send_request.assert_has_calls([
-            mock.call('volume-clone-create', volume_clone_create_args)])
-        if split:
-            self.client.volume_clone_split_start.assert_called_once_with(
-                fake.SHARE_NAME)
-        else:
-            self.assertFalse(self.client.volume_clone_split_start.called)
 
     @ddt.data(None,
               mock.Mock(side_effect=netapp_api.NaApiError(

manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode_rest.py

@@ -3283,11 +3283,7 @@ class NetAppRestCmodeClientTestCase(test.TestCase):
             fake.PARENT_SNAPSHOT_NAME,
             split=split)
 
-        if split:
+        self.assertFalse(self.client.volume_clone_split_start.called)
-            self.client.volume_clone_split_start.assert_called_once_with(
-                fake.SHARE_NAME)
-        else:
-            self.assertFalse(self.client.volume_clone_split_start.called)
 
         self.client.send_request.assert_called_once_with(
             '/storage/volumes', 'post', body=body)

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

@@ -2081,6 +2081,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
             fake_snapshot,
             vserver,
             vserver_client,
+            split=split,
             create_fpolicy=create_fpolicy)
 
         share_name = self.library._get_backend_share_name(
@@ -2113,6 +2114,15 @@ class NetAppFileStorageLibraryTestCase(test.TestCase):
         else:
             mock_create_fpolicy.assert_not_called()
 
+        if split is None:
+            vserver_client.volume_clone_split_start.assert_called_once_with(
+                fake.SHARE_INSTANCE_NAME)
+        if split:
+            vserver_client.volume_clone_split_start.assert_called_once_with(
+                fake.SHARE_INSTANCE_NAME)
+        if split is False:
+            vserver_client.volume_clone_split_start.assert_not_called()
+
     def test_share_exists(self):
 
         vserver_client = mock.Mock()

releasenotes/notes/netapp-delay-clone-split-after-share-creation-fbae159d988fe2a0.yaml

@@ -0,0 +1,9 @@
+---
+fixes:
+  - |
+    NetApp driver `bug #2029467
+    <https://bugs.launchpad.net/manila/+bug/2029467>`_:
+    When creating a share from a snapshot, the clone split operation is delayed
+    until the share creation is complete. This ensures that the share creation
+    operation is not blocked by the clone split operation.
+