diff --git a/manila/share/drivers/netapp/dataontap/client/client_cmode.py b/manila/share/drivers/netapp/dataontap/client/client_cmode.py index a0293472b1..5396c9f4f1 100644 --- a/manila/share/drivers/netapp/dataontap/client/client_cmode.py +++ b/manila/share/drivers/netapp/dataontap/client/client_cmode.py @@ -1650,6 +1650,10 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): qos_policy_group=None, adaptive_qos_policy_group=None, encrypt=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.' + raise exception.NetAppException(msg) + api_args = { 'containing-aggr-name': aggregate_name, 'size': six.text_type(size_gb) + 'g', @@ -1920,8 +1924,13 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): language=None, dedup_enabled=False, compression_enabled=False, max_files=None, qos_policy_group=None, hide_snapdir=None, - autosize_attributes=None, **options): + autosize_attributes=None, + adaptive_qos_policy_group=None, **options): """Update backend volume for a share as necessary.""" + if adaptive_qos_policy_group and not self.features.ADAPTIVE_QOS: + msg = 'Adaptive QoS not supported on this backend ONTAP version.' + raise exception.NetAppException(msg) + api_args = { 'query': { 'volume-attributes': { @@ -1961,6 +1970,11 @@ class NetAppCmodeClient(client_base.NetAppBaseClient): 'volume-qos-attributes'] = { 'policy-group-name': qos_policy_group, } + if adaptive_qos_policy_group: + api_args['attributes']['volume-attributes'][ + 'volume-qos-attributes'] = { + 'adaptive-policy-group-name': adaptive_qos_policy_group, + } if hide_snapdir in (True, False): # Value of hide_snapdir needs to be inverted for ZAPI parameter api_args['attributes']['volume-attributes'][ 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 f2d0336b7c..627991e7c3 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_base.py @@ -550,6 +550,7 @@ class NetAppCmodeFileStorageLibrary(object): share, snapshot, src_vserver, src_vserver_client) return self._create_export(share, share_server, src_vserver, src_vserver_client) + parent_share_server = {} if parent_share['share_server'] is not None: # Get only the information needed by Data Motion @@ -1040,20 +1041,9 @@ class NetAppCmodeFileStorageLibrary(object): self._check_extra_specs_validity(share, extra_specs) provisioning_options = self._get_provisioning_options(extra_specs) qos_specs = self._get_normalized_qos_specs(extra_specs) - if (provisioning_options.get('adaptive_qos_policy_group') is not None - and qos_specs): - msg = _('Share cannot be provisioned with both qos_specs ' - '%(qos_specs_string)s and adaptive_qos_policy_group ' - '%(adaptive_qos_policy_group)s.') - qos_specs_string = "" - for key in qos_specs: - qos_specs_string += key + "=" + str(qos_specs[key]) + " " - msg_args = { - 'adaptive_qos_policy_group': - provisioning_options['adaptive_qos_policy_group'], - 'qos_specs_string': qos_specs_string - } - raise exception.NetAppException(msg % msg_args) + self.validate_provisioning_options_for_share(provisioning_options, + extra_specs=extra_specs, + qos_specs=qos_specs) if qos_specs and not replica: qos_policy_group = self._create_qos_policy_group( share, vserver, qos_specs, vserver_client) @@ -1075,6 +1065,41 @@ class NetAppCmodeFileStorageLibrary(object): return result + @na_utils.trace + def validate_provisioning_options_for_share(self, provisioning_options, + extra_specs=None, + qos_specs=None): + """Checks if provided provisioning options are valid.""" + adaptive_qos = provisioning_options.get('adaptive_qos_policy_group') + replication_type = (extra_specs.get('replication_type') + if extra_specs else None) + if adaptive_qos and qos_specs: + msg = _('Share cannot be provisioned with both qos_specs ' + '%(qos_specs_string)s and adaptive_qos_policy_group ' + '%(adaptive_qos_policy_group)s.') + qos_specs_string = "" + for key in qos_specs: + qos_specs_string += key + "=" + str(qos_specs[key]) + " " + msg_args = { + 'adaptive_qos_policy_group': + provisioning_options['adaptive_qos_policy_group'], + 'qos_specs_string': qos_specs_string + } + raise exception.NetAppException(msg % msg_args) + + if adaptive_qos and replication_type: + msg = _("The extra spec 'adaptive_qos_policy_group' is not " + "supported by share replication feature.") + raise exception.NetAppException(msg) + + # NOTE(dviroel): This validation will need to be updated if newer + # versions of ONTAP stop requiring cluster credentials to associate + # QoS to volumes. + if (adaptive_qos or qos_specs) and not self._have_cluster_creds: + msg = _('Share cannot be provisioned with QoS without having ' + 'cluster credentials.') + raise exception.NetAppException(msg) + def _get_nve_option(self, specs): if 'netapp_flexvol_encryption' in specs: nve = specs['netapp_flexvol_encryption'].lower() == 'true' @@ -1407,6 +1432,10 @@ class NetAppCmodeFileStorageLibrary(object): self._validate_volume_for_manage(volume, vserver_client) provisioning_options = self._get_provisioning_options(extra_specs) + qos_specs = self._get_normalized_qos_specs(extra_specs) + self.validate_provisioning_options_for_share(provisioning_options, + extra_specs=extra_specs, + qos_specs=qos_specs) debug_args = { 'share': share_name, @@ -2393,6 +2422,14 @@ class NetAppCmodeFileStorageLibrary(object): destination_share) self._check_extra_specs_validity( destination_share, extra_specs) + # NOTE(dviroel): Check if the destination share-type has valid + # provisioning options. + provisioning_options = self._get_provisioning_options( + extra_specs) + qos_specs = self._get_normalized_qos_specs(extra_specs) + self.validate_provisioning_options_for_share( + provisioning_options, extra_specs=extra_specs, + qos_specs=qos_specs) # NOTE (felipe_rodrigues): NetApp only can migrate within the # same server, so it does not need to check that the @@ -2411,6 +2448,10 @@ class NetAppCmodeFileStorageLibrary(object): share_volume = self._get_backend_share_name( source_share['id']) + # NOTE(dviroel): If source and destination vservers are + # compatible for volume move, the provisioning option + # 'adaptive_qos_policy_group' will also be supported since the + # share will remain in the same vserver. self._check_destination_vserver_for_vol_move( source_share, source_vserver, destination_share_server) diff --git a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py index 55784b1209..3eb832af88 100644 --- a/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py +++ b/manila/share/drivers/netapp/dataontap/cluster_mode/lib_multi_svm.py @@ -1318,3 +1318,17 @@ class NetAppCmodeMultiSVMFileStorageLibrary( # Modify volume to match extra specs dest_client.modify_volume(dest_aggregate, volume_name, **provisioning_options) + + def validate_provisioning_options_for_share(self, provisioning_options, + extra_specs=None, + qos_specs=None): + if provisioning_options.get('adaptive_qos_policy_group') is not None: + msg = _("The extra spec 'adaptive_qos_policy_group' is not " + "supported by backends configured with " + "'driver_handles_share_server' == True mode.") + raise exception.NetAppException(msg) + + (super(NetAppCmodeMultiSVMFileStorageLibrary, self) + .validate_provisioning_options_for_share(provisioning_options, + extra_specs=extra_specs, + qos_specs=qos_specs)) diff --git a/manila/tests/share/drivers/netapp/dataontap/client/fakes.py b/manila/tests/share/drivers/netapp/dataontap/client/fakes.py index d1bdc7f320..7652189938 100644 --- a/manila/tests/share/drivers/netapp/dataontap/client/fakes.py +++ b/manila/tests/share/drivers/netapp/dataontap/client/fakes.py @@ -74,6 +74,7 @@ DELETED_EXPORT_POLICIES = { } QOS_POLICY_GROUP_NAME = 'fake_qos_policy_group_name' QOS_MAX_THROUGHPUT = '5000B/s' +ADAPTIVE_QOS_POLICY_GROUP_NAME = 'fake_adaptive_qos_policy_group_name' VSERVER_TYPE_DEFAULT = 'default' VSERVER_TYPE_DP_DEST = 'dp_destination' VSERVER_OP_STATE_RUNNING = 'running' 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 4b864b0331..7b184d8b2d 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 @@ -2844,12 +2844,13 @@ class NetAppClientCmodeTestCase(test.TestCase): {'qos_policy_group_name': fake.QOS_POLICY_GROUP_NAME, 'adaptive_policy_group_name': None}, {'qos_policy_group_name': None, - 'adaptive_policy_group_name': fake.QOS_POLICY_GROUP_NAME}, + 'adaptive_policy_group_name': + fake.ADAPTIVE_QOS_POLICY_GROUP_NAME}, ) @ddt.unpack def test_create_volume_with_extra_specs(self, qos_policy_group_name, adaptive_policy_group_name): - + self.client.features.add_feature('ADAPTIVE_QOS') self.mock_object(self.client, 'set_volume_max_files') self.mock_object(self.client, 'enable_dedup') self.mock_object(self.client, 'enable_compression') @@ -3211,8 +3212,12 @@ class NetAppClientCmodeTestCase(test.TestCase): mock_update_volume_efficiency_attributes.assert_called_once_with( fake.SHARE_NAME, False, False) - def test_modify_volume_all_optional_args(self): - + @ddt.data((fake.QOS_POLICY_GROUP_NAME, None), + (None, fake.ADAPTIVE_QOS_POLICY_GROUP_NAME)) + @ddt.unpack + def test_modify_volume_all_optional_args(self, qos_group, + adaptive_qos_group): + self.client.features.add_feature('ADAPTIVE_QOS') self.mock_object(self.client, 'send_request') mock_update_volume_efficiency_attributes = self.mock_object( self.client, 'update_volume_efficiency_attributes') @@ -3226,7 +3231,8 @@ class NetAppClientCmodeTestCase(test.TestCase): dedup_enabled=True, compression_enabled=False, max_files=fake.MAX_FILES, - qos_policy_group=fake.QOS_POLICY_GROUP_NAME, + qos_policy_group=qos_group, + adaptive_qos_policy_group=adaptive_qos_group, autosize_attributes=fake.VOLUME_AUTOSIZE_ATTRS, hide_snapdir=True) @@ -3254,13 +3260,26 @@ class NetAppClientCmodeTestCase(test.TestCase): 'volume-space-attributes': { 'space-guarantee': 'none', }, - 'volume-qos-attributes': { - 'policy-group-name': fake.QOS_POLICY_GROUP_NAME, - }, 'volume-autosize-attributes': fake.VOLUME_AUTOSIZE_ATTRS, }, }, } + if qos_group: + qos_update = { + 'volume-qos-attributes': { + 'policy-group-name': qos_group, + }, + } + volume_modify_iter_api_args[ + 'attributes']['volume-attributes'].update(qos_update) + if adaptive_qos_group: + qos_update = { + 'volume-qos-attributes': { + 'adaptive-policy-group-name': adaptive_qos_group, + }, + } + volume_modify_iter_api_args[ + 'attributes']['volume-attributes'].update(qos_update) self.client.send_request.assert_called_once_with( 'volume-modify-iter', volume_modify_iter_api_args) @@ -3981,12 +4000,13 @@ class NetAppClientCmodeTestCase(test.TestCase): {'qos_policy_group_name': fake.QOS_POLICY_GROUP_NAME, 'adaptive_qos_policy_group_name': None}, {'qos_policy_group_name': None, - 'adaptive_qos_policy_group_name': fake.QOS_POLICY_GROUP_NAME}, + 'adaptive_qos_policy_group_name': + fake.ADAPTIVE_QOS_POLICY_GROUP_NAME}, ) @ddt.unpack def test_create_volume_clone(self, qos_policy_group_name, adaptive_qos_policy_group_name): - + self.client.features.add_feature('ADAPTIVE_QOS') self.mock_object(self.client, 'send_request') self.mock_object(self.client, 'split_volume_clone') set_qos_adapt_mock = self.mock_object( @@ -4012,7 +4032,7 @@ class NetAppClientCmodeTestCase(test.TestCase): {'qos-policy-group-name': fake.QOS_POLICY_GROUP_NAME}) if adaptive_qos_policy_group_name: set_qos_adapt_mock.assert_called_once_with( - fake.SHARE_NAME, fake.QOS_POLICY_GROUP_NAME + 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)]) 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 d278f6996f..707eb06d32 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 @@ -1409,6 +1409,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): qos = True if fake.QOS_EXTRA_SPEC in extra_specs else False vserver_client = mock.Mock() + self.library._have_cluster_creds = True mock_get_extra_specs_from_share = self.mock_object( share_types, 'get_extra_specs_from_share', mock.Mock(return_value=extra_specs)) @@ -2208,6 +2209,7 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): def test_manage_container_with_qos(self, qos): vserver_client = mock.Mock() + self.library._have_cluster_creds = True qos_policy_group_name = fake.QOS_POLICY_GROUP_NAME if qos else None extra_specs = fake.EXTRA_SPEC_WITH_QOS if qos else fake.EXTRA_SPEC provisioning_opts = self.library._get_provisioning_options(extra_specs) @@ -4960,6 +4962,34 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): mock_exception_log.assert_called_once() self.assertFalse(data_motion.get_backend_configuration.called) + def test_migration_check_compatibility_invalid_qos_configuration(self): + self.library._have_cluster_creds = True + self.mock_object(self.library, '_get_backend_share_name', + mock.Mock(return_value=fake.SHARE_NAME)) + mock_exception_log = self.mock_object(lib_base.LOG, 'exception') + self.mock_object(share_types, 'get_extra_specs_from_share') + self.mock_object(self.library, '_check_extra_specs_validity') + self.mock_object( + self.library, '_get_provisioning_options', + mock.Mock(return_value=fake.PROVISIONING_OPTS_WITH_ADAPT_QOS)) + self.mock_object(self.library, '_get_normalized_qos_specs', + mock.Mock(return_value=fake.QOS_NORMALIZED_SPEC)) + + migration_compatibility = self.library.migration_check_compatibility( + self.context, fake_share.fake_share_instance(), + fake_share.fake_share_instance(), share_server=fake.SHARE_SERVER, + destination_share_server=None) + + expected_compatibility = { + 'compatible': False, + 'writable': False, + 'nondisruptive': False, + 'preserve_metadata': False, + 'preserve_snapshots': False, + } + self.assertDictMatch(expected_compatibility, migration_compatibility) + mock_exception_log.assert_called_once() + def test_migration_check_compatibility_destination_not_configured(self): self.library._have_cluster_creds = True self.mock_object(self.library, '_get_backend_share_name', @@ -4974,6 +5004,10 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.mock_object(share_types, 'get_extra_specs_from_share') self.mock_object(self.library, '_check_extra_specs_validity') self.mock_object(self.library, '_check_aggregate_extra_specs_validity') + self.mock_object(self.library, '_get_provisioning_options') + self.mock_object(self.library, '_get_normalized_qos_specs') + self.mock_object(self.library, + 'validate_provisioning_options_for_share') mock_vserver_compatibility_check = self.mock_object( self.library, '_check_destination_vserver_for_vol_move') self.mock_object(self.library, '_get_dest_flexvol_encryption_value', @@ -5012,6 +5046,10 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.mock_object(self.library, '_check_aggregate_extra_specs_validity') self.mock_object(self.library, '_get_backend_share_name', mock.Mock(return_value=fake.SHARE_NAME)) + self.mock_object(self.library, '_get_provisioning_options') + self.mock_object(self.library, '_get_normalized_qos_specs') + self.mock_object(self.library, + 'validate_provisioning_options_for_share') self.mock_object(data_motion, 'get_backend_configuration') self.mock_object(self.library, '_get_vserver', mock.Mock(side_effect=side_effects)) @@ -5047,6 +5085,10 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.mock_object(self.library, '_get_backend_share_name', mock.Mock(return_value=fake.SHARE_NAME)) self.mock_object(data_motion, 'get_backend_configuration') + self.mock_object(self.library, '_get_provisioning_options') + self.mock_object(self.library, '_get_normalized_qos_specs') + self.mock_object(self.library, + 'validate_provisioning_options_for_share') mock_exception_log = self.mock_object(lib_base.LOG, 'exception') get_vserver_returns = [ (fake.VSERVER1, mock.Mock()), @@ -5084,6 +5126,10 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): self.mock_object(share_types, 'get_extra_specs_from_share') self.mock_object(self.library, '_check_extra_specs_validity') self.mock_object(self.library, '_check_aggregate_extra_specs_validity') + self.mock_object(self.library, '_get_provisioning_options') + self.mock_object(self.library, '_get_normalized_qos_specs') + self.mock_object(self.library, + 'validate_provisioning_options_for_share') self.mock_object(self.library, '_get_backend_share_name', mock.Mock(return_value=fake.SHARE_NAME)) mock_exception_log = self.mock_object(lib_base.LOG, 'exception') @@ -5136,6 +5182,10 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): mock_move_check = self.mock_object(self.client, 'check_volume_move') self.mock_object(self.library, '_get_dest_flexvol_encryption_value', mock.Mock(return_value=False)) + self.mock_object(self.library, '_get_provisioning_options') + self.mock_object(self.library, '_get_normalized_qos_specs') + self.mock_object(self.library, + 'validate_provisioning_options_for_share') migration_compatibility = self.library.migration_check_compatibility( self.context, fake_share.fake_share_instance(), @@ -5177,6 +5227,10 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): '_check_extra_specs_validity') self.mock_object(self.library, '_check_aggregate_extra_specs_validity') + self.mock_object(self.library, '_get_provisioning_options') + self.mock_object(self.library, '_get_normalized_qos_specs') + self.mock_object(self.library, + 'validate_provisioning_options_for_share') migration_compatibility = self.library.migration_check_compatibility( self.context, fake_share.fake_share_instance(), @@ -5941,3 +5995,30 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): result = self.library._check_capacity_compatibility(pools, thin, size) self.assertEqual(compatible, result) + + @ddt.data({'provisioning_opts': fake.PROVISIONING_OPTS_WITH_ADAPT_QOS, + 'qos_specs': {fake.QOS_NORMALIZED_SPEC: 3000}, + 'extra_specs': None, + 'cluster_credentials': True}, + {'provisioning_opts': fake.PROVISIONING_OPTS_WITH_ADAPT_QOS, + 'qos_specs': None, + 'extra_specs': fake.EXTRA_SPEC_WITH_REPLICATION, + 'cluster_credentials': True}, + {'provisioning_opts': fake.PROVISIONING_OPTIONS, + 'qos_specs': {fake.QOS_NORMALIZED_SPEC: 3000}, + 'extra_specs': None, + 'cluster_credentials': False}, + {'provisioning_opts': fake.PROVISIONING_OPTS_WITH_ADAPT_QOS, + 'qos_specs': None, + 'extra_specs': None, + 'cluster_credentials': False},) + @ddt.unpack + def test_validate_provisioning_options_for_share_invalid_params( + self, provisioning_opts, qos_specs, extra_specs, + cluster_credentials): + self.library._have_cluster_creds = cluster_credentials + + self.assertRaises(exception.NetAppException, + self.library.validate_provisioning_options_for_share, + provisioning_opts, extra_specs=extra_specs, + qos_specs=qos_specs) diff --git a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py index 718a85c872..1a75ada441 100644 --- a/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py +++ b/manila/tests/share/drivers/netapp/dataontap/cluster_mode/test_lib_multi_svm.py @@ -2544,3 +2544,22 @@ class NetAppFileStorageLibraryTestCase(test.TestCase): mock_get_vol_autosize_attrs.assert_called_once_with(fake_share_name) mock_modify_volume.assert_called_once_with( fake_aggregate, fake_share_name, **fake_provisioning_opts) + + def test_validate_provisioning_options_for_share(self): + mock_create_from_snap = self.mock_object( + lib_base.NetAppCmodeFileStorageLibrary, + 'validate_provisioning_options_for_share') + + self.library.validate_provisioning_options_for_share( + fake.PROVISIONING_OPTIONS, extra_specs=fake.EXTRA_SPEC, + qos_specs=fake.QOS_NORMALIZED_SPEC) + + mock_create_from_snap.assert_called_once_with( + fake.PROVISIONING_OPTIONS, extra_specs=fake.EXTRA_SPEC, + qos_specs=fake.QOS_NORMALIZED_SPEC) + + def test_validate_provisioning_options_for_share_aqos_not_supported(self): + self.assertRaises( + exception.NetAppException, + self.library.validate_provisioning_options_for_share, + fake.PROVISIONING_OPTS_WITH_ADAPT_QOS, qos_specs=None) diff --git a/manila/tests/share/drivers/netapp/dataontap/fakes.py b/manila/tests/share/drivers/netapp/dataontap/fakes.py index dd78147caa..e2e47d4e57 100644 --- a/manila/tests/share/drivers/netapp/dataontap/fakes.py +++ b/manila/tests/share/drivers/netapp/dataontap/fakes.py @@ -194,6 +194,11 @@ EXTRA_SPEC = { 'netapp:adaptive_qos_policy_group': None, } +EXTRA_SPEC_WITH_REPLICATION = copy.copy(EXTRA_SPEC) +EXTRA_SPEC_WITH_REPLICATION.update({ + 'replication_type': 'dr' +}) + NFS_CONFIG_DEFAULT = { 'tcp-max-xfer-size': 65536, 'udp-max-xfer-size': 32768, diff --git a/releasenotes/notes/netapp-add-support-for-adaptive-qos-d036238e7f29cf75.yaml b/releasenotes/notes/netapp-add-support-for-adaptive-qos-d036238e7f29cf75.yaml index 65ede3fd66..558ab2a13d 100644 --- a/releasenotes/notes/netapp-add-support-for-adaptive-qos-d036238e7f29cf75.yaml +++ b/releasenotes/notes/netapp-add-support-for-adaptive-qos-d036238e7f29cf75.yaml @@ -13,4 +13,6 @@ features: configuration in order to use QoS in clustered ONTAP. Other notes: - This only works for backends without share server management. - - This does not work for share replicas or share migration. + - This does not work for share replicas or can fail when creating share + from snapshot across backends, if the destination backend does not have + the pre-created "adaptive_qos_policy_group".