diff --git a/manila/share/drivers/netapp/dataontap/client/client_cmode.py b/manila/share/drivers/netapp/dataontap/client/client_cmode.py index 42631c2426..91c4180f10 100644 --- a/manila/share/drivers/netapp/dataontap/client/client_cmode.py +++ b/manila/share/drivers/netapp/dataontap/client/client_cmode.py @@ -1207,7 +1207,7 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): 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 = { @@ -1361,7 +1361,7 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): 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': { @@ -1703,7 +1703,7 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): @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, @@ -1713,6 +1713,9 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): } 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.""" 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 39b0736b51..de4993bfa8 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py +++ b/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 @@ class NetAppCmodeFileStorageLibrary(object): 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 @@ class NetAppCmodeFileStorageLibrary(object): # 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 @@ class NetAppCmodeFileStorageLibrary(object): 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): diff --git a/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py b/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py index 61dc515b44..c66bb2c908 100644 --- a/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py +++ b/manila/tests/share/drivers/netapp/dataontap/client/test_client_cmode.py @@ -2981,6 +2981,7 @@ class NetAppClientCmodeTestCase(test.TestCase): 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, @@ -2995,6 +2996,33 @@ class NetAppClientCmodeTestCase(test.TestCase): 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( 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 d8b2e3f84a..e433d07190 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 @@ -597,14 +597,9 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): 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 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): 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 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): 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 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): 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 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): 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 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): 'thin_provisioned': False, 'compression_enabled': False, 'dedup_enabled': False, + 'split': False, } self.assertEqual(expected, result) @@ -775,6 +792,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): 'thin_provisioned': False, 'dedup_enabled': False, 'compression_enabled': False, + 'split': False, } result = self.library._get_boolean_provisioning_options( @@ -846,23 +864,31 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): 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): diff --git a/manila/tests/share/drivers/netapp/dataontap/fakes.py b/manila/tests/share/drivers/netapp/dataontap/fakes.py index faf76acb1f..8bb8e29b25 100644 --- a/manila/tests/share/drivers/netapp/dataontap/fakes.py +++ b/manila/tests/share/drivers/netapp/dataontap/fakes.py @@ -109,6 +109,7 @@ EXTRA_SPEC = { '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 @@ PROVISIONING_OPTIONS = { '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 @@ PROVISIONING_OPTIONS_BOOLEAN_THIN_PROVISIONED_TRUE = { 'dedup_enabled': False, 'compression_enabled': False, 'max_files': None, + 'split': False, } PROVISIONING_OPTIONS_STRING = { diff --git a/releasenotes/notes/netapp-cdot-clone-split-control-a68b5fc80f1fc368.yaml b/releasenotes/notes/netapp-cdot-clone-split-control-a68b5fc80f1fc368.yaml new file mode 100644 index 0000000000..2fe7848dcb --- /dev/null +++ b/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. \ No newline at end of file