Merge "NetApp cDOT driver configurable clone split"

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

@@ -1226,7 +1226,7 @@ def create_volume(self, aggregate_name, volume_name, size_gb,
                       thin_provisioned=False, snapshot_policy=None,
                       language=None, dedup_enabled=False,
                       compression_enabled=False, max_files=None,
-                      snapshot_reserve=None, volume_type='rw'):
+                      snapshot_reserve=None, volume_type='rw', **options):
 
         """Creates a volume."""
         api_args = {
@@ -1380,7 +1380,7 @@ def set_volume_name(self, volume_name, new_volume_name):
     def manage_volume(self, aggregate_name, volume_name,
                       thin_provisioned=False, snapshot_policy=None,
                       language=None, dedup_enabled=False,
-                      compression_enabled=False, max_files=None):
+                      compression_enabled=False, max_files=None, **options):
         """Update volume as needed to bring under management as a share."""
         api_args = {
             'query': {
@@ -1722,7 +1722,7 @@ def get_volume_to_manage(self, aggregate_name, volume_name):
 
     @na_utils.trace
     def create_volume_clone(self, volume_name, parent_volume_name,
-                            parent_snapshot_name=None):
+                            parent_snapshot_name=None, split=False, **options):
         """Clones a volume."""
         api_args = {
             'volume': volume_name,
@@ -1732,6 +1732,9 @@ def create_volume_clone(self, volume_name, parent_volume_name,
         }
         self.send_request('volume-clone-create', api_args)
 
+        if split:
+            self.split_volume_clone(volume_name)
+
     @na_utils.trace
     def split_volume_clone(self, volume_name):
         """Begins splitting a clone from its parent."""

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

@@ -62,6 +62,7 @@ class NetAppCmodeFileStorageLibrary(object):
         'netapp:thin_provisioned': 'thin_provisioned',
         'netapp:dedup': 'dedup_enabled',
         'netapp:compression': 'compression_enabled',
+        'netapp:split_clone_on_create': 'split',
     }
     STRING_QUALIFIED_EXTRA_SPECS_MAP = {
         'netapp:snapshot_policy': 'snapshot_policy',
@@ -390,10 +391,8 @@ def _allocate_container(self, share, vserver_client, replica=False):
             msg = _("Pool is not available in the share host field.")
             raise exception.InvalidHost(reason=msg)
 
-        extra_specs = share_types.get_extra_specs_from_share(share)
-        extra_specs = self._remap_standard_boolean_extra_specs(extra_specs)
-        self._check_extra_specs_validity(share, extra_specs)
-        provisioning_options = self._get_provisioning_options(extra_specs)
+        provisioning_options = self._get_provisioning_options_for_share(share)
+
         if replica:
             # If this volume is intended to be a replication destination,
             # create it as the 'data-protection' type
@@ -527,6 +526,20 @@ def _get_string_provisioning_options(self, specs, string_specs_map):
         # provisioning methods from the client API library.
         return dict(zip(provisioning_args, provisioning_values))
 
+    @na_utils.trace
+    def _get_provisioning_options_for_share(self, share):
+        """Return provisioning options from a share.
+
+        Starting with a share, this method gets the extra specs, rationalizes
+        NetApp vs. standard extra spec values, ensures their validity, and
+        returns them in a form suitable for passing to various API client
+        methods.
+        """
+        extra_specs = share_types.get_extra_specs_from_share(share)
+        extra_specs = self._remap_standard_boolean_extra_specs(extra_specs)
+        self._check_extra_specs_validity(share, extra_specs)
+        return self._get_provisioning_options(extra_specs)
+
     @na_utils.trace
     def _get_provisioning_options(self, specs):
         """Return a merged result of string and binary provisioning options."""
@@ -567,9 +580,13 @@ def _allocate_container_from_snapshot(
             parent_snapshot_name = snapshot_name_func(self, snapshot['id'])
         else:
             parent_snapshot_name = snapshot['provider_location']
+
+        provisioning_options = self._get_provisioning_options_for_share(share)
+
         LOG.debug('Creating share from snapshot %s', snapshot['id'])
         vserver_client.create_volume_clone(share_name, parent_share_name,
-                                           parent_snapshot_name)
+                                           parent_snapshot_name,
+                                           **provisioning_options)
 
     @na_utils.trace
     def _share_exists(self, share_name, vserver_client):

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

@@ -3014,6 +3014,7 @@ def test_get_volume_to_manage_not_found(self):
     def test_create_volume_clone(self):
 
         self.mock_object(self.client, 'send_request')
+        self.mock_object(self.client, 'split_volume_clone')
 
         self.client.create_volume_clone(fake.SHARE_NAME,
                                         fake.PARENT_SHARE_NAME,
@@ -3028,6 +3029,33 @@ def test_create_volume_clone(self):
 
         self.client.send_request.assert_has_calls([
             mock.call('volume-clone-create', volume_clone_create_args)])
+        self.assertFalse(self.client.split_volume_clone.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, 'split_volume_clone')
+
+        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.split_volume_clone.assert_called_once_with(
+                fake.SHARE_NAME)
+        else:
+            self.assertFalse(self.client.split_volume_clone.called)
 
     @ddt.data(None,
               mock.Mock(side_effect=netapp_api.NaApiError(

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

@@ -597,14 +597,9 @@ def test_allocate_container(self):
             return_value=fake.SHARE_NAME))
         self.mock_object(share_utils, 'extract_host', mock.Mock(
             return_value=fake.POOL_NAME))
-        self.mock_object(share_types, 'get_extra_specs_from_share',
-                         mock.Mock(return_value=fake.EXTRA_SPEC))
-        mock_remap_standard_boolean_extra_specs = self.mock_object(
-            self.library, '_remap_standard_boolean_extra_specs',
-            mock.Mock(return_value=fake.EXTRA_SPEC))
-        self.mock_object(self.library, '_check_boolean_extra_specs_validity')
-        self.mock_object(self.library, '_get_boolean_provisioning_options',
-                         mock.Mock(return_value=fake.PROVISIONING_OPTIONS))
+        self.mock_object(
+            self.library, '_get_provisioning_options_for_share',
+            mock.Mock(return_value=copy.deepcopy(fake.PROVISIONING_OPTIONS)))
         vserver_client = mock.Mock()
 
         self.library._allocate_container(fake.EXTRA_SPEC_SHARE,
@@ -613,10 +608,8 @@ def test_allocate_container(self):
         vserver_client.create_volume.assert_called_once_with(
             fake.POOL_NAME, fake.SHARE_NAME, fake.SHARE['size'],
             thin_provisioned=True, snapshot_policy='default',
-            language='en-US', dedup_enabled=True,
+            language='en-US', dedup_enabled=True, split=True,
             compression_enabled=False, max_files=5000, snapshot_reserve=8)
-        mock_remap_standard_boolean_extra_specs.assert_called_once_with(
-            fake.EXTRA_SPEC)
 
     def test_remap_standard_boolean_extra_specs(self):
 
@@ -631,12 +624,9 @@ def test_allocate_container_as_replica(self):
             return_value=fake.SHARE_NAME))
         self.mock_object(share_utils, 'extract_host', mock.Mock(
             return_value=fake.POOL_NAME))
-        self.mock_object(share_types, 'get_extra_specs_from_share',
-                         mock.Mock(return_value=fake.EXTRA_SPEC))
-
-        self.mock_object(self.library, '_check_boolean_extra_specs_validity')
-        self.mock_object(self.library, '_get_boolean_provisioning_options',
-                         mock.Mock(return_value=fake.PROVISIONING_OPTIONS))
+        self.mock_object(
+            self.library, '_get_provisioning_options_for_share',
+            mock.Mock(return_value=copy.deepcopy(fake.PROVISIONING_OPTIONS)))
         vserver_client = mock.Mock()
 
         self.library._allocate_container(fake.EXTRA_SPEC_SHARE,
@@ -645,7 +635,7 @@ def test_allocate_container_as_replica(self):
         vserver_client.create_volume.assert_called_once_with(
             fake.POOL_NAME, fake.SHARE_NAME, fake.SHARE['size'],
             thin_provisioned=True, snapshot_policy='default',
-            language='en-US', dedup_enabled=True,
+            language='en-US', dedup_enabled=True, split=True,
             compression_enabled=False, max_files=5000,
             snapshot_reserve=8, volume_type='dp')
 
@@ -729,6 +719,32 @@ def test_check_extra_specs_validity_invalid_combination(self):
             fake.EXTRA_SPEC_SHARE, fake.INVALID_EXTRA_SPEC_COMBO,
             list(self.library.BOOLEAN_QUALIFIED_EXTRA_SPECS_MAP))
 
+    def test_get_provisioning_options_for_share(self):
+
+        mock_get_extra_specs_from_share = self.mock_object(
+            share_types, 'get_extra_specs_from_share',
+            mock.Mock(return_value=fake.EXTRA_SPEC))
+        mock_remap_standard_boolean_extra_specs = self.mock_object(
+            self.library, '_remap_standard_boolean_extra_specs',
+            mock.Mock(return_value=fake.EXTRA_SPEC))
+        mock_check_extra_specs_validity = self.mock_object(
+            self.library, '_check_extra_specs_validity')
+        mock_get_provisioning_options = self.mock_object(
+            self.library, '_get_provisioning_options',
+            mock.Mock(return_value=fake.PROVISIONING_OPTIONS))
+
+        result = self.library._get_provisioning_options_for_share(
+            fake.EXTRA_SPEC_SHARE)
+
+        self.assertEqual(fake.PROVISIONING_OPTIONS, result)
+        mock_get_extra_specs_from_share.assert_called_once_with(
+            fake.EXTRA_SPEC_SHARE)
+        mock_remap_standard_boolean_extra_specs.assert_called_once_with(
+            fake.EXTRA_SPEC)
+        mock_check_extra_specs_validity.assert_called_once_with(
+            fake.EXTRA_SPEC_SHARE, fake.EXTRA_SPEC)
+        mock_get_provisioning_options.assert_called_once_with(fake.EXTRA_SPEC)
+
     def test_get_provisioning_options(self):
         result = self.library._get_provisioning_options(fake.EXTRA_SPEC)
 
@@ -752,6 +768,7 @@ def test_get_provisioning_options_implicit_false(self):
             'thin_provisioned': False,
             'compression_enabled': False,
             'dedup_enabled': False,
+            'split': False,
         }
 
         self.assertEqual(expected, result)
@@ -775,6 +792,7 @@ def test_get_boolean_provisioning_options_implicit_false(self):
             'thin_provisioned': False,
             'dedup_enabled': False,
             'compression_enabled': False,
+            'split': False,
         }
 
         result = self.library._get_boolean_provisioning_options(
@@ -846,23 +864,31 @@ def test_check_aggregate_extra_specs_validity_no_match(self):
                           fake.AGGREGATES[1],
                           fake.EXTRA_SPEC)
 
-    def test_allocate_container_from_snapshot(self):
+    @ddt.data(None, 'fake_location')
+    def test_allocate_container_from_snapshot(self, provider_location):
 
+        self.mock_object(
+            self.library, '_get_provisioning_options_for_share',
+            mock.Mock(return_value=copy.deepcopy(fake.PROVISIONING_OPTIONS)))
         vserver_client = mock.Mock()
 
+        fake_snapshot = copy.deepcopy(fake.SNAPSHOT)
+        fake_snapshot['provider_location'] = provider_location
+
         self.library._allocate_container_from_snapshot(fake.SHARE,
-                                                       fake.SNAPSHOT,
+                                                       fake_snapshot,
                                                        vserver_client)
 
         share_name = self.library._get_backend_share_name(fake.SHARE['id'])
         parent_share_name = self.library._get_backend_share_name(
             fake.SNAPSHOT['share_id'])
         parent_snapshot_name = self.library._get_backend_snapshot_name(
-            fake.SNAPSHOT['id'])
+            fake.SNAPSHOT['id']) if not provider_location else 'fake_location'
         vserver_client.create_volume_clone.assert_called_once_with(
-            share_name,
-            parent_share_name,
-            parent_snapshot_name)
+            share_name, parent_share_name, parent_snapshot_name,
+            thin_provisioned=True, snapshot_policy='default',
+            language='en-US', dedup_enabled=True, split=True,
+            compression_enabled=False, max_files=5000)
 
     def test_share_exists(self):
 

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

@@ -109,6 +109,7 @@
     'netapp:dedup': 'True',
     'netapp:compression': 'false',
     'netapp:max_files': 5000,
+    'netapp:split_clone_on_create': 'true',
     'netapp_disk_type': 'FCAL',
     'netapp_raid_type': 'raid4',
 }
@@ -120,12 +121,14 @@
     'dedup_enabled': True,
     'compression_enabled': False,
     'max_files': 5000,
+    'split': True,
 }
 
 PROVISIONING_OPTIONS_BOOLEAN = {
     'thin_provisioned': True,
     'dedup_enabled': False,
     'compression_enabled': False,
+    'split': False,
 }
 
 PROVISIONING_OPTIONS_BOOLEAN_THIN_PROVISIONED_TRUE = {
@@ -135,6 +138,7 @@
     'dedup_enabled': False,
     'compression_enabled': False,
     'max_files': None,
+    'split': False,
 }
 
 PROVISIONING_OPTIONS_STRING = {

releasenotes/notes/netapp-cdot-clone-split-control-a68b5fc80f1fc368.yaml

@@ -0,0 +1,7 @@
+---
+features:
+  - NetApp cDOT driver now supports a scoped extra-spec
+    ``netapp:split_clone_on_create`` to be used in share types when creating
+    shares (NetApp FlexClone) from snapshots. If this extra-spec is not
+    included, or set to ``false``, the cDOT driver will perform the clone-split
+    only if/when the parent snapshot is being deleted.