From 9d394372823053e6a4038e14f81e14d1a32a6e3b Mon Sep 17 00:00:00 2001
From: whoami-rajat <rajatdhasmana@gmail.com>
Date: Mon, 17 Jul 2023 15:34:44 +0530
Subject: [PATCH] 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 | 12 +-
openstackclient/common/limits.py | 102 ++++++++++------
.../tests/unit/common/test_limits.py | 108 +++++++++++------
.../tests/unit/compute/v2/fakes.py | 87 ++------------
openstackclient/tests/unit/volume/v2/fakes.py | 111 ------------------
openstackclient/tests/unit/volume/v3/fakes.py | 53 +++++++++
.../migrate-limits-show-f586c9762dfd7d0c.yaml | 4 +
7 files changed, 222 insertions(+), 255 deletions(-)
create mode 100644 releasenotes/notes/migrate-limits-show-f586c9762dfd7d0c.yaml
diff --git a/openstackclient/common/clientmanager.py b/openstackclient/common/clientmanager.py
index 8a67dc0c0d..8caf94a5d9 100644
--- a/openstackclient/common/clientmanager.py
+++ b/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
diff --git a/openstackclient/common/limits.py b/openstackclient/common/limits.py
index 7050d2fd04..043bf5321b 100644
--- a/openstackclient/common/limits.py
+++ b/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)'
- ' [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 @@ class ShowLimits(command.Lister):
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()
-
- 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)
- ),
+ volume_limits = volume_client.get_limits(
+ project_id=project_id,
)
- 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 {}, {}
diff --git a/openstackclient/tests/unit/common/test_limits.py b/openstackclient/tests/unit/common/test_limits.py
index 86b17b1826..b7309ab7c6 100644
--- a/openstackclient/tests/unit/common/test_limits.py
+++ b/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 @@ 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(19, len(ret_limits))
+ self.assertEqual(self.absolute_data, data)
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(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 @@ 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(10, len(ret_limits))
+ self.assertEqual(self.absolute_data, data)
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(3, len(ret_limits))
+ self.assertEqual(self.rate_data, data)
diff --git a/openstackclient/tests/unit/compute/v2/fakes.py b/openstackclient/tests/unit/compute/v2/fakes.py
index 614bd90281..3e89d49057 100644
--- a/openstackclient/tests/unit/compute/v2/fakes.py
+++ b/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:
- """Fake limits"""
+def create_limits(attrs=None):
+ """Create a fake limits object."""
+ attrs = attrs or {}
- def __init__(self, absolute_attrs=None, rate_attrs=None):
- self.absolute_limits_attrs = {
+ 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 {}
- self.absolute_limits_attrs.update(absolute_attrs)
-
- self.rate_limits_attrs = [
+ },
+ 'rate': [
{
"uri": "*",
"limit": [
@@ -1232,69 +1228,12 @@ class FakeLimits:
},
],
}
- ]
+ ],
+ }
+ limits_attrs.update(attrs)
- @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
+ limits = _limits.Limits(**limits_attrs)
+ return limits
def create_one_migration(attrs=None):
diff --git a/openstackclient/tests/unit/volume/v2/fakes.py b/openstackclient/tests/unit/volume/v2/fakes.py
index 9ba3b2bf5a..389b10b0f9 100644
--- a/openstackclient/tests/unit/volume/v2/fakes.py
+++ b/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
diff --git a/openstackclient/tests/unit/volume/v3/fakes.py b/openstackclient/tests/unit/volume/v3/fakes.py
index 6a9f1bb0e5..e792827047 100644
--- a/openstackclient/tests/unit/volume/v3/fakes.py
+++ b/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.
diff --git a/releasenotes/notes/migrate-limits-show-f586c9762dfd7d0c.yaml b/releasenotes/notes/migrate-limits-show-f586c9762dfd7d0c.yaml
new file mode 100644
index 0000000000..81a5119ada
--- /dev/null
+++ b/releasenotes/notes/migrate-limits-show-f586c9762dfd7d0c.yaml
@@ -0,0 +1,4 @@
+---
+upgrade:
+ - |
+ The ``limits show`` command has been migrated to SDK.