Merge "[NetApp] Improve Adaptive QoS functionality"

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'][

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
+        self.validate_provisioning_options_for_share(provisioning_options,
-                and qos_specs):
+                                                     extra_specs=extra_specs,
-            msg = _('Share cannot be provisioned with both qos_specs '
+                                                     qos_specs=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 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)
 

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))

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'

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)])

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)

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)

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,

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".