common: Migrate 'limits show' to SDK

This is done for both the compute and block storage services even though
the compute service hasn't supported rate limits since API v2.1 was
introduced [1].

[1] openstack/nova@ca4ec762804

Change-Id: Idd9f4a1c23952a6087f08c03ac8b5bebd5a0c86d
Co-authored-by: Stephen Finucane <stephenfin@redhat.com>
Depends-on: https://review.opendev.org/c/openstack/openstacksdk/+/918519

Author: rajathere

openstackclient/common/clientmanager.py

@@ -133,7 +133,17 @@ def is_volume_endpoint_enabled(self, volume_client):
         # NOTE(jcross): Cinder did some interesting things with their service
         #               name so we need to figure out which version to look
         #               for when calling is_service_available()
-        volume_version = volume_client.api_version.ver_major
+        endpoint_data = volume_client.get_endpoint_data()
+        # Not sure how endpoint data stores the api version for v2 API,
+        # for v3 it is a tuple (3, 0)
+        if endpoint_data.api_version and isinstance(
+            endpoint_data.api_version, tuple
+        ):
+            volume_version = endpoint_data.api_version[0]
+        else:
+            # Setting volume_version as 2 here if it doesn't satisfy the
+            # conditions for version 3
+            volume_version = 2
         if (
             self.is_service_available("volumev%s" % volume_version)
             is not False

openstackclient/common/limits.py

@@ -24,6 +24,31 @@
 from openstackclient.identity import common as identity_common
 
 
+def _format_absolute_limit(absolute_limits):
+    info = {}
+
+    for key in set(absolute_limits):
+        if key in ('id', 'name', 'location'):
+            continue
+
+        info[key] = absolute_limits[key]
+
+    return info
+
+
+def _format_rate_limit(rate_limits):
+    # flatten this:
+    #
+    #   {'uri': '<uri>', 'limit': [{'value': '<value>', ...], ...}
+    #
+    # to this:
+    #
+    #   {'uri': '<uri>', 'value': '<value>', ...}, ...}
+    return itertools.chain(
+        *[[{'uri': x['uri'], **y} for y in x['limit']] for x in rate_limits]
+    )
+
+
 class ShowLimits(command.Lister):
     _description = _("Show compute and block storage limits")
 
@@ -42,36 +67,42 @@ def get_parser(self, prog_name):
             dest="is_rate",
             action="store_true",
             default=False,
-            help=_("Show rate limits"),
+            help=_(
+                'Show rate limits. This is not supported by the compute '
+                'service since the 12.0.0 (Liberty) release and is only '
+                'supported by the block storage service when the '
+                'rate-limiting middleware is enabled. It is therefore a no-op '
+                'in most deployments.'
+            ),
         )
         parser.add_argument(
             "--reserved",
             dest="is_reserved",
             action="store_true",
             default=False,
-            help=_("Include reservations count [only valid with --absolute]"),
+            help=_("Include reservations count (only valid with --absolute)"),
         )
         parser.add_argument(
             '--project',
             metavar='<project>',
             help=_(
-                'Show limits for a specific project (name or ID)'
-                ' [only valid with --absolute]'
+                'Show limits for a specific project (name or ID) '
+                '(only valid with --absolute)'
             ),
         )
         parser.add_argument(
             '--domain',
             metavar='<domain>',
             help=_(
-                'Domain the project belongs to (name or ID)'
-                ' [only valid with --absolute]'
+                'Domain the project belongs to (name or ID) '
+                '(only valid with --absolute)'
             ),
         )
         return parser
 
     def take_action(self, parsed_args):
-        compute_client = self.app.client_manager.compute
-        volume_client = self.app.client_manager.volume
+        compute_client = self.app.client_manager.sdk_connection.compute
+        volume_client = self.app.client_manager.sdk_connection.volume
 
         project_id = None
         if parsed_args.project is not None:
@@ -94,33 +125,30 @@ def take_action(self, parsed_args):
         volume_limits = None
 
         if self.app.client_manager.is_compute_endpoint_enabled():
-            compute_limits = compute_client.limits.get(
-                parsed_args.is_reserved, tenant_id=project_id
+            compute_limits = compute_client.get_limits(
+                reserved=parsed_args.is_reserved, tenant_id=project_id
             )
 
         if self.app.client_manager.is_volume_endpoint_enabled(volume_client):
-            volume_limits = volume_client.limits.get()
+            volume_limits = volume_client.get_limits(
+                project_id=project_id,
+            )
 
-        data = []
         if parsed_args.is_absolute:
+            columns = ["Name", "Value"]
+            info = {}
             if compute_limits:
-                data.append(compute_limits.absolute)
+                info.update(_format_absolute_limit(compute_limits.absolute))
             if volume_limits:
-                data.append(volume_limits.absolute)
-            columns = ["Name", "Value"]
-            return (
-                columns,
-                (
-                    utils.get_item_properties(s, columns)
-                    for s in itertools.chain(*data)
-                ),
-            )
+                info.update(_format_absolute_limit(volume_limits.absolute))
 
-        elif parsed_args.is_rate:
+            return (columns, sorted(info.items(), key=lambda x: x[0]))
+        else:  # parsed_args.is_rate
+            data = []
             if compute_limits:
-                data.append(compute_limits.rate)
+                data.extend(_format_rate_limit(compute_limits.rate))
             if volume_limits:
-                data.append(volume_limits.rate)
+                data.extend(_format_rate_limit(volume_limits.rate))
             columns = [
                 "Verb",
                 "URI",
@@ -129,12 +157,18 @@ def take_action(self, parsed_args):
                 "Unit",
                 "Next Available",
             ]
+
             return (
                 columns,
-                (
-                    utils.get_item_properties(s, columns)
-                    for s in itertools.chain(*data)
-                ),
+                [
+                    (
+                        s['verb'],
+                        s['uri'],
+                        s['value'],
+                        s['remaining'],
+                        s['unit'],
+                        s.get('next-available') or s['next_available'],
+                    )
+                    for s in data
+                ],
             )
-        else:
-            return {}, {}

openstackclient/tests/unit/common/test_limits.py

@@ -13,23 +13,62 @@
 
 from openstackclient.common import limits
 from openstackclient.tests.unit.compute.v2 import fakes as compute_fakes
-from openstackclient.tests.unit.volume.v2 import fakes as volume_fakes
+from openstackclient.tests.unit.volume.v3 import fakes as volume_fakes
 
 
 class TestComputeLimits(compute_fakes.TestComputev2):
-    absolute_columns = [
-        'Name',
-        'Value',
-    ]
-
+    absolute_columns = ['Name', 'Value']
     rate_columns = ["Verb", "URI", "Value", "Remain", "Unit", "Next Available"]
 
     def setUp(self):
         super().setUp()
         self.app.client_manager.volume_endpoint_enabled = False
 
-        self.fake_limits = compute_fakes.FakeLimits()
-        self.compute_client.limits.get.return_value = self.fake_limits
+        self.fake_limits = compute_fakes.create_limits()
+
+        self.absolute_data = [
+            ('floating_ips', 10),
+            ('floating_ips_used', 0),
+            ('image_meta', 128),
+            ('instances', 10),
+            ('instances_used', 0),
+            ('keypairs', 100),
+            ('max_image_meta', 128),
+            ('max_security_group_rules', 20),
+            ('max_security_groups', 10),
+            ('max_server_group_members', 10),
+            ('max_server_groups', 10),
+            ('max_server_meta', 128),
+            ('max_total_cores', 20),
+            ('max_total_floating_ips', 10),
+            ('max_total_instances', 10),
+            ('max_total_keypairs', 100),
+            ('max_total_ram_size', 51200),
+            ('personality', 5),
+            ('personality_size', 10240),
+            ('security_group_rules', 20),
+            ('security_groups', 10),
+            ('security_groups_used', 0),
+            ('server_group_members', 10),
+            ('server_groups', 10),
+            ('server_groups_used', 0),
+            ('server_meta', 128),
+            ('total_cores', 20),
+            ('total_cores_used', 0),
+            ('total_floating_ips_used', 0),
+            ('total_instances_used', 0),
+            ('total_ram', 51200),
+            ('total_ram_used', 0),
+            ('total_security_groups_used', 0),
+            ('total_server_groups_used', 0),
+        ]
+        self.rate_data = [
+            ('POST', '*', 10, 2, 'MINUTE', '2011-12-15T22:42:45Z'),
+            ('PUT', '*', 10, 2, 'MINUTE', '2011-12-15T22:42:45Z'),
+            ('DELETE', '*', 100, 100, 'MINUTE', '2011-12-15T22:42:45Z'),
+        ]
+
+        self.compute_sdk_client.get_limits.return_value = self.fake_limits
 
     def test_compute_show_absolute(self):
         arglist = ['--absolute']
@@ -39,12 +78,8 @@ def test_compute_show_absolute(self):
 
         columns, data = cmd.take_action(parsed_args)
 
-        ret_limits = list(data)
-        compute_reference_limits = self.fake_limits.absolute_limits()
-
         self.assertEqual(self.absolute_columns, columns)
-        self.assertEqual(compute_reference_limits, ret_limits)
-        self.assertEqual(19, len(ret_limits))
+        self.assertEqual(self.absolute_data, data)
 
     def test_compute_show_rate(self):
         arglist = ['--rate']
@@ -54,28 +89,39 @@ def test_compute_show_rate(self):
 
         columns, data = cmd.take_action(parsed_args)
 
-        ret_limits = list(data)
-        compute_reference_limits = self.fake_limits.rate_limits()
-
         self.assertEqual(self.rate_columns, columns)
-        self.assertEqual(compute_reference_limits, ret_limits)
-        self.assertEqual(3, len(ret_limits))
+        self.assertEqual(self.rate_data, data)
 
 
 class TestVolumeLimits(volume_fakes.TestVolume):
-    absolute_columns = [
-        'Name',
-        'Value',
-    ]
-
+    absolute_columns = ['Name', 'Value']
     rate_columns = ["Verb", "URI", "Value", "Remain", "Unit", "Next Available"]
 
     def setUp(self):
         super().setUp()
         self.app.client_manager.compute_endpoint_enabled = False
 
-        self.fake_limits = volume_fakes.FakeLimits()
-        self.volume_client.limits.get.return_value = self.fake_limits
+        self.fake_limits = volume_fakes.create_limits()
+
+        self.absolute_data = [
+            ('max_total_backup_gigabytes', 1000),
+            ('max_total_backups', 10),
+            ('max_total_snapshots', 10),
+            ('max_total_volume_gigabytes', 1000),
+            ('max_total_volumes', 10),
+            ('total_backup_gigabytes_used', 0),
+            ('total_backups_used', 0),
+            ('total_gigabytes_used', 35),
+            ('total_snapshots_used', 1),
+            ('total_volumes_used', 4),
+        ]
+        self.rate_data = [
+            ('POST', '*', 10, 2, 'MINUTE', '2011-12-15T22:42:45Z'),
+            ('PUT', '*', 10, 2, 'MINUTE', '2011-12-15T22:42:45Z'),
+            ('DELETE', '*', 100, 100, 'MINUTE', '2011-12-15T22:42:45Z'),
+        ]
+
+        self.volume_sdk_client.get_limits.return_value = self.fake_limits
 
     def test_volume_show_absolute(self):
         arglist = ['--absolute']
@@ -85,12 +131,8 @@ def test_volume_show_absolute(self):
 
         columns, data = cmd.take_action(parsed_args)
 
-        ret_limits = list(data)
-        compute_reference_limits = self.fake_limits.absolute_limits()
-
         self.assertEqual(self.absolute_columns, columns)
-        self.assertEqual(compute_reference_limits, ret_limits)
-        self.assertEqual(10, len(ret_limits))
+        self.assertEqual(self.absolute_data, data)
 
     def test_volume_show_rate(self):
         arglist = ['--rate']
@@ -100,9 +142,5 @@ def test_volume_show_rate(self):
 
         columns, data = cmd.take_action(parsed_args)
 
-        ret_limits = list(data)
-        compute_reference_limits = self.fake_limits.rate_limits()
-
         self.assertEqual(self.rate_columns, columns)
-        self.assertEqual(compute_reference_limits, ret_limits)
-        self.assertEqual(3, len(ret_limits))
+        self.assertEqual(self.rate_data, data)