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] https://github.com/openstack/nova/commit/ca4ec762804

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

openstackclient/common/clientmanager.py

@@ -133,7 +133,17 @@ class ClientManager(clientmanager.ClientManager):
         # 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.i18n import _
 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 @@ class ShowLimits(command.Lister):
             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)'
+                'Show limits for a specific project (name or ID) '
-                ' [only valid with --absolute]'
+                '(only valid with --absolute)'
             ),
         )
         parser.add_argument(
             '--domain',
             metavar='<domain>',
             help=_(
-                'Domain the project belongs to (name or ID)'
+                'Domain the project belongs to (name or ID) '
-                ' [only valid with --absolute]'
+                '(only valid with --absolute)'
             ),
         )
         return parser
 
     def take_action(self, parsed_args):
-        compute_client = self.app.client_manager.compute
+        compute_client = self.app.client_manager.sdk_connection.compute
-        volume_client = self.app.client_manager.volume
+        volume_client = self.app.client_manager.sdk_connection.volume
 
         project_id = None
         if parsed_args.project is not None:
@@ -94,33 +125,30 @@ class ShowLimits(command.Lister):
         volume_limits = None
 
         if self.app.client_manager.is_compute_endpoint_enabled():
-            compute_limits = compute_client.limits.get(
+            compute_limits = compute_client.get_limits(
-                parsed_args.is_reserved, tenant_id=project_id
+                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:
-            if compute_limits:
-                data.append(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)
-                ),
             )
 
-        elif parsed_args.is_rate:
+        if parsed_args.is_absolute:
+            columns = ["Name", "Value"]
+            info = {}
             if compute_limits:
-                data.append(compute_limits.rate)
+                info.update(_format_absolute_limit(compute_limits.absolute))
             if volume_limits:
-                data.append(volume_limits.rate)
+                info.update(_format_absolute_limit(volume_limits.absolute))
+
+            return (columns, sorted(info.items(), key=lambda x: x[0]))
+        else:  # parsed_args.is_rate
+            data = []
+            if compute_limits:
+                data.extend(_format_rate_limit(compute_limits.rate))
+            if volume_limits:
+                data.extend(_format_rate_limit(volume_limits.rate))
             columns = [
                 "Verb",
                 "URI",
@@ -129,12 +157,18 @@ class ShowLimits(command.Lister):
                 "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 = [
+    absolute_columns = ['Name', 'Value']
-        '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.fake_limits = compute_fakes.create_limits()
-        self.compute_client.limits.get.return_value = self.fake_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 @@ class TestComputeLimits(compute_fakes.TestComputev2):
 
         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(self.absolute_data, data)
-        self.assertEqual(19, len(ret_limits))
 
     def test_compute_show_rate(self):
         arglist = ['--rate']
@@ -54,28 +89,39 @@ class TestComputeLimits(compute_fakes.TestComputev2):
 
         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(self.rate_data, data)
-        self.assertEqual(3, len(ret_limits))
 
 
 class TestVolumeLimits(volume_fakes.TestVolume):
-    absolute_columns = [
+    absolute_columns = ['Name', 'Value']
-        '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.fake_limits = volume_fakes.create_limits()
-        self.volume_client.limits.get.return_value = self.fake_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 @@ class TestVolumeLimits(volume_fakes.TestVolume):
 
         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(self.absolute_data, data)
-        self.assertEqual(10, len(ret_limits))
 
     def test_volume_show_rate(self):
         arglist = ['--rate']
@@ -100,9 +142,5 @@ class TestVolumeLimits(volume_fakes.TestVolume):
 
         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(self.rate_data, data)
-        self.assertEqual(3, len(ret_limits))

openstackclient/tests/unit/compute/v2/fakes.py

@@ -28,6 +28,7 @@ from openstack.compute.v2 import extension as _extension
 from openstack.compute.v2 import flavor as _flavor
 from openstack.compute.v2 import hypervisor as _hypervisor
 from openstack.compute.v2 import keypair as _keypair
+from openstack.compute.v2 import limits as _limits
 from openstack.compute.v2 import migration as _migration
 from openstack.compute.v2 import server as _server
 from openstack.compute.v2 import server_action as _server_action
@@ -91,9 +92,6 @@ class FakeComputev2Client:
         self.images = mock.Mock()
         self.images.resource_class = fakes.FakeResource(None, {})
 
-        self.limits = mock.Mock()
-        self.limits.resource_class = fakes.FakeResource(None, {})
-
         self.servers = mock.Mock()
         self.servers.resource_class = fakes.FakeResource(None, {})
 
@@ -1177,11 +1175,12 @@ def create_one_comp_detailed_quota(attrs=None):
     return quota
 
 
-class FakeLimits:
+def create_limits(attrs=None):
-    """Fake limits"""
+    """Create a fake limits object."""
+    attrs = attrs or {}
 
-    def __init__(self, absolute_attrs=None, rate_attrs=None):
+    limits_attrs = {
-        self.absolute_limits_attrs = {
+        'absolute': {
             'maxServerMeta': 128,
             'maxTotalInstances': 10,
             'maxPersonality': 5,
@@ -1201,11 +1200,8 @@ class FakeLimits:
             'maxTotalFloatingIps': 10,
             'totalSecurityGroupsUsed': 0,
             'maxTotalCores': 20,
-        }
+        },
-        absolute_attrs = absolute_attrs or {}
+        'rate': [
-        self.absolute_limits_attrs.update(absolute_attrs)
-
-        self.rate_limits_attrs = [
             {
                 "uri": "*",
                 "limit": [
@@ -1232,69 +1228,12 @@ class FakeLimits:
                     },
                 ],
             }
-        ]
+        ],
+    }
+    limits_attrs.update(attrs)
 
-    @property
+    limits = _limits.Limits(**limits_attrs)
-    def absolute(self):
+    return limits
-        for name, value in self.absolute_limits_attrs.items():
-            yield FakeAbsoluteLimit(name, value)
-
-    def absolute_limits(self):
-        reference_data = []
-        for name, value in self.absolute_limits_attrs.items():
-            reference_data.append((name, value))
-        return reference_data
-
-    @property
-    def rate(self):
-        for group in self.rate_limits_attrs:
-            uri = group['uri']
-            for rate in group['limit']:
-                yield FakeRateLimit(
-                    rate['verb'],
-                    uri,
-                    rate['value'],
-                    rate['remaining'],
-                    rate['unit'],
-                    rate['next-available'],
-                )
-
-    def rate_limits(self):
-        reference_data = []
-        for group in self.rate_limits_attrs:
-            uri = group['uri']
-            for rate in group['limit']:
-                reference_data.append(
-                    (
-                        rate['verb'],
-                        uri,
-                        rate['value'],
-                        rate['remaining'],
-                        rate['unit'],
-                        rate['next-available'],
-                    )
-                )
-        return reference_data
-
-
-class FakeAbsoluteLimit:
-    """Data model that represents an absolute limit"""
-
-    def __init__(self, name, value):
-        self.name = name
-        self.value = value
-
-
-class FakeRateLimit:
-    """Data model that represents a flattened view of a single rate limit"""
-
-    def __init__(self, verb, uri, value, remain, unit, next_available):
-        self.verb = verb
-        self.uri = uri
-        self.value = value
-        self.remain = remain
-        self.unit = unit
-        self.next_available = next_available
 
 
 def create_one_migration(attrs=None):

openstackclient/tests/unit/volume/v2/fakes.py

@@ -1062,114 +1062,3 @@ def create_one_detailed_quota(attrs=None):
     quota = fakes.FakeResource(info=copy.deepcopy(quota_attrs), loaded=True)
 
     return quota
-
-
-class FakeLimits:
-    """Fake limits"""
-
-    def __init__(self, absolute_attrs=None):
-        self.absolute_limits_attrs = {
-            'totalSnapshotsUsed': 1,
-            'maxTotalBackups': 10,
-            'maxTotalVolumeGigabytes': 1000,
-            'maxTotalSnapshots': 10,
-            'maxTotalBackupGigabytes': 1000,
-            'totalBackupGigabytesUsed': 0,
-            'maxTotalVolumes': 10,
-            'totalVolumesUsed': 4,
-            'totalBackupsUsed': 0,
-            'totalGigabytesUsed': 35,
-        }
-        absolute_attrs = absolute_attrs or {}
-        self.absolute_limits_attrs.update(absolute_attrs)
-
-        self.rate_limits_attrs = [
-            {
-                "uri": "*",
-                "limit": [
-                    {
-                        "value": 10,
-                        "verb": "POST",
-                        "remaining": 2,
-                        "unit": "MINUTE",
-                        "next-available": "2011-12-15T22:42:45Z",
-                    },
-                    {
-                        "value": 10,
-                        "verb": "PUT",
-                        "remaining": 2,
-                        "unit": "MINUTE",
-                        "next-available": "2011-12-15T22:42:45Z",
-                    },
-                    {
-                        "value": 100,
-                        "verb": "DELETE",
-                        "remaining": 100,
-                        "unit": "MINUTE",
-                        "next-available": "2011-12-15T22:42:45Z",
-                    },
-                ],
-            }
-        ]
-
-    @property
-    def absolute(self):
-        for name, value in self.absolute_limits_attrs.items():
-            yield FakeAbsoluteLimit(name, value)
-
-    def absolute_limits(self):
-        reference_data = []
-        for name, value in self.absolute_limits_attrs.items():
-            reference_data.append((name, value))
-        return reference_data
-
-    @property
-    def rate(self):
-        for group in self.rate_limits_attrs:
-            uri = group['uri']
-            for rate in group['limit']:
-                yield FakeRateLimit(
-                    rate['verb'],
-                    uri,
-                    rate['value'],
-                    rate['remaining'],
-                    rate['unit'],
-                    rate['next-available'],
-                )
-
-    def rate_limits(self):
-        reference_data = []
-        for group in self.rate_limits_attrs:
-            uri = group['uri']
-            for rate in group['limit']:
-                reference_data.append(
-                    (
-                        rate['verb'],
-                        uri,
-                        rate['value'],
-                        rate['remaining'],
-                        rate['unit'],
-                        rate['next-available'],
-                    )
-                )
-        return reference_data
-
-
-class FakeAbsoluteLimit:
-    """Data model that represents an absolute limit."""
-
-    def __init__(self, name, value):
-        self.name = name
-        self.value = value
-
-
-class FakeRateLimit:
-    """Data model that represents a flattened view of a single rate limit."""
-
-    def __init__(self, verb, uri, value, remain, unit, next_available):
-        self.verb = verb
-        self.uri = uri
-        self.value = value
-        self.remain = remain
-        self.unit = unit
-        self.next_available = next_available

openstackclient/tests/unit/volume/v3/fakes.py

@@ -22,6 +22,7 @@ from openstack.block_storage.v3 import _proxy
 from openstack.block_storage.v3 import availability_zone as _availability_zone
 from openstack.block_storage.v3 import backup as _backup
 from openstack.block_storage.v3 import extension as _extension
+from openstack.block_storage.v3 import limits as _limits
 from openstack.block_storage.v3 import resource_filter as _filters
 from openstack.block_storage.v3 import volume as _volume
 from openstack.compute.v2 import _proxy as _compute_proxy
@@ -423,6 +424,58 @@ def create_one_encryption_volume_type(attrs=None):
     return encryption_type
 
 
+def create_limits(attrs=None):
+    """Create a fake limits object."""
+    attrs = attrs or {}
+
+    limits_attrs = {
+        'absolute': {
+            'totalSnapshotsUsed': 1,
+            'maxTotalBackups': 10,
+            'maxTotalVolumeGigabytes': 1000,
+            'maxTotalSnapshots': 10,
+            'maxTotalBackupGigabytes': 1000,
+            'totalBackupGigabytesUsed': 0,
+            'maxTotalVolumes': 10,
+            'totalVolumesUsed': 4,
+            'totalBackupsUsed': 0,
+            'totalGigabytesUsed': 35,
+        },
+        'rate': [
+            {
+                "uri": "*",
+                "limit": [
+                    {
+                        "value": 10,
+                        "verb": "POST",
+                        "remaining": 2,
+                        "unit": "MINUTE",
+                        "next-available": "2011-12-15T22:42:45Z",
+                    },
+                    {
+                        "value": 10,
+                        "verb": "PUT",
+                        "remaining": 2,
+                        "unit": "MINUTE",
+                        "next-available": "2011-12-15T22:42:45Z",
+                    },
+                    {
+                        "value": 100,
+                        "verb": "DELETE",
+                        "remaining": 100,
+                        "unit": "MINUTE",
+                        "next-available": "2011-12-15T22:42:45Z",
+                    },
+                ],
+            }
+        ],
+    }
+    limits_attrs.update(attrs)
+
+    limits = _limits.Limit(**limits_attrs)
+    return limits
+
+
 def create_one_resource_filter(attrs=None):
     """Create a fake resource filter.
 

releasenotes/notes/migrate-limits-show-f586c9762dfd7d0c.yaml

@@ -0,0 +1,4 @@
+---
+upgrade:
+  - |
+    The ``limits show`` command has been migrated to SDK.