From f371f29cd0ed0fc3d8c4811dce8e2c31f39a0673 Mon Sep 17 00:00:00 2001 From: Salvatore Orlando Date: Tue, 10 Mar 2015 13:37:07 -0700 Subject: [PATCH] Raise QuotaResourceUnknown in the quota engine This patch lets the quota engine verify whether it manages a resource for which a limit is being checked. So far this check has been delegated to the drivers. This is conceptually wrong and also lead to code duplication. Unit tests and some docstrings are also fixed accordingly as a part of this patch. Closes-Bug: #1430519 Related to blueprint better-quotas Change-Id: If1467881f13e30afa53a23b904f8eae5c7264834 --- neutron/db/quota_db.py | 21 ++---------- neutron/quota.py | 46 ++++++++++++-------------- neutron/tests/unit/db/test_quota_db.py | 12 ------- neutron/tests/unit/test_quota_ext.py | 11 ++++-- 4 files changed, 33 insertions(+), 57 deletions(-) diff --git a/neutron/db/quota_db.py b/neutron/db/quota_db.py index dc6a3cf4bb2..262b8f85fca 100644 --- a/neutron/db/quota_db.py +++ b/neutron/db/quota_db.py @@ -112,7 +112,7 @@ class DbQuotaDriver(object): limit=limit) context.session.add(tenant_quota) - def _get_quotas(self, context, tenant_id, resources, keys): + def _get_quotas(self, context, tenant_id, resources): """Retrieves the quotas for specific resources. A helper method which retrieves the quotas for the specific @@ -122,21 +122,10 @@ class DbQuotaDriver(object): :param context: The request context, for access checks. :param tenant_id: the tenant_id to check quota. :param resources: A dictionary of the registered resources. - :param keys: A list of the desired quotas to retrieve. - """ - desired = set(keys) - sub_resources = dict((k, v) for k, v in resources.items() - if k in desired) - - # Make sure we accounted for all of them... - if len(keys) != len(sub_resources): - unknown = desired - set(sub_resources.keys()) - raise exceptions.QuotaResourceUnknown(unknown=sorted(unknown)) - # Grab and return the quotas (without usages) quotas = DbQuotaDriver.get_tenant_quotas( - context, sub_resources, tenant_id) + context, resources, tenant_id) return dict((k, v) for k, v in quotas.items()) @@ -147,10 +136,6 @@ class DbQuotaDriver(object): synchronization function--this method checks that a set of proposed values are permitted by the limit restriction. - This method will raise a QuotaResourceUnknown exception if a - given resource is unknown or if it is not a simple limit - resource. - If any of the proposed values is over the defined quota, an OverQuota exception will be raised with the sorted list of the resources which are too high. Otherwise, the method returns @@ -169,7 +154,7 @@ class DbQuotaDriver(object): raise exceptions.InvalidQuotaValue(unders=sorted(unders)) # Get the applicable quotas - quotas = self._get_quotas(context, tenant_id, resources, values.keys()) + quotas = self._get_quotas(context, tenant_id, resources) # Check the quotas and construct a list of the resources that # would be put over limit by the desired values diff --git a/neutron/quota.py b/neutron/quota.py index f390629ff34..25c6ad2c256 100644 --- a/neutron/quota.py +++ b/neutron/quota.py @@ -66,7 +66,7 @@ class ConfDriver(object): in neutron.conf. """ - def _get_quotas(self, context, resources, keys): + def _get_quotas(self, context, resources): """Get quotas. A helper method which retrieves the quotas for the specific @@ -75,20 +75,10 @@ class ConfDriver(object): :param context: The request context, for access checks. :param resources: A dictionary of the registered resources. - :param keys: A list of the desired quotas to retrieve. """ - # Filter resources - desired = set(keys) - sub_resources = dict((k, v) for k, v in resources.items() - if k in desired) - - # Make sure we accounted for all of them... - if len(keys) != len(sub_resources): - unknown = desired - set(sub_resources.keys()) - raise exceptions.QuotaResourceUnknown(unknown=sorted(unknown)) quotas = {} - for resource in sub_resources.values(): + for resource in resources.values(): quotas[resource.name] = resource.default return quotas @@ -100,10 +90,6 @@ class ConfDriver(object): synchronization function--this method checks that a set of proposed values are permitted by the limit restriction. - This method will raise a QuotaResourceUnknown exception if a - given resource is unknown or if it is not a simple limit - resource. - If any of the proposed values is over the defined quota, an OverQuota exception will be raised with the sorted list of the resources which are too high. Otherwise, the method returns @@ -115,14 +101,13 @@ class ConfDriver(object): :param values: A dictionary of the values to check against the quota. """ - # Ensure no value is less than zero unders = [key for key, val in values.items() if val < 0] if unders: raise exceptions.InvalidQuotaValue(unders=sorted(unders)) # Get the applicable quotas - quotas = self._get_quotas(context, resources, values.keys()) + quotas = self._get_quotas(context, resources) # Check the quotas and construct a list of the resources that # would be put over limit by the desired values @@ -284,16 +269,27 @@ class QuotaEngine(object): the proposed value. This method will raise a QuotaResourceUnknown exception if a - given resource is unknown or if it is not a simple limit - resource. + given resource is unknown or if it is not a countable resource. - If any of the proposed values is over the defined quota, an - OverQuota exception will be raised with the sorted list of the - resources which are too high. Otherwise, the method returns - nothing. + If any of the proposed values exceeds the respective quota defined + for the tenant, an OverQuota exception will be raised. + The exception will include a sorted list with the resources + which exceed the quota limit. Otherwise, the method returns nothing. - :param context: The request context, for access checks. + :param context: Request context + :param tenant_id: Tenant for which the quota limit is being checked + :param values: Dict specifying requested deltas for each resource """ + # Verify that resources are managed by the quota engine + requested_resources = set(values.keys()) + managed_resources = set([res for res in self._resources.keys() + if res in requested_resources]) + + # Make sure we accounted for all of them... + unknown_resources = requested_resources - managed_resources + if unknown_resources: + raise exceptions.QuotaResourceUnknown( + unknown=sorted(unknown_resources)) return self.get_driver().limit_check(context, tenant_id, self._resources, values) diff --git a/neutron/tests/unit/db/test_quota_db.py b/neutron/tests/unit/db/test_quota_db.py index b6c9ed83f81..b6ba3f52016 100644 --- a/neutron/tests/unit/db/test_quota_db.py +++ b/neutron/tests/unit/db/test_quota_db.py @@ -132,15 +132,3 @@ class TestDbQuotaDriver(testlib_api.SqlTestCase): self.assertRaises(exceptions.InvalidQuotaValue, self.plugin.limit_check, context.get_admin_context(), PROJECT, resources, values) - - def test_limit_check_wrong_values_size(self): - resource_1 = 'res_test_1' - resource_2 = 'res_test_2' - - resources = {resource_1: TestResource(resource_1, 2)} - values = {resource_1: 1, resource_2: 1} - - self.plugin.update_quota_limit(self.context, PROJECT, resource_1, 2) - self.assertRaises(exceptions.QuotaResourceUnknown, - self.plugin.limit_check, context.get_admin_context(), - PROJECT, resources, values) diff --git a/neutron/tests/unit/test_quota_ext.py b/neutron/tests/unit/test_quota_ext.py index 88df8946fc9..2f700618ec0 100644 --- a/neutron/tests/unit/test_quota_ext.py +++ b/neutron/tests/unit/test_quota_ext.py @@ -302,6 +302,14 @@ class QuotaExtensionDbTestCase(QuotaExtensionTestCase): tenant_id, network=-2) + def test_quotas_limit_check_with_not_registered_resource_fails(self): + tenant_id = 'tenant_id1' + self.assertRaises(exceptions.QuotaResourceUnknown, + quota.QUOTAS.limit_check, + context.get_admin_context(load_admin_roles=False), + tenant_id, + foobar=1) + def test_quotas_get_tenant_from_request_context(self): tenant_id = 'tenant_id1' env = {'neutron.context': context.Context('', tenant_id, @@ -393,8 +401,7 @@ class TestDbQuotaDriver(base.BaseTestCase): quotas = driver._get_quotas(ctx, target_tenant, - default_quotas, - ['network']) + default_quotas) self.assertEqual(quotas, foo_quotas) get_tenant_quotas.assert_called_once_with(ctx,