quota: Add 'quota show --usage' option

Provide an more sane way to get usage information for a particular
project's quotas. This requires using the 'Lister' command type since
the 'ShowOne' command type only allows for simple key-value pair output.

We also add a note indicating that the '<project>' argument is optional.

Change-Id: Ic7342cf08f024cc690049414c5eef5b9a7594677
Signed-off-by: Stephen Finucane <sfinucan@redhat.com>

Author: stephenfin

doc/source/cli/data/cinder.csv

@@ -94,7 +94,7 @@ quota-defaults,quota show --default,Lists default quotas for a tenant.
 quota-delete,quota delete --volume,Delete the quotas for a tenant.
 quota-show,quota show,Lists quotas for a tenant.
 quota-update,quota set,Updates quotas for a tenant.
-quota-usage,quota list --detail,Lists quota usage for a tenant.
+quota-usage,quota show --usage,Lists quota usage for a tenant.
 rate-limits,limits show --rate,Lists rate limits for a user.
 readonly-mode-update,volume set --read-only-mode | --read-write-mode,Updates volume read-only access-mode flag.
 rename,volume set --name,Renames a volume.

openstackclient/common/quota.py

@@ -233,19 +233,26 @@ class ListQuota(command.Lister):
 
     def get_parser(self, prog_name):
         parser = super().get_parser(prog_name)
+        # TODO(stephenfin): Remove in OSC 8.0
         parser.add_argument(
             '--project',
             metavar='<project>',
-            help=_('List quotas for this project <project> (name or ID)'),
+            help=_(
+                "**Deprecated** List quotas for this project <project> "
+                "(name or ID). "
+                "Use 'quota show' instead."
+            ),
         )
-        # TODO(stephenfin): This doesn't belong here. We should put it into the
-        # 'quota show' command and deprecate this.
+        # TODO(stephenfin): Remove in OSC 8.0
         parser.add_argument(
             '--detail',
             dest='detail',
             action='store_true',
             default=False,
-            help=_('Show details about quotas usage'),
+            help=_(
+                "**Deprecated** Show details about quotas usage. "
+                "Use 'quota show --usage' instead."
+            ),
         )
         option = parser.add_mutually_exclusive_group(required=True)
         option.add_argument(
@@ -332,6 +339,19 @@ def _get_detailed_quotas(self, parsed_args):
         )
 
     def take_action(self, parsed_args):
+        if parsed_args.detail:
+            msg = _(
+                "The --detail option has been deprecated. "
+                "Use 'openstack quota show --usage' instead."
+            )
+            self.log.warning(msg)
+        elif parsed_args.project:  # elif to avoid being too noisy
+            msg = _(
+                "The --project option has been deprecated. "
+                "Use 'openstack quota show' instead."
+            )
+            self.log.warning(msg)
+
         result = []
         project_ids = []
         if parsed_args.project is None:
@@ -678,7 +698,7 @@ def take_action(self, parsed_args):
                     **network_kwargs)
 
 
-class ShowQuota(command.ShowOne):
+class ShowQuota(command.Lister):
     _description = _(
         "Show quotas for project or class. "
         "Specify ``--os-compute-api-version 2.50`` or higher to see "
@@ -692,7 +712,10 @@ def get_parser(self, prog_name):
             'project',
             metavar='<project/class>',
             nargs='?',
-            help=_('Show quotas for this project or class (name or ID)'),
+            help=_(
+                'Show quotas for this project or class (name or ID) '
+                '(defaults to current project)'
+            ),
         )
         type_group = parser.add_mutually_exclusive_group()
         type_group.add_argument(
@@ -709,6 +732,13 @@ def get_parser(self, prog_name):
             default=False,
             help=_('Show default quotas for <project>'),
         )
+        type_group.add_argument(
+            '--usage',
+            dest='usage',
+            action='store_true',
+            default=False,
+            help=_('Show details about quotas usage'),
+        )
         return parser
 
     def take_action(self, parsed_args):
@@ -726,18 +756,21 @@ def take_action(self, parsed_args):
         compute_quota_info = get_compute_quotas(
             self.app,
             project,
+            detail=parsed_args.usage,
             quota_class=parsed_args.quota_class,
             default=parsed_args.default,
         )
         volume_quota_info = get_volume_quotas(
             self.app,
             project,
+            detail=parsed_args.usage,
             quota_class=parsed_args.quota_class,
             default=parsed_args.default,
         )
         network_quota_info = get_network_quotas(
             self.app,
             project,
+            detail=parsed_args.usage,
             quota_class=parsed_args.quota_class,
             default=parsed_args.default,
         )
@@ -762,20 +795,46 @@ def take_action(self, parsed_args):
                 info[v] = info[k]
                 info.pop(k)
 
+        # Remove the 'id' field since it's not very useful
+        if 'id' in info:
+            del info['id']
+
         # Remove the 'location' field for resources from openstacksdk
         if 'location' in info:
             del info['location']
 
-        # Handle class or project ID specially as they only appear in output
-        if parsed_args.quota_class:
-            info.pop('id', None)
-        elif 'id' in info:
-            info['project'] = info.pop('id')
-            if 'project_id' in info:
-                del info['project_id']
-            info['project_name'] = project_info['name']
-
-        return zip(*sorted(info.items()))
+        if not parsed_args.usage:
+            result = [
+                {'resource': k, 'limit': v} for k, v in info.items()
+            ]
+        else:
+            result = [
+                {'resource': k, **v} for k, v in info.items()
+            ]
+
+        columns = (
+            'resource',
+            'limit',
+        )
+        column_headers = (
+            'Resource',
+            'Limit',
+        )
+
+        if parsed_args.usage:
+            columns += (
+                'in_use',
+                'reserved',
+            )
+            column_headers += (
+                'In Use',
+                'Reserved',
+            )
+
+        return (
+            column_headers,
+            (utils.get_dict_properties(s, columns) for s in result),
+        )
 
 
 class DeleteQuota(command.Command):

openstackclient/tests/functional/common/test_quota.py

@@ -114,6 +114,7 @@ def test_quota_set_project(self):
         cmd_output = json.loads(self.openstack(
             'quota show -f json ' + self.PROJECT_NAME
         ))
+        cmd_output = {x['Resource']: x['Limit'] for x in cmd_output}
         self.assertIsNotNone(cmd_output)
         self.assertEqual(
             31,
@@ -136,6 +137,7 @@ def test_quota_set_project(self):
         self.assertIsNotNone(cmd_output)
         # We don't necessarily know the default quotas, we're checking the
         # returned attributes
+        cmd_output = {x['Resource']: x['Limit'] for x in cmd_output}
         self.assertTrue(cmd_output["cores"] >= 0)
         self.assertTrue(cmd_output["backups"] >= 0)
         if self.haz_network:
@@ -150,6 +152,7 @@ def test_quota_set_class(self):
             'quota show -f json --class default'
         ))
         self.assertIsNotNone(cmd_output)
+        cmd_output = {x['Resource']: x['Limit'] for x in cmd_output}
         self.assertEqual(
             33,
             cmd_output["key-pairs"],
@@ -166,6 +169,7 @@ def test_quota_set_class(self):
         self.assertIsNotNone(cmd_output)
         # We don't necessarily know the default quotas, we're checking the
         # returned attributes
+        cmd_output = {x['Resource']: x['Limit'] for x in cmd_output}
         self.assertTrue(cmd_output["key-pairs"] >= 0)
         self.assertTrue(cmd_output["snapshots"] >= 0)
 

openstackclient/tests/unit/common/test_quota.py

@@ -1094,17 +1094,20 @@ def test_quota_show(self):
         self.cmd.take_action(parsed_args)
 
         self.compute_quotas_mock.get.assert_called_once_with(
-            self.projects[0].id, detail=False
+            self.projects[0].id,
+            detail=False,
         )
         self.volume_quotas_mock.get.assert_called_once_with(
-            self.projects[0].id, usage=False
+            self.projects[0].id,
+            usage=False,
         )
         self.network.get_quota.assert_called_once_with(
-            self.projects[0].id, details=False
+            self.projects[0].id,
+            details=False,
         )
         self.assertNotCalled(self.network.get_quota_default)
 
-    def test_quota_show_with_default(self):
+    def test_quota_show__with_default(self):
         arglist = [
             '--default',
             self.projects[0].name,
@@ -1128,30 +1131,67 @@ def test_quota_show_with_default(self):
         )
         self.assertNotCalled(self.network.get_quota)
 
-    def test_quota_show_with_class(self):
+    def test_quota_show__with_class(self):
         arglist = [
             '--class',
-            self.projects[0].name,
+            'default',
         ]
         verifylist = [
             ('quota_class', True),
-            ('project', self.projects[0].name),
+            ('project', 'default'),  # project is actually a class here
         ]
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         self.cmd.take_action(parsed_args)
 
-        self.compute_quotas_class_mock.get.assert_called_once_with(
+        self.compute_quotas_class_mock.get.assert_called_once_with('default')
+        self.volume_quotas_class_mock.get.assert_called_once_with('default')
+        # neutron doesn't have the concept of quota classes
+        self.assertNotCalled(self.network.get_quota)
+        self.assertNotCalled(self.network.get_quota_default)
+
+    def test_quota_show__with_usage(self):
+        # update mocks to return detailed quota instead
+        self.compute_quota = \
+            compute_fakes.FakeQuota.create_one_comp_detailed_quota()
+        self.compute_quotas_mock.get.return_value = self.compute_quota
+        self.volume_quota = \
+            volume_fakes.FakeQuota.create_one_detailed_quota()
+        self.volume_quotas_mock.get.return_value = self.volume_quota
+        self.network.get_quota.return_value = \
+            network_fakes.FakeQuota.create_one_net_detailed_quota()
+
+        arglist = [
+            '--usage',
             self.projects[0].name,
+        ]
+        verifylist = [
+            ('usage', True),
+            ('project', self.projects[0].name),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
+
+        self.cmd.take_action(parsed_args)
+
+        self.compute_quotas_mock.get.assert_called_once_with(
+            self.projects[0].id,
+            detail=True,
         )
-        self.volume_quotas_class_mock.get.assert_called_once_with(
-            self.projects[0].name,
+        self.volume_quotas_mock.get.assert_called_once_with(
+            self.projects[0].id,
+            usage=True,
+        )
+        self.network.get_quota.assert_called_once_with(
+            self.projects[0].id,
+            details=True,
         )
-        self.assertNotCalled(self.network.get_quota)
-        self.assertNotCalled(self.network.get_quota_default)
 
-    def test_quota_show_no_project(self):
-        parsed_args = self.check_parser(self.cmd, [], [])
+    def test_quota_show__no_project(self):
+        arglist = []
+        verifylist = [
+            ('project', None),
+        ]
+        parsed_args = self.check_parser(self.cmd, arglist, verifylist)
 
         self.cmd.take_action(parsed_args)
 

releasenotes/notes/quota-show-usage-option-19b1f59fb5f3498f.yaml

@@ -0,0 +1,18 @@
+---
+features:
+  - |
+    The ``quota show`` command now supports a ``--usage`` option. When
+    provided, this will result in the command returning usage information for
+    each quota. This replaces the ``quota list --detail`` command which is now
+    deprecated for removal.
+deprecations:
+  - |
+    The ``--detail`` option for the ``quota list`` command has been deprecated
+    for removal. When used without the ``--detail`` option, the ``quota list``
+    command returned quota information for multiple projects yet when used with
+    this option it only returned (detailed) quota information for a single
+    project. This detailed quota information is now available via the
+    ``quota show --usage`` command.
+  - |
+    The ``--project`` option for the ``quota list`` command has been deprecated
+    for removal. Use the ``quota show`` command instead.