From a2ff2dbc5bfb2247adf9b5fc53591c01221a1b52 Mon Sep 17 00:00:00 2001 From: Chuan Date: Mon, 5 Sep 2022 13:36:29 +0200 Subject: [PATCH] ccloud: Disable cross-volume-deduplication to prevent metadata from filling volume (#52) This PR patches the NetApp driver to disable cross volume deduplication when necessary. When disabled, together with set sis state to inline-only, we prevent the deduplication metadata from filling up the Volume. When not disabled, the metadata could take capacity up to 4% of the application's data in Volume. NetApp is working on a change to OnTap, so that the metadata are stored seperated. When it is delivered, we can remove this patch. If some parent share has disabled cross-volume-deduplication, but the efficiency policy is 'auto'. Check policy name and set cross_volume_disabled only when policy name is 'inline-only'. Otherwise don't change the cross-volume-deduplication and policy name. Change-Id: I3e6291c536633b4bdef8e831f370ab2043bc8d7c --- manila/share/api.py | 3 + .../netapp/dataontap/client/client_cmode.py | 69 ++++++++++++++----- .../netapp/dataontap/cluster_mode/lib_base.py | 37 +++++++++- manila/share/manager.py | 5 ++ .../drivers/netapp/dataontap/client/fakes.py | 3 + .../dataontap/client/test_client_cmode.py | 26 +++++-- .../dataontap/cluster_mode/test_lib_base.py | 47 +++++++++---- 7 files changed, 153 insertions(+), 37 deletions(-) diff --git a/manila/share/api.py b/manila/share/api.py index 32d92a9ac5..023dd698d3 100644 --- a/manila/share/api.py +++ b/manila/share/api.py @@ -572,6 +572,9 @@ def create_instance(self, context, share, share_network_id=None, az_request_multiple_subnet_support_map=( az_request_multiple_subnet_support_map))) + # SAPCC add project_domain_name in request_spec and pass to driver + request_spec['project_domain_name'] = context.project_domain_name + if share_group_snapshot_member: # Inherit properties from the share_group_snapshot_member member_share_instance = share_group_snapshot_member[ diff --git a/manila/share/drivers/netapp/dataontap/client/client_cmode.py b/manila/share/drivers/netapp/dataontap/client/client_cmode.py index 0d884b6741..2a15dee598 100644 --- a/manila/share/drivers/netapp/dataontap/client/client_cmode.py +++ b/manila/share/drivers/netapp/dataontap/client/client_cmode.py @@ -2233,7 +2233,8 @@ def create_volume(self, aggregate_name, volume_name, size_gb, compression_enabled=False, max_files=None, snapshot_reserve=None, volume_type='rw', comment='', qos_policy_group=None, adaptive_qos_policy_group=None, - encrypt=None, logical_space_reporting=None, **options): + encrypt=None, logical_space_reporting=None, + cross_dedup_disabled=False, **options): """Creates a volume.""" if adaptive_qos_policy_group and not self.features.ADAPTIVE_QOS: msg = 'Adaptive QoS not supported on this backend ONTAP version.' @@ -2250,9 +2251,9 @@ def create_volume(self, aggregate_name, volume_name, size_gb, self.send_request('volume-create', api_args) - self.update_volume_efficiency_attributes(volume_name, - dedup_enabled, - compression_enabled) + self.update_volume_efficiency_attributes( + volume_name, dedup_enabled, compression_enabled, + cross_dedup_disabled=cross_dedup_disabled) if volume_type != 'dp': if options.get('max_files_multiplier') is not None: @@ -2379,6 +2380,11 @@ def update_volume_snapshot_policy(self, volume_name, snapshot_policy): } self.send_request('volume-modify-iter', api_args) + @na_utils.trace + def set_sis_config(self, volume_name, api_args): + api_args.update({'path': '/vol/%s' % volume_name}) + self.send_request('sis-set-config', api_args) + @na_utils.trace @manila_utils.retry(retry_param=exception.NetAppException, interval=3, @@ -2455,7 +2461,10 @@ def get_volume_efficiency_status(self, volume_name): 'desired-attributes': { 'sis-status-info': { 'state': None, + 'policy': None, 'is-compression-enabled': None, + 'is-cross-volume-background-dedupe-enabled': None, + 'is-cross-volume-inline-dedupe-enabled': None, }, }, } @@ -2470,11 +2479,23 @@ def get_volume_efficiency_status(self, volume_name): LOG.error(msg, volume_name) sis_status_info = netapp_api.NaElement('none') + # SAPCC disable cross volume dedup when both cross-volume-inline + # and cross-volume-background dedupe are 'false' return { - 'dedupe': True if 'enabled' == sis_status_info.get_child_content( - 'state') else False, - 'compression': True if 'true' == sis_status_info.get_child_content( - 'is-compression-enabled') else False, + 'dedupe': + True if 'enabled' == sis_status_info.get_child_content('state') + else False, + 'compression': + True if ('true' == sis_status_info.get_child_content( + 'is-compression-enabled')) else False, + 'policy': + sis_status_info.get_child_content('policy'), + 'cross_dedup_disabled': + True if + ('false' == sis_status_info.get_child_content( + 'is-cross-volume-inline-dedupe-enabled') + and 'false' == sis_status_info.get_child_content( + 'is-cross-volume-background-dedupe-enabled')) else False, } @na_utils.trace @@ -2719,7 +2740,8 @@ def modify_volume(self, aggregate_name, volume_name, qos_policy_group=None, hide_snapdir=None, autosize_attributes=None, comment=None, replica=False, adaptive_qos_policy_group=None, - logical_space_reporting=None, **options): + logical_space_reporting=None, cross_dedup_disabled=False, + **options): """Update backend volume for a share as necessary. :param aggregate_name: either a list or a string. List for aggregate @@ -2818,18 +2840,33 @@ def modify_volume(self, aggregate_name, volume_name, if not replica: # Efficiency options must be handled separately - self.update_volume_efficiency_attributes(volume_name, - dedup_enabled, - compression_enabled, - is_flexgroup=is_flexgroup) + self.update_volume_efficiency_attributes( + volume_name, dedup_enabled, compression_enabled, + cross_dedup_disabled=cross_dedup_disabled, + is_flexgroup=is_flexgroup) @na_utils.trace - def update_volume_efficiency_attributes(self, volume_name, dedup_enabled, - compression_enabled, - is_flexgroup=False): + def update_volume_efficiency_attributes( + self, volume_name, dedup_enabled, compression_enabled, + cross_dedup_disabled=False, is_flexgroup=False): """Update dedupe & compression attributes to match desired values.""" efficiency_status = self.get_volume_efficiency_status(volume_name) + # SAPCC Kepp deduplication metadata from filling up the volume + if cross_dedup_disabled: + if not efficiency_status['dedupe']: + self.enable_dedup(volume_name) + if not efficiency_status['compression']: + self.enable_compression(volume_name) + self.set_sis_config( + volume_name, { + 'policy-name': 'inline-only', + 'enable-cross-volume-background-dedupe': 'false', + 'enable-cross-volume-inline-dedupe': 'false', + 'enable-data-compaction': 'true', + }) + return + # cDOT compression requires dedup to be enabled dedup_enabled = dedup_enabled or compression_enabled 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 037544b5de..9ee265d844 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py @@ -739,6 +739,18 @@ def _get_logical_space_options(self, vserver_client, share_name): src_volume.get('is-space-reporting-logical') } + def _get_efficiency_options(self, vserver_client, share_name): + status = vserver_client.get_volume_efficiency_status(share_name) + cross_dedup_disabled = (status.get('policy') == 'inline-only' + and status.get('cross_dedup_disabled')) + provisioning_opts = { + 'dedup_enabled': status.get('dedupe'), + 'compression_enabled': status.get('compression'), + 'policy': None, + 'cross_dedup_disabled': cross_dedup_disabled, + } + return provisioning_opts + @na_utils.trace def create_share(self, context, share, share_server): """Creates new share.""" @@ -747,6 +759,10 @@ def create_share(self, context, share, share_server): # Force enabling logical-space-reporting on new Volumes. # Disable cross volume dedupe in neo projects. provisioning_options = {'logical_space_reporting': True} + if context.project_domain_name in ['neo']: + provisioning_options['dedup_enabled'] = True + provisioning_options['compression_enabled'] = True + provisioning_options['cross_dedup_disabled'] = True self._allocate_container(share, vserver, vserver_client, **provisioning_options) return self._create_export(share, share_server, vserver, @@ -770,9 +786,11 @@ def create_share_from_snapshot(self, context, share, snapshot, src_share_name = self._get_backend_share_name(snapshot['share_id']) logical_opts = self._get_logical_space_options( src_vserver_client, src_share_name) + effi_opts = self._get_efficiency_options(src_vserver_client, + src_share_name) self._allocate_container_from_snapshot( share, snapshot, src_vserver, src_vserver_client, - **logical_opts) + **logical_opts, **effi_opts) return self._create_export(share, share_server, src_vserver, src_vserver_client) @@ -854,6 +872,8 @@ def create_share_from_snapshot(self, context, share, snapshot, # SAPCC Get attributes from parent share logical_opts = self._get_logical_space_options(src_vserver_client, parent_share_name) + effi_opts = self._get_efficiency_options(src_vserver_client, + parent_share_name) try: # NOTE(felipe_rodrigues): no support to move volumes that are @@ -870,7 +890,7 @@ def create_share_from_snapshot(self, context, share, snapshot, # 2. Create a replica in destination host. self._allocate_container( dest_share, dest_vserver, dest_vserver_client, - replica=True, set_qos=False, **logical_opts) + replica=True, set_qos=False, **logical_opts, **effi_opts) # 3. Initialize snapmirror relationship with cloned share. src_share_instance['replica_state'] = ( constants.REPLICA_STATE_ACTIVE) @@ -889,7 +909,7 @@ def create_share_from_snapshot(self, context, share, snapshot, # vserver. self._allocate_container_from_snapshot( dest_share, snapshot, src_vserver, src_vserver_client, - split=True, **logical_opts) + split=True, **logical_opts, **effi_opts) # The split volume clone operation can take some time to be # concluded and we'll answer the call asynchronously. state = self.STATE_SPLITTING_VOLUME_CLONE @@ -1085,6 +1105,9 @@ def _create_from_snapshot_continue(self, share, share_server=None): provisioning_options.update( self._get_logical_space_options(src_vserver_client, share_name)) + provisioning_options.update( + self._get_efficiency_options(src_vserver_client, + share_name)) qos_policy_group_name = ( self._modify_or_create_qos_for_existing_share( share, extra_specs, dest_vserver, dest_vserver_client)) @@ -2548,8 +2571,11 @@ def update_share(self, share, share_comment=None, share_server=None): share, vserver, vserver_client=vserver_client, set_qos=False) # SAPCC Keep logical space reporting attributes while update share + # SAPCC Keep efficiency attributes while update share provisioning_options.update( self._get_logical_space_options(vserver_client, share_name)) + provisioning_options.update( + self._get_efficiency_options(vserver_client, share_name)) qos_policy_group_name = self._modify_or_create_qos_for_existing_share( share, extra_specs, vserver, vserver_client) @@ -2965,6 +2991,8 @@ def promote_replica(self, context, replica_list, replica, access_rules, logical_opts = self._get_logical_space_options( orig_active_vserver_client, orig_active_replica_name) is_logical_space_reporting = logical_opts['logical_space_reporting'] + effi_opts = self._get_efficiency_options(orig_active_vserver_client, + orig_active_replica_name) new_replica_list = [] @@ -3057,6 +3085,9 @@ def promote_replica(self, context, replica_list, replica, access_rules, new_active_replica['id']) new_active_vserver_client.update_volume_space_attributes( new_active_replica_name, is_logical_space_reporting) + if effi_opts['cross_dedup_disabled']: + new_active_vserver_client.update_volume_efficiency_attributes( + new_active_replica_name, True, True, cross_dedup_disabled=True) return new_replica_list diff --git a/manila/share/manager.py b/manila/share/manager.py index c4e882530a..73cfb67083 100644 --- a/manila/share/manager.py +++ b/manila/share/manager.py @@ -2227,6 +2227,11 @@ def create_share_instance(self, context, share_instance_id, "mandatory for protocol %s.") % share_instance.get('share_proto')) + # SAPCC Get project_domain_name from request_spec and set to context + if context.project_domain_name is None and request_spec: + context.project_domain_name = request_spec.get( + 'project_domain_name') + status = constants.STATUS_AVAILABLE try: if snapshot_ref: diff --git a/manila/tests/share/drivers/netapp/dataontap/client/fakes.py b/manila/tests/share/drivers/netapp/dataontap/client/fakes.py index dce328abff..bfe3008284 100644 --- a/manila/tests/share/drivers/netapp/dataontap/client/fakes.py +++ b/manila/tests/share/drivers/netapp/dataontap/client/fakes.py @@ -2468,6 +2468,9 @@ /vol/%(volume)s enabled %(vserver)s + auto + False + False 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 e5a20eba3f..e31d0b5fab 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 @@ -3207,7 +3207,8 @@ def test_create_volume(self, set_max_files): self.client.send_request.assert_called_with('volume-create', volume_create_args) (self.client.update_volume_efficiency_attributes. - assert_called_once_with(fake.SHARE_NAME, False, False)) + assert_called_once_with(fake.SHARE_NAME, False, False, + cross_dedup_disabled=False)) if set_max_files: self.client.set_volume_max_files.assert_called_once_with( fake.SHARE_NAME, fake.MAX_FILES) @@ -3610,13 +3611,21 @@ def test_get_volume_efficiency_status(self): 'sis-status-info': { 'state': None, 'is-compression-enabled': None, + 'policy': None, + 'is-cross-volume-background-dedupe-enabled': None, + 'is-cross-volume-inline-dedupe-enabled': None, }, }, } self.client.send_iter_request.assert_has_calls([ mock.call('sis-get-iter', sis_get_iter_args)]) - expected = {'dedupe': True, 'compression': True} + expected = { + 'dedupe': True, + 'compression': True, + 'cross_dedup_disabled': False, + 'policy': 'auto' + } self.assertDictEqual(expected, result) def test_get_volume_efficiency_status_not_found(self): @@ -3628,7 +3637,12 @@ def test_get_volume_efficiency_status_not_found(self): result = self.client.get_volume_efficiency_status(fake.SHARE_NAME) - expected = {'dedupe': False, 'compression': False} + expected = { + 'dedupe': False, + 'compression': False, + 'policy': None, + 'cross_dedup_disabled': False + } self.assertDictEqual(expected, result) def test_set_volume_max_files(self): @@ -3737,7 +3751,8 @@ def test_modify_volume_no_optional_args(self, is_flexgroup): self.client.send_request.assert_called_once_with( 'volume-modify-iter', volume_modify_iter_api_args) mock_update_volume_efficiency_attributes.assert_called_once_with( - fake.SHARE_NAME, False, False, is_flexgroup=is_flexgroup) + fake.SHARE_NAME, False, False, is_flexgroup=is_flexgroup, + cross_dedup_disabled=False) @ddt.data((fake.QOS_POLICY_GROUP_NAME, None), (None, fake.ADAPTIVE_QOS_POLICY_GROUP_NAME)) @@ -3815,7 +3830,8 @@ def test_modify_volume_all_optional_args(self, qos_group, self.client.send_request.assert_called_once_with( 'volume-modify-iter', volume_modify_iter_api_args) mock_update_volume_efficiency_attributes.assert_called_once_with( - fake.SHARE_NAME, True, False, is_flexgroup=False) + fake.SHARE_NAME, True, False, is_flexgroup=False, + cross_dedup_disabled=False) @ddt.data( {'existing': (True, True), 'desired': (True, True), 'fg': False}, 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 b02fa836ee..2f76f26d4b 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 @@ -818,6 +818,12 @@ def test_create_share_from_snapshot(self, share_group_id): self.mock_object( vserver_client, 'get_volume', mock.Mock(return_value={'is-space-reporting-logical': False})) + self.mock_object( + vserver_client, 'get_volume_efficiency_status', + mock.Mock(return_value={ + 'compression': True, + 'dedupe': True, + })) mock_allocate_container_from_snapshot = self.mock_object( self.library, '_allocate_container_from_snapshot') @@ -834,13 +840,10 @@ def test_create_share_from_snapshot(self, share_group_id): parent_share=share) mock_allocate_container_from_snapshot.assert_called_once_with( - share, - fake.SNAPSHOT, - fake.VSERVER1, - vserver_client, - logical_space_reporting=False) - mock_create_export.assert_called_once_with(share, - fake.SHARE_SERVER, + share, fake.SNAPSHOT, fake.VSERVER1, vserver_client, + compression_enabled=True, dedup_enabled=True, policy=None, + cross_dedup_disabled=False, logical_space_reporting=False) + mock_create_export.assert_called_once_with(share, fake.SHARE_SERVER, fake.VSERVER1, vserver_client) self.assertEqual('fake_export_location', result) @@ -914,6 +917,12 @@ def to_dict(self): self.mock_object( self.src_vserver_client, 'get_volume', mock.Mock(return_value={'is-space-reporting-logical': False})) + self.mock_object( + self.src_vserver_client, 'get_volume_efficiency_status', + mock.Mock(return_value={ + 'compression': True, + 'dedupe': True + })) # Parent share on MANILA_HOST_2 self.parent_share = copy.copy(fake.SHARE) @@ -971,8 +980,9 @@ def test_create_share_from_snapshot_another_host(self, dest_cluster, self.fake_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, - self.dest_vserver_client, replica=True, set_qos=False, + self.fake_share, fake.VSERVER2, self.dest_vserver_client, + replica=True, set_qos=False, compression_enabled=True, + dedup_enabled=True, policy=None, cross_dedup_disabled=False, logical_space_reporting=False) self.mock_dm_create_snapmirror.assert_called_once() self.temp_src_share['replica_state'] = ( @@ -981,7 +991,8 @@ def test_create_share_from_snapshot_another_host(self, dest_cluster, else: self.mock_allocate_container_from_snapshot.assert_called_once_with( self.fake_share, fake.SNAPSHOT, fake.VSERVER1, - self.src_vserver_client, split=True, + self.src_vserver_client, split=True, compression_enabled=True, + dedup_enabled=True, policy=None, cross_dedup_disabled=False, logical_space_reporting=False) state = self.library.STATE_SPLITTING_VOLUME_CLONE @@ -1025,7 +1036,9 @@ def test_create_share_from_snapshot_another_host_driver_error(self): self.mock_get_dest_cluster.assert_called_once() self.mock_allocate_container_from_snapshot.assert_called_once_with( self.fake_share, fake.SNAPSHOT, fake.VSERVER1, - self.src_vserver_client, split=True, logical_space_reporting=False) + self.src_vserver_client, split=True, compression_enabled=True, + dedup_enabled=True, policy=None, cross_dedup_disabled=False, + logical_space_reporting=False) mock_delete_snapmirror.assert_called_once_with(self.temp_src_share, self.fake_share) mock_delete_share.assert_called_once_with( @@ -1255,6 +1268,12 @@ def _setup_mocks_for_create_from_snapshot_continue( mock.Mock(return_value={ 'is-space-reporting-logical': True, })) + self.mock_get_volume_efficiency_status = self.mock_object( + self.src_vserver_client, 'get_volume_efficiency_status', + mock.Mock(return_value={ + 'dedupe': True, + 'compression': False, + })) @ddt.data(fake.MANILA_HOST_NAME, fake.MANILA_HOST_NAME_2) def test__create_from_snapshot_continue_state_splitting(self, src_host): @@ -1307,7 +1326,8 @@ def test__create_from_snapshot_continue_state_splitting(self, src_host): self.mock_modify_vol.assert_called_once_with( fake.POOL_NAME, fake.SHARE_NAME, **fake.PROVISIONING_OPTIONS_WITH_QOS, - logical_space_reporting=True) + logical_space_reporting=True, policy=None, + cross_dedup_disabled=False) self.mock_pvt_storage_delete.assert_called_once_with( fake.SHARE['id']) self.mock_create_export.assert_called_once_with( @@ -1415,7 +1435,8 @@ def test__create_from_snapshot_continue_state_snapmirror(self, self.mock_modify_vol.assert_called_once_with( fake.POOL_NAME, fake.SHARE_NAME, **fake.PROVISIONING_OPTIONS_WITH_QOS, - logical_space_reporting=True) + logical_space_reporting=True, policy=None, + cross_dedup_disabled=False) expect_result['status'] = constants.STATUS_AVAILABLE self.mock_pvt_storage_delete.assert_called_once_with( fake.SHARE['id'])