From 72058b7a405bade6ca9584cb74591e35a73fb458 Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Mon, 7 Feb 2022 12:30:46 -0800 Subject: [PATCH] Join quota exception family trees For some reason, we have two lineages of quota-related exceptions in Nova. We have QuotaError (which sounds like an actual error), from which all of our case-specific "over quota" exceptions inhert, such as KeypairLimitExceeded, etc. In contrast, we have OverQuota which lives outside that hierarchy and is unrelated. In a number of places, we raise one and translate to the other, or raise the generic QuotaError to signal an overquota situation, instead of OverQuota. This leads to places where we have to catch both, signaling the same over quota situation, but looking like there could be two different causes (i.e. an error and being over quota). This joins the two cases, by putting OverQuota at the top of the hierarchy of specific exceptions and removing QuotaError. The latter was only used in a few situations, so this isn't actually much change. Cleaning this up will help with the unified limits work, reducing the number of potential exceptions that mean the same thing. Related to blueprint bp/unified-limits-nova Change-Id: I17a3e20b8be98f9fb1a04b91fcf1237d67165871 --- nova/api/openstack/compute/deferred_delete.py | 2 +- nova/api/openstack/compute/migrate_server.py | 2 +- nova/api/openstack/compute/server_metadata.py | 2 +- nova/api/openstack/compute/servers.py | 7 +++--- nova/compute/api.py | 6 ++--- nova/exception.py | 25 +++++++------------ .../unit/api/openstack/compute/test_api.py | 2 +- nova/tests/unit/compute/test_compute.py | 2 +- nova/tests/unit/test_quota.py | 22 ++++++++-------- 9 files changed, 31 insertions(+), 39 deletions(-) diff --git a/nova/api/openstack/compute/deferred_delete.py b/nova/api/openstack/compute/deferred_delete.py index 55879267ffdf..b3f461cca42b 100644 --- a/nova/api/openstack/compute/deferred_delete.py +++ b/nova/api/openstack/compute/deferred_delete.py @@ -40,7 +40,7 @@ class DeferredDeleteController(wsgi.Controller): target={'project_id': instance.project_id}) try: self.compute_api.restore(context, instance) - except exception.QuotaError as error: + except exception.OverQuota as error: raise webob.exc.HTTPForbidden(explanation=error.format_message()) except exception.InstanceInvalidState as state_error: common.raise_http_conflict_for_instance_invalid_state(state_error, diff --git a/nova/api/openstack/compute/migrate_server.py b/nova/api/openstack/compute/migrate_server.py index 855f51a7c64f..59b9c384df60 100644 --- a/nova/api/openstack/compute/migrate_server.py +++ b/nova/api/openstack/compute/migrate_server.py @@ -57,7 +57,7 @@ class MigrateServerController(wsgi.Controller): try: self.compute_api.resize(req.environ['nova.context'], instance, host_name=host_name) - except (exception.TooManyInstances, exception.QuotaError) as e: + except exception.OverQuota as e: raise exc.HTTPForbidden(explanation=e.format_message()) except ( exception.InstanceIsLocked, diff --git a/nova/api/openstack/compute/server_metadata.py b/nova/api/openstack/compute/server_metadata.py index 448441a346cc..e92becb582ad 100644 --- a/nova/api/openstack/compute/server_metadata.py +++ b/nova/api/openstack/compute/server_metadata.py @@ -114,7 +114,7 @@ class ServerMetadataController(wsgi.Controller): server, metadata, delete) - except exception.QuotaError as error: + except exception.OverQuota as error: raise exc.HTTPForbidden(explanation=error.format_message()) except exception.InstanceIsLocked as e: raise exc.HTTPConflict(explanation=e.format_message()) diff --git a/nova/api/openstack/compute/servers.py b/nova/api/openstack/compute/servers.py index 1d6d29b45f60..9c2ed62961c0 100644 --- a/nova/api/openstack/compute/servers.py +++ b/nova/api/openstack/compute/servers.py @@ -797,8 +797,7 @@ class ServersController(wsgi.Controller): supports_multiattach=supports_multiattach, supports_port_resource_request=supports_port_resource_request, **create_kwargs) - except (exception.QuotaError, - exception.PortLimitExceeded) as error: + except exception.OverQuota as error: raise exc.HTTPForbidden( explanation=error.format_message()) except exception.ImageNotFound: @@ -1053,7 +1052,7 @@ class ServersController(wsgi.Controller): try: self.compute_api.resize(context, instance, flavor_id, auto_disk_config=auto_disk_config) - except exception.QuotaError as error: + except exception.OverQuota as error: raise exc.HTTPForbidden( explanation=error.format_message()) except ( @@ -1237,7 +1236,7 @@ class ServersController(wsgi.Controller): except exception.KeypairNotFound: msg = _("Invalid key_name provided.") raise exc.HTTPBadRequest(explanation=msg) - except exception.QuotaError as error: + except exception.OverQuota as error: raise exc.HTTPForbidden(explanation=error.format_message()) except (exception.AutoDiskConfigDisabledByImage, exception.CertificateValidationFailed, diff --git a/nova/compute/api.py b/nova/compute/api.py index 28368d910fde..ae756945ea2a 100644 --- a/nova/compute/api.py +++ b/nova/compute/api.py @@ -398,7 +398,7 @@ class API: def _check_injected_file_quota(self, context, injected_files): """Enforce quota limits on injected files. - Raises a QuotaError if any limit is exceeded. + Raises a OverQuota if any limit is exceeded. """ if not injected_files: return @@ -1455,7 +1455,7 @@ class API: except exception.OverQuota: msg = _("Quota exceeded, too many servers in " "group") - raise exception.QuotaError(msg) + raise exception.OverQuota(msg) members = objects.InstanceGroup.add_members( context, instance_group.uuid, [instance.uuid]) @@ -1475,7 +1475,7 @@ class API: context, instance_group.id, [instance.uuid]) msg = _("Quota exceeded, too many servers in " "group") - raise exception.QuotaError(msg) + raise exception.OverQuota(msg) # list of members added to servers group in this iteration # is needed to check quota of server group during add next # instance diff --git a/nova/exception.py b/nova/exception.py index c0f25bd260bb..a140a2e30b2d 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -993,10 +993,6 @@ class QuotaClassExists(NovaException): msg_fmt = _("Quota class %(class_name)s exists for resource %(resource)s") -class OverQuota(NovaException): - msg_fmt = _("Quota exceeded for resources: %(overs)s") - - class SecurityGroupNotFound(NotFound): msg_fmt = _("Security group %(security_group_id)s not found.") @@ -1233,29 +1229,26 @@ class MaxRetriesExceeded(NoValidHost): msg_fmt = _("Exceeded maximum number of retries. %(reason)s") -class QuotaError(NovaException): - msg_fmt = _("Quota exceeded: code=%(code)s") - # NOTE(cyeoh): 413 should only be used for the ec2 API - # The error status code for out of quota for the nova api should be - # 403 Forbidden. +class OverQuota(NovaException): + msg_fmt = _("Quota exceeded for resources: %(overs)s") code = 413 safe = True -class TooManyInstances(QuotaError): +class TooManyInstances(OverQuota): msg_fmt = _("Quota exceeded for %(overs)s: Requested %(req)s," " but already used %(used)s of %(allowed)s %(overs)s") -class FloatingIpLimitExceeded(QuotaError): +class FloatingIpLimitExceeded(OverQuota): msg_fmt = _("Maximum number of floating IPs exceeded") -class MetadataLimitExceeded(QuotaError): +class MetadataLimitExceeded(OverQuota): msg_fmt = _("Maximum number of metadata items exceeds %(allowed)d") -class OnsetFileLimitExceeded(QuotaError): +class OnsetFileLimitExceeded(OverQuota): msg_fmt = _("Personality file limit exceeded") @@ -1267,15 +1260,15 @@ class OnsetFileContentLimitExceeded(OnsetFileLimitExceeded): msg_fmt = _("Personality file content exceeds maximum %(allowed)s") -class KeypairLimitExceeded(QuotaError): +class KeypairLimitExceeded(OverQuota): msg_fmt = _("Maximum number of key pairs exceeded") -class SecurityGroupLimitExceeded(QuotaError): +class SecurityGroupLimitExceeded(OverQuota): msg_fmt = _("Maximum number of security groups or rules exceeded") -class PortLimitExceeded(QuotaError): +class PortLimitExceeded(OverQuota): msg_fmt = _("Maximum number of ports exceeded") diff --git a/nova/tests/unit/api/openstack/compute/test_api.py b/nova/tests/unit/api/openstack/compute/test_api.py index ca54be9a74c9..d1bb6babb779 100644 --- a/nova/tests/unit/api/openstack/compute/test_api.py +++ b/nova/tests/unit/api/openstack/compute/test_api.py @@ -143,7 +143,7 @@ class APITest(test.NoDBTestCase): self.assertEqual(resp.headers[key], str(value)) def test_quota_error_mapping(self): - self._do_test_exception_mapping(exception.QuotaError, 'too many used') + self._do_test_exception_mapping(exception.OverQuota, 'too many used') def test_non_nova_notfound_exception_mapping(self): class ExceptionWithCode(Exception): diff --git a/nova/tests/unit/compute/test_compute.py b/nova/tests/unit/compute/test_compute.py index f65f1abdb740..78dfff2fc7f1 100644 --- a/nova/tests/unit/compute/test_compute.py +++ b/nova/tests/unit/compute/test_compute.py @@ -8856,7 +8856,7 @@ class ComputeAPITestCase(BaseTestCase): group.create() get_group_mock.return_value = group - self.assertRaises(exception.QuotaError, self.compute_api.create, + self.assertRaises(exception.OverQuota, self.compute_api.create, self.context, self.default_flavor, self.fake_image['id'], scheduler_hints={'group': group.uuid}, check_server_group_quota=True) diff --git a/nova/tests/unit/test_quota.py b/nova/tests/unit/test_quota.py index 312449b13a22..48910cf75cb8 100644 --- a/nova/tests/unit/test_quota.py +++ b/nova/tests/unit/test_quota.py @@ -109,7 +109,7 @@ class QuotaIntegrationTestCase(test.TestCase): self.compute_api.create( self.context, min_count=1, max_count=1, flavor=self.flavor, image_href=image_uuid) - except exception.QuotaError as e: + except exception.OverQuota as e: expected_kwargs = {'code': 413, 'req': '1, 1', 'used': '8, 2', @@ -117,7 +117,7 @@ class QuotaIntegrationTestCase(test.TestCase): 'overs': 'cores, instances'} self.assertEqual(expected_kwargs, e.kwargs) else: - self.fail('Expected QuotaError exception') + self.fail('Expected OverQuota exception') def test_too_many_cores(self): self._create_instance() @@ -126,7 +126,7 @@ class QuotaIntegrationTestCase(test.TestCase): self.compute_api.create( self.context, min_count=1, max_count=1, flavor=self.flavor, image_href=image_uuid) - except exception.QuotaError as e: + except exception.OverQuota as e: expected_kwargs = {'code': 413, 'req': '1', 'used': '4', @@ -134,7 +134,7 @@ class QuotaIntegrationTestCase(test.TestCase): 'overs': 'cores'} self.assertEqual(expected_kwargs, e.kwargs) else: - self.fail('Expected QuotaError exception') + self.fail('Expected OverQuota exception') def test_many_cores_with_unlimited_quota(self): # Setting cores quota to unlimited: @@ -150,7 +150,7 @@ class QuotaIntegrationTestCase(test.TestCase): metadata['key%s' % i] = 'value%s' % i image_uuid = 'cedef40a-ed67-4d10-800e-17455edce175' self.assertRaises( - exception.QuotaError, self.compute_api.create, + exception.OverQuota, self.compute_api.create, self.context, min_count=1, max_count=1, flavor=self.flavor, image_href=image_uuid, metadata=metadata) @@ -170,39 +170,39 @@ class QuotaIntegrationTestCase(test.TestCase): files = [] for i in range(CONF.quota.injected_files): files.append(('/my/path%d' % i, 'config = test\n')) - self._create_with_injected_files(files) # no QuotaError + self._create_with_injected_files(files) # no OverQuota def test_too_many_injected_files(self): files = [] for i in range(CONF.quota.injected_files + 1): files.append(('/my/path%d' % i, 'my\ncontent%d\n' % i)) - self.assertRaises(exception.QuotaError, + self.assertRaises(exception.OverQuota, self._create_with_injected_files, files) def test_max_injected_file_content_bytes(self): max = CONF.quota.injected_file_content_bytes content = ''.join(['a' for i in range(max)]) files = [('/test/path', content)] - self._create_with_injected_files(files) # no QuotaError + self._create_with_injected_files(files) # no OverQuota def test_too_many_injected_file_content_bytes(self): max = CONF.quota.injected_file_content_bytes content = ''.join(['a' for i in range(max + 1)]) files = [('/test/path', content)] - self.assertRaises(exception.QuotaError, + self.assertRaises(exception.OverQuota, self._create_with_injected_files, files) def test_max_injected_file_path_bytes(self): max = CONF.quota.injected_file_path_length path = ''.join(['a' for i in range(max)]) files = [(path, 'config = quotatest')] - self._create_with_injected_files(files) # no QuotaError + self._create_with_injected_files(files) # no OverQuota def test_too_many_injected_file_path_bytes(self): max = CONF.quota.injected_file_path_length path = ''.join(['a' for i in range(max + 1)]) files = [(path, 'config = quotatest')] - self.assertRaises(exception.QuotaError, + self.assertRaises(exception.OverQuota, self._create_with_injected_files, files)