Microversion 2.50 - fix quota class sets resource usage

This adds support for the 2.50 microversion which does the following:

* Adds the server_groups and server_groups_members resources to the output
  for the 'nova quota-class-show' and 'nova quota-class-update' CLIs.
* Removes the ability to show or update network-related resource quota class
  values, specifically floating_ips, fixed_ips, security_groups and
  security_group_members.
* Defines explicit kwargs for the update() method in the python API binding.

This also fixes a problem where the 'nova quota-class-update' CLI was
incorrectly capped at the 2.35 microversion for updating network-related
resources. That was true for the os-quota-sets API which is tenant-specific,
but not for the os-quota-class-sets API which is global.

Functional tests are added for the 2.1 and 2.50 microversion behavior for
both commands.

Part of blueprint fix-quota-classes-api

Change-Id: I2531f9094d92e1b9ed36ab03bc43ae1be5290790

7 files changed, 295 insertions, 5 deletions

diff --git a/novaclient/__init__.py b/novaclient/__init__.py
index 768ad119..0ce0f7c4 100644
--- a/novaclient/__init__.py
+++ b/novaclient/__init__.py
@@ -25,4 +25,4 @@ API_MIN_VERSION = api_versions.APIVersion("2.1")
 # when client supported the max version, and bumped sequentially, otherwise
 # the client may break due to server side new version may include some
 # backward incompatible change.
-API_MAX_VERSION = api_versions.APIVersion("2.49")
+API_MAX_VERSION = api_versions.APIVersion("2.50")
diff --git a/novaclient/tests/functional/v2/test_quota_classes.py b/novaclient/tests/functional/v2/test_quota_classes.py
new file mode 100644
index 00000000..399f5396
--- /dev/null
+++ b/novaclient/tests/functional/v2/test_quota_classes.py
@@ -0,0 +1,133 @@
+# Copyright 2017 Huawei Technologies Co.,LTD.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from tempest.lib import exceptions
+
+from novaclient.tests.functional import base
+
+
+class TestQuotaClassesNovaClient(base.ClientTestBase):
+    """Nova quota classes functional tests for the v2.1 microversion."""
+
+    COMPUTE_API_VERSION = '2.1'
+
+    # The list of quota class resources we expect in the output table.
+    _included_resources = ['instances', 'cores', 'ram',
+                           'floating_ips', 'fixed_ips', 'metadata_items',
+                           'injected_files', 'injected_file_content_bytes',
+                           'injected_file_path_bytes', 'key_pairs',
+                           'security_groups', 'security_group_rules']
+
+    # The list of quota class resources we do not expect in the output table.
+    _excluded_resources = ['server_groups', 'server_group_members']
+
+    # Any resources that are not shown but can be updated. For example, before
+    # microversion 2.50 you can update server_groups and server_groups_members
+    # quota class values but they are not shown in the GET response.
+    _extra_update_resources = _excluded_resources
+
+    # The list of resources which are blocked from being updated.
+    _blocked_update_resources = []
+
+    def _get_quota_class_name(self):
+        """Returns a fake quota class name specific to this test class."""
+        return 'fake-class-%s' % self.COMPUTE_API_VERSION.replace('.', '-')
+
+    def _verify_qouta_class_show_output(self, output, expected_values):
+        # Assert that the expected key/value pairs are in the output table
+        for quota_name in self._included_resources:
+            # First make sure the resource is actually in expected quota.
+            self.assertIn(quota_name, expected_values)
+            expected_value = expected_values[quota_name]
+            actual_value = self._get_value_from_the_table(output, quota_name)
+            self.assertEqual(expected_value, actual_value)
+
+        # Now make sure anything that we don't expect in the output table is
+        # actually not showing up.
+        for quota_name in self._excluded_resources:
+            # ValueError is raised when the key isn't found in the table.
+            self.assertRaises(ValueError,
+                              self._get_value_from_the_table,
+                              output, quota_name)
+
+    def test_quota_class_show(self):
+        """Tests showing quota class values for a fake non-existing quota
+        class. The API will return the defaults if the quota class does not
+        actually exist. We use a fake class to avoid any interaction with the
+        real default quota class values.
+        """
+        default_quota_class_set = self.client.quota_classes.get('default')
+        default_values = {
+            quota_name: str(getattr(default_quota_class_set, quota_name))
+            for quota_name in self._included_resources
+        }
+        output = self.nova('quota-class-show %s' %
+                           self._get_quota_class_name())
+        self._verify_qouta_class_show_output(output, default_values)
+
+    def test_quota_class_update(self):
+        """Tests updating a fake quota class. The way this works in the API
+        is that if the quota class is not found, it is created. So in this
+        test we can use a fake quota class with fake values and they will all
+        get set. We don't use the default quota class because it is global
+        and we don't want to interfere with other tests.
+        """
+        class_name = self._get_quota_class_name()
+        params = [class_name]
+        expected_values = {}
+        for quota_name in (
+                self._included_resources + self._extra_update_resources):
+            params.append("--%s 99" % quota_name.replace("_", "-"))
+            expected_values[quota_name] = '99'
+
+        # Note that the quota-class-update CLI doesn't actually output any
+        # information from the response.
+        self.nova("quota-class-update", params=" ".join(params))
+        # Assert the results using the quota-class-show output.
+        output = self.nova('quota-class-show %s' % class_name)
+        self._verify_qouta_class_show_output(output, expected_values)
+
+        # Assert that attempting to update resources that are blocked will
+        # result in a failure.
+        for quota_name in self._blocked_update_resources:
+            self.assertRaises(
+                exceptions.CommandFailed,
+                self.nova, "quota-class-update %s --%s 99" %
+                           (class_name, quota_name.replace("_", "-")))
+
+
+class TestQuotasNovaClient2_50(TestQuotaClassesNovaClient):
+    """Nova quota classes functional tests for the v2.50 microversion."""
+
+    COMPUTE_API_VERSION = '2.50'
+
+    # The 2.50 microversion added the server_groups and server_group_members
+    # to the response, and filtered out floating_ips, fixed_ips,
+    # security_groups and security_group_members, similar to the 2.36
+    # microversion in the os-qouta-sets API.
+    _included_resources = ['instances', 'cores', 'ram', 'metadata_items',
+                           'injected_files', 'injected_file_content_bytes',
+                           'injected_file_path_bytes', 'key_pairs',
+                           'server_groups', 'server_group_members']
+
+    # The list of quota class resources we do not expect in the output table.
+    _excluded_resources = ['floating_ips', 'fixed_ips',
+                           'security_groups', 'security_group_rules']
+
+    # In 2.50, server_groups and server_group_members can be both updated
+    # in a PUT request and shown in a GET response.
+    _extra_update_resources = []
+
+    # In 2.50, you can't update the network-related resources.
+    _blocked_update_resources = _excluded_resources
diff --git a/novaclient/tests/unit/v2/fakes.py b/novaclient/tests/unit/v2/fakes.py
index c2eb36bd..1ebf1e3e 100644
--- a/novaclient/tests/unit/v2/fakes.py
+++ b/novaclient/tests/unit/v2/fakes.py
@@ -1289,6 +1289,20 @@ class FakeSessionClient(base_client.SessionClient):
     #
 
     def get_os_quota_class_sets_test(self, **kw):
+        if self.api_version >= api_versions.APIVersion('2.50'):
+            return (200, FAKE_RESPONSE_HEADERS, {
+                'quota_class_set': {
+                    'id': 'test',
+                    'metadata_items': 1,
+                    'injected_file_content_bytes': 1,
+                    'injected_file_path_bytes': 1,
+                    'ram': 1,
+                    'instances': 1,
+                    'injected_files': 1,
+                    'cores': 1,
+                    'key_pairs': 1,
+                    'server_groups': 1,
+                    'server_group_members': 1}})
         return (200, FAKE_RESPONSE_HEADERS, {
             'quota_class_set': {
                 'id': 'test',
@@ -1297,6 +1311,7 @@ class FakeSessionClient(base_client.SessionClient):
                 'injected_file_path_bytes': 1,
                 'ram': 1,
                 'floating_ips': 1,
+                'fixed_ips': -1,
                 'instances': 1,
                 'injected_files': 1,
                 'cores': 1,
@@ -1306,6 +1321,19 @@ class FakeSessionClient(base_client.SessionClient):
 
     def put_os_quota_class_sets_test(self, body, **kw):
         assert list(body) == ['quota_class_set']
+        if self.api_version >= api_versions.APIVersion('2.50'):
+            return (200, {}, {
+                'quota_class_set': {
+                    'metadata_items': 1,
+                    'injected_file_content_bytes': 1,
+                    'injected_file_path_bytes': 1,
+                    'ram': 1,
+                    'instances': 1,
+                    'injected_files': 1,
+                    'cores': 1,
+                    'key_pairs': 1,
+                    'server_groups': 1,
+                    'server_group_members': 1}})
         return (200, {}, {
             'quota_class_set': {
                 'metadata_items': 1,
@@ -1313,6 +1341,7 @@ class FakeSessionClient(base_client.SessionClient):
                 'injected_file_path_bytes': 1,
                 'ram': 1,
                 'floating_ips': 1,
+                'fixed_ips': -1,
                 'instances': 1,
                 'injected_files': 1,
                 'cores': 1,
diff --git a/novaclient/tests/unit/v2/test_quota_classes.py b/novaclient/tests/unit/v2/test_quota_classes.py
index a1da954d..e9d9ad75 100644
--- a/novaclient/tests/unit/v2/test_quota_classes.py
+++ b/novaclient/tests/unit/v2/test_quota_classes.py
@@ -28,12 +28,14 @@ class QuotaClassSetsTest(utils.TestCase):
         q = self.cs.quota_classes.get(class_name)
         self.assert_request_id(q, fakes.FAKE_REQUEST_ID_LIST)
         self.cs.assert_called('GET', '/os-quota-class-sets/%s' % class_name)
+        return q
 
     def test_update_quota(self):
         q = self.cs.quota_classes.get('test')
         self.assert_request_id(q, fakes.FAKE_REQUEST_ID_LIST)
         q.update(cores=2)
         self.cs.assert_called('PUT', '/os-quota-class-sets/test')
+        return q
 
     def test_refresh_quota(self):
         q = self.cs.quota_classes.get('test')
@@ -43,3 +45,53 @@ class QuotaClassSetsTest(utils.TestCase):
         self.assertNotEqual(q.cores, q2.cores)
         q2.get()
         self.assertEqual(q.cores, q2.cores)
+
+
+class QuotaClassSetsTest2_50(QuotaClassSetsTest):
+    """Tests the quota classes API binding using the 2.50 microversion."""
+    def setUp(self):
+        super(QuotaClassSetsTest2_50, self).setUp()
+        self.cs = fakes.FakeClient(api_versions.APIVersion("2.50"))
+
+    def test_class_quotas_get(self):
+        """Tests that network-related resources aren't in a 2.50 response
+        and server group related resources are in the response.
+        """
+        q = super(QuotaClassSetsTest2_50, self).test_class_quotas_get()
+        for invalid_resource in ('floating_ips', 'fixed_ips', 'networks',
+                                 'security_groups', 'security_group_rules'):
+            self.assertFalse(hasattr(q, invalid_resource),
+                             '%s should not be in %s' % (invalid_resource, q))
+        # Also make sure server_groups and server_group_members are in the
+        # response.
+        for valid_resource in ('server_groups', 'server_group_members'):
+            self.assertTrue(hasattr(q, valid_resource),
+                            '%s should be in %s' % (invalid_resource, q))
+
+    def test_update_quota(self):
+        """Tests that network-related resources aren't in a 2.50 response
+        and server group related resources are in the response.
+        """
+        q = super(QuotaClassSetsTest2_50, self).test_update_quota()
+        for invalid_resource in ('floating_ips', 'fixed_ips', 'networks',
+                                 'security_groups', 'security_group_rules'):
+            self.assertFalse(hasattr(q, invalid_resource),
+                             '%s should not be in %s' % (invalid_resource, q))
+        # Also make sure server_groups and server_group_members are in the
+        # response.
+        for valid_resource in ('server_groups', 'server_group_members'):
+            self.assertTrue(hasattr(q, valid_resource),
+                            '%s should be in %s' % (invalid_resource, q))
+
+    def test_update_quota_invalid_resources(self):
+        """Tests trying to update quota class values for invalid resources.
+
+        This will fail with TypeError because the network-related resource
+        kwargs aren't defined.
+        """
+        q = self.cs.quota_classes.get('test')
+        self.assertRaises(TypeError, q.update, floating_ips=1)
+        self.assertRaises(TypeError, q.update, fixed_ips=1)
+        self.assertRaises(TypeError, q.update, security_groups=1)
+        self.assertRaises(TypeError, q.update, security_group_rules=1)
+        self.assertRaises(TypeError, q.update, networks=1)
diff --git a/novaclient/v2/quota_classes.py b/novaclient/v2/quota_classes.py
index 4a38a970..eae5bfde 100644
--- a/novaclient/v2/quota_classes.py
+++ b/novaclient/v2/quota_classes.py
@@ -13,6 +13,7 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+from novaclient import api_versions
 from novaclient import base
 
 
@@ -32,6 +33,10 @@ class QuotaClassSetManager(base.Manager):
     def _update_body(self, **kwargs):
         return {'quota_class_set': kwargs}
 
+    # NOTE(mriedem): Before 2.50 the resources you could update was just a
+    # kwargs dict and not validated on the client-side, only on the API server
+    # side.
+    @api_versions.wraps("2.0", "2.49")
     def update(self, class_name, **kwargs):
         body = self._update_body(**kwargs)
 
@@ -42,3 +47,37 @@ class QuotaClassSetManager(base.Manager):
         return self._update('/os-quota-class-sets/%s' % (class_name),
                             body,
                             'quota_class_set')
+
+    # NOTE(mriedem): 2.50 does strict validation of the resources you can
+    # specify since the network-related resources are blocked in 2.50.
+    @api_versions.wraps("2.50")
+    def update(self, class_name, instances=None, cores=None, ram=None,
+               metadata_items=None, injected_files=None,
+               injected_file_content_bytes=None, injected_file_path_bytes=None,
+               key_pairs=None, server_groups=None, server_group_members=None):
+        resources = {}
+        if instances is not None:
+            resources['instances'] = instances
+        if cores is not None:
+            resources['cores'] = cores
+        if ram is not None:
+            resources['ram'] = ram
+        if metadata_items is not None:
+            resources['metadata_items'] = metadata_items
+        if injected_files is not None:
+            resources['injected_files'] = injected_files
+        if injected_file_content_bytes is not None:
+            resources['injected_file_content_bytes'] = (
+                injected_file_content_bytes)
+        if injected_file_path_bytes is not None:
+            resources['injected_file_path_bytes'] = injected_file_path_bytes
+        if key_pairs is not None:
+            resources['key_pairs'] = key_pairs
+        if server_groups is not None:
+            resources['server_groups'] = server_groups
+        if server_group_members is not None:
+            resources['server_group_members'] = server_group_members
+
+        body = {'quota_class_set': resources}
+        return self._update('/os-quota-class-sets/%s' % class_name, body,
+                            'quota_class_set')
diff --git a/novaclient/v2/shell.py b/novaclient/v2/shell.py
index 72d71e2b..e390c26e 100644
--- a/novaclient/v2/shell.py
+++ b/novaclient/v2/shell.py
@@ -3827,6 +3827,11 @@ def do_ssh(cs, args):
     os.system(cmd)
 
 
+# NOTE(mriedem): In the 2.50 microversion, the os-quota-class-sets API
+# will return the server_groups and server_group_members, but no longer
+# return floating_ips, fixed_ips, security_groups or security_group_members
+# as those are deprecated as networking service proxies and/or because
+# nova-network is deprecated. Similar to the 2.36 microversion.
 _quota_resources = ['instances', 'cores', 'ram',
                     'floating_ips', 'fixed_ips', 'metadata_items',
                     'injected_files', 'injected_file_content_bytes',
@@ -4137,7 +4142,7 @@ def do_quota_class_show(cs, args):
     _quota_show(cs.quota_classes.get(args.class_name))
 
 
-@api_versions.wraps("2.0", "2.35")
+@api_versions.wraps("2.0", "2.49")
 @utils.arg(
     'class_name',
     metavar='<class>',
@@ -4233,9 +4238,9 @@ def do_quota_class_update(cs, args):
     _quota_update(cs.quota_classes, args.class_name, args)
 
 
-# 2.36 does not support updating quota for floating IPs, fixed IPs, security
-# groups or security group rules.
-@api_versions.wraps("2.36")
+# 2.50 does not support updating quota class values for floating IPs,
+# fixed IPs, security groups or security group rules.
+@api_versions.wraps("2.50")
 @utils.arg(
     'class_name',
     metavar='<class>',
diff --git a/releasenotes/notes/microversion-v2_50-4f484658d66d01aa.yaml b/releasenotes/notes/microversion-v2_50-4f484658d66d01aa.yaml
new file mode 100644
index 00000000..3ca1908c
--- /dev/null
+++ b/releasenotes/notes/microversion-v2_50-4f484658d66d01aa.yaml
@@ -0,0 +1,32 @@
+---
+fixes:
+  - |
+    Adds support for the ``2.50`` microversion which fixes the
+    ``nova quota-class-show`` and ``nova quota-class-update`` commands in the
+    following ways:
+
+    * The ``server_groups`` and ``server_group_members`` quota resources will
+      now be shown in the output table for ``nova quota-class-show``.
+    * The ``floating_ips``, ``fixed_ips``, ``security_groups`` and
+      ``security_group_rules`` quota resources will no longer be able to
+      be updated using ``nova quota-class-update`` nor will they be shown in
+      the output of ``nova quota-class-show``. Use python-openstackclient or
+      python-neutronclient to work with quotas for network resources.
+
+    In addition, the ``nova quota-class-update`` CLI was previously incorrectly
+    limiting the ability to update quota class values for ``floating_ips``,
+    ``fixed_ips``, ``security_groups`` and ``security_group_rules`` based on
+    the 2.36 microversion. That has been changed to limit based on the ``2.50``
+    microversion.
+upgrade:
+  - |
+    The ``novaclient.v2.quota_classes.QuotaClassSetManager.update`` method
+    now defines specific kwargs starting with microversion ``2.50`` since
+    updating network-related resource quota class values is not supported on
+    the server with microversion ``2.50``. The list of excluded resources is:
+
+    - ``fixed_ips``
+    - ``floating_ips``
+    - ``networks``
+    - ``security_groups``
+    - ``security_group_rules``