From a625860dde88f14a211cb55826f60a753abdf1c0 Mon Sep 17 00:00:00 2001 From: wangzihao Date: Thu, 12 Aug 2021 11:11:31 +0800 Subject: [PATCH 1/9] Changed minversion in tox to 3.18.0 The patch bumps min version of tox to 3.18.0 in order to replace tox's whitelist_externals by allowlist_externals option: https://github.com/tox-dev/tox/blob/master/docs/changelog.rst#v3180-2020-07-23 Change-Id: I1d65fb9d8d302ff1e3f33e92b668b2241741e7e0 --- tox.ini | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tox.ini b/tox.ini index ee7b5e1..54c1f12 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -minversion = 3.2.0 +minversion = 3.18.0 envlist = py3,pep8,docs ignore_basepython_conflict = true @@ -28,7 +28,7 @@ commands = {posargs} deps = -c{env:TOX_CONSTRAINTS_FILE:https://releases.openstack.org/constraints/upper/master} -r{toxinidir}/doc/requirements.txt -whitelist_externals = rm +allowlist_externals = rm commands = rm -fr doc/build sphinx-build -W --keep-going -b html doc/source doc/build/html @@ -37,7 +37,7 @@ commands = commands = python setup.py test --coverage --coverage-package-name=oslo_limit --testr-args='{posargs}' [testenv:releasenotes] -whitelist_externals = rm +allowlist_externals = rm deps = -r{toxinidir}/doc/requirements.txt commands = -- GitLab From a7d8f4119bfe11c5400cedb996636229025254a4 Mon Sep 17 00:00:00 2001 From: wangzihao Date: Thu, 12 Aug 2021 11:14:44 +0800 Subject: [PATCH 2/9] setup.cfg: Replace dashes with underscores Setuptools v54.1.0 introduces a warning that the use of dash-separated options in 'setup.cfg' will not be supported in a future version [1]. Get ahead of the issue by replacing the dashes with underscores. Without this, we see 'UserWarning' messages like the following on new enough versions of setuptools: UserWarning: Usage of dash-separated 'description-file' will not be supported in future versions. Please use the underscore name 'description_file' instead [1]https://github.com/pypa/setuptools/commit/a2e9ae4cb Change-Id: I2ea9570dcf018c484054d8078f6e5a767b23dd82 --- setup.cfg | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/setup.cfg b/setup.cfg index abbf8d7..92aff68 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,12 +1,12 @@ [metadata] name = oslo.limit summary = Limit enforcement library to assist with quota calculation. -description-file = +description_file = README.rst author = OpenStack -author-email = openstack-discuss@lists.openstack.org -home-page = https://docs.openstack.org/oslo.limit/latest/ -python-requires = >=3.6 +author_email = openstack-discuss@lists.openstack.org +home_page = https://docs.openstack.org/oslo.limit/latest/ +python_requires = >=3.6 classifier = Environment :: OpenStack Intended Audience :: Information Technology -- GitLab From e60489fb0737cc3876775b053d60c24d1d0fb930 Mon Sep 17 00:00:00 2001 From: OpenStack Release Bot Date: Fri, 10 Sep 2021 14:34:50 +0000 Subject: [PATCH 3/9] Update master for stable/xena Add file to the reno documentation build to show release notes for stable/xena. Use pbr instruction to increment the minor version number automatically so that master versions are higher than the versions on stable/xena. Sem-Ver: feature Change-Id: If079ffb950122121a9cd9254cce28bf62106ed57 --- releasenotes/source/index.rst | 1 + releasenotes/source/xena.rst | 6 ++++++ 2 files changed, 7 insertions(+) create mode 100644 releasenotes/source/xena.rst diff --git a/releasenotes/source/index.rst b/releasenotes/source/index.rst index 32a2d5b..704a974 100644 --- a/releasenotes/source/index.rst +++ b/releasenotes/source/index.rst @@ -6,6 +6,7 @@ :maxdepth: 1 unreleased + xena wallaby victoria ussuri diff --git a/releasenotes/source/xena.rst b/releasenotes/source/xena.rst new file mode 100644 index 0000000..1be85be --- /dev/null +++ b/releasenotes/source/xena.rst @@ -0,0 +1,6 @@ +========================= +Xena Series Release Notes +========================= + +.. release-notes:: + :branch: stable/xena -- GitLab From 489feb55c1fb9218153ae9afef4708f3bd078e8c Mon Sep 17 00:00:00 2001 From: OpenStack Release Bot Date: Fri, 10 Sep 2021 14:34:52 +0000 Subject: [PATCH 4/9] Add Python3 yoga unit tests This is an automatically generated patch to ensure unit testing is in place for all the of the tested runtimes for yoga. See also the PTI in governance [1]. [1]: https://governance.openstack.org/tc/reference/project-testing-interface.html Change-Id: Ic2bf80ec8f183476facb9710be968050641532e0 --- .zuul.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.zuul.yaml b/.zuul.yaml index 723bb7d..9052ced 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -28,7 +28,7 @@ templates: - check-requirements - lib-forward-testing-python3 - - openstack-python3-xena-jobs + - openstack-python3-yoga-jobs - periodic-stable-jobs - publish-openstack-docs-pti - release-notes-jobs-python3 -- GitLab From ea5ff2dc126bac5dfae3fe4bbbc423477de043f4 Mon Sep 17 00:00:00 2001 From: Takashi Kajinami Date: Tue, 12 Oct 2021 08:21:27 +0900 Subject: [PATCH 5/9] Add auth plugin options to options list Currently the oslo.config.opts entry_points provided by oslo.limit doesn't include options for auth plugins, thus the parameters to define credentials like username, password and etc are not picked up by oslo-config-generator. This adds the options for auth plugin options to the entry point so that the auth parameters are included by the generated config files and users can easily find the parameters to define the required user credential. Note that keystoneauth provides several plugins but this change covers only password plugins, assuming the password authentication is most popularly used. Change-Id: Ib440f58b589076677be9e90dd960cd4459e63746 --- oslo_limit/opts.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/oslo_limit/opts.py b/oslo_limit/opts.py index a9dc9f9..b944649 100644 --- a/oslo_limit/opts.py +++ b/oslo_limit/opts.py @@ -44,6 +44,9 @@ def list_opts(): return [(_option_group, copy.deepcopy(_options) + loading.get_session_conf_options() + + loading.get_auth_plugin_conf_options('password'), + loading.get_auth_plugin_conf_options('v2password'), + loading.get_auth_plugin_conf_options('v3password'), loading.get_adapter_conf_options(include_deprecated=False) )] -- GitLab From 43683f543ec196502ed13192518e4eb89be8868c Mon Sep 17 00:00:00 2001 From: melanie witt Date: Wed, 28 Jul 2021 23:13:17 +0000 Subject: [PATCH 6/9] Add caching of limits in Enforcer This adds caching of resource limits for an Enforcer in order to improve performance when repeated limit checks are needed. The cache lasts the lifetime of an Enforcer and is enabled by default. It can be disabled by passing cache=False when instantiating an Enforcer. One usage pattern for a caching Enforcer would be to create an Enforcer per service request so that the caching lives only as long as the request. Change-Id: I8e43dceec76aecd2b2ae23a137e56519efe29777 --- oslo_limit/fixture.py | 2 +- oslo_limit/limit.py | 49 +++++++++++++++---- oslo_limit/tests/test_limit.py | 39 +++++++++++++++ ...forcer-limit-caching-fb59725aad88b039.yaml | 7 +++ 4 files changed, 86 insertions(+), 11 deletions(-) create mode 100644 releasenotes/notes/enforcer-limit-caching-fb59725aad88b039.yaml diff --git a/oslo_limit/fixture.py b/oslo_limit/fixture.py index 032b7d0..dfea2cb 100644 --- a/oslo_limit/fixture.py +++ b/oslo_limit/fixture.py @@ -31,7 +31,7 @@ class LimitFixture(fixtures.Fixture): As in reality, only per-project overrides need be provided here; any unmentioned projects or resources will take the registered limit defaults. - :type reglimits: dict + :type projlimits: dict """ self.reglimits = reglimits self.projlimits = projlimits diff --git a/oslo_limit/limit.py b/oslo_limit/limit.py index d63f430..65c71eb 100644 --- a/oslo_limit/limit.py +++ b/oslo_limit/limit.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. +from collections import defaultdict from collections import namedtuple from keystoneauth1 import exceptions as ksa_exceptions @@ -55,31 +56,34 @@ def _get_keystone_connection(): class Enforcer(object): - def __init__(self, usage_callback): + def __init__(self, usage_callback, cache=True): """An object for checking usage against resource limits and requests. :param usage_callback: A callable function that accepts a project_id string as a parameter and calculates the current usage of a resource. :type usage_callback: callable function + :param cache: Whether to cache resource limits for the lifetime of this + enforcer. Defaults to True. + :type cache: boolean """ if not callable(usage_callback): msg = 'usage_callback must be a callable function.' raise ValueError(msg) self.connection = _get_keystone_connection() - self.model = self._get_model_impl(usage_callback) + self.model = self._get_model_impl(usage_callback, cache=cache) def _get_enforcement_model(self): """Query keystone for the configured enforcement model.""" return self.connection.get('/limits/model').json()['model']['name'] - def _get_model_impl(self, usage_callback): + def _get_model_impl(self, usage_callback, cache=True): """get the enforcement model based on configured model in keystone.""" model = self._get_enforcement_model() for impl in _MODELS: if model == impl.name: - return impl(usage_callback) + return impl(usage_callback, cache=cache) raise ValueError("enforcement model %s is not supported" % model) def enforce(self, project_id, deltas): @@ -167,16 +171,16 @@ class Enforcer(object): usage = self.model.get_project_usage(project_id, resources_to_check) return {resource: ProjectUsage(limit, usage[resource]) - for resource, limit in dict(limits).items()} + for resource, limit in limits} class _FlatEnforcer(object): name = 'flat' - def __init__(self, usage_callback): + def __init__(self, usage_callback, cache=True): self._usage_callback = usage_callback - self._utils = _EnforcerUtils() + self._utils = _EnforcerUtils(cache=cache) def get_project_limits(self, project_id, resources_to_check): return self._utils.get_project_limits(project_id, resources_to_check) @@ -202,7 +206,7 @@ class _StrictTwoLevelEnforcer(object): name = 'strict-two-level' - def __init__(self, usage_callback): + def __init__(self, usage_callback, cache=True): self._usage_callback = usage_callback def get_project_limits(self, project_id, resources_to_check): @@ -229,8 +233,13 @@ class _LimitNotFound(Exception): class _EnforcerUtils(object): """Logic common used by multiple enforcers""" - def __init__(self): + def __init__(self, cache=True): self.connection = _get_keystone_connection() + self.should_cache = cache + # {project_id: {resource_name: project_limit}} + self.plimit_cache = defaultdict(dict) + # {resource_name: registered_limit} + self.rlimit_cache = {} # get and cache endpoint info endpoint_id = CONF.oslo_limit.endpoint_id @@ -298,12 +307,32 @@ class _EnforcerUtils(object): return project_limits def _get_limit(self, project_id, resource_name): - # TODO(johngarbutt): might need to cache here + # If we are configured to cache limits, look in the cache first and use + # the cached value if there is one. Else, retrieve the limit and add it + # to the cache. Do this for both project limits and registered limits. + + # Look for a project limit first. + if (project_id in self.plimit_cache and + resource_name in self.plimit_cache[project_id]): + return self.plimit_cache[project_id][resource_name].resource_limit + project_limit = self._get_project_limit(project_id, resource_name) + + if self.should_cache and project_limit: + self.plimit_cache[project_id][resource_name] = project_limit + if project_limit: return project_limit.resource_limit + # If there is no project limit, look for a registered limit. + if resource_name in self.rlimit_cache: + return self.rlimit_cache[resource_name].default_limit + registered_limit = self._get_registered_limit(resource_name) + + if self.should_cache and registered_limit: + self.rlimit_cache[resource_name] = registered_limit + if registered_limit: return registered_limit.default_limit diff --git a/oslo_limit/tests/test_limit.py b/oslo_limit/tests/test_limit.py index c229b6d..4c3d7ff 100644 --- a/oslo_limit/tests/test_limit.py +++ b/oslo_limit/tests/test_limit.py @@ -29,6 +29,7 @@ from oslo_config import fixture as config_fixture from oslotest import base from oslo_limit import exception +from oslo_limit import fixture from oslo_limit import limit from oslo_limit import opts @@ -309,3 +310,41 @@ class TestEnforcerUtils(base.BaseTestCase): self.assertEqual(0, over_d.limit) self.assertEqual(0, over_d.current_usage) self.assertEqual(0, over_d.delta) + + def test_get_limit_cache(self, cache=True): + # No project limit and registered limit = 5 + fix = self.useFixture(fixture.LimitFixture({'foo': 5}, {})) + project_id = uuid.uuid4().hex + + utils = limit._EnforcerUtils(cache=cache) + foo_limit = utils._get_limit(project_id, 'foo') + + self.assertEqual(5, foo_limit) + self.assertEqual(1, fix.mock_conn.registered_limits.call_count) + + # Second call should be cached, so call_count for registered limits + # should remain 1. When cache is disabled, it should increase to 2 + foo_limit = utils._get_limit(project_id, 'foo') + self.assertEqual(5, foo_limit) + count = 1 if cache else 2 + self.assertEqual(count, fix.mock_conn.registered_limits.call_count) + + # Add a project limit = 1 + fix.projlimits[project_id] = {'foo': 1} + + foo_limit = utils._get_limit(project_id, 'foo') + + self.assertEqual(1, foo_limit) + # Project limits should have been queried 3 times total, once per + # _get_limit call + self.assertEqual(3, fix.mock_conn.limits.call_count) + + # Fourth call should be cached, so call_count for project limits should + # remain 3. When cache is disabled, it should increase to 4 + foo_limit = utils._get_limit(project_id, 'foo') + self.assertEqual(1, foo_limit) + count = 3 if cache else 4 + self.assertEqual(count, fix.mock_conn.limits.call_count) + + def test_get_limit_no_cache(self): + self.test_get_limit_cache(cache=False) diff --git a/releasenotes/notes/enforcer-limit-caching-fb59725aad88b039.yaml b/releasenotes/notes/enforcer-limit-caching-fb59725aad88b039.yaml new file mode 100644 index 0000000..c2b45e4 --- /dev/null +++ b/releasenotes/notes/enforcer-limit-caching-fb59725aad88b039.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + ``Enforcer`` objects now cache limits by default for the lifetime of the + object to provide improved performance when multiple calls of ``enforce()`` + are needed. This behavior is controlled by the boolean ``cache`` keyword + argument to the ``__init__`` method. -- GitLab From a49f3a04d02ef6fe43423eda313d9115588f0287 Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Mon, 30 Aug 2021 08:45:39 -0700 Subject: [PATCH 7/9] Make calculate_usage() work if limits are missing The calculate_usage interface was added recently to allow consumers to probe limits and usage without requiring the enforce behavior workflow. If a limit was passed to it that was not registered in keystone, get_project_limits() would raise a ProjectOverLimit exception itself to abort the process immediately, providing the "unregistered means zero" behavior. This works fine for the enforce workflow, but not the calculate one. This changes get_project_limits() to just return a zero limit for a missing one, which will be considered by the enforce workflow in the same way, keeping the existing behavior. It will merely be reported by the calculate workflow, which is the desired change. Change-Id: Iaab1f0d5eb0da9a667267537d86f6c70bc8db51d --- doc/source/user/usage.rst | 5 ++- oslo_limit/limit.py | 16 +++------ oslo_limit/tests/test_limit.py | 65 ++++++++++++++++++++++------------ 3 files changed, 50 insertions(+), 36 deletions(-) diff --git a/doc/source/user/usage.rst b/doc/source/user/usage.rst index 8eca3e5..b62450b 100644 --- a/doc/source/user/usage.rst +++ b/doc/source/user/usage.rst @@ -146,7 +146,10 @@ Another usage pattern is to check a limit and usage for a given project, outside the scope of enforcement. This may be useful in a reporting API to be able to expose to a user the limit and usage information that the enforcer would use to judge a resource -consumption event. +consumption event. Any limit passed to this API which is not +registered in keystone will be considered to be zero, in keeping with +the behavior of the enforcer assuming that "unregistered means no +quota." .. note:: This should ideally not be used to provide your own enforcement of diff --git a/oslo_limit/limit.py b/oslo_limit/limit.py index 65c71eb..00260ee 100644 --- a/oslo_limit/limit.py +++ b/oslo_limit/limit.py @@ -280,29 +280,21 @@ class _EnforcerUtils(object): def get_project_limits(self, project_id, resource_names): """Get all the limits for given project a resource_name list - We will raise ClaimExceedsLimit if no limit is found to ensure that - all clients of this library react to this situation in the same way. + If a limit is not found, it will be considered to be zero + (i.e. no quota) :param project_id: :param resource_names: list of resource_name strings :return: list of (resource_name,limit) pairs - - :raises exception.ClaimExceedsLimit: if no limit is found """ # Using a list to preserver the resource_name order project_limits = [] - missing_limits = [] for resource_name in resource_names: try: limit = self._get_limit(project_id, resource_name) - project_limits.append((resource_name, limit)) except _LimitNotFound: - missing_limits.append(resource_name) - - if len(missing_limits) > 0: - over_limit_list = [exception.OverLimitInfo(name, 0, 0, 0) - for name in missing_limits] - raise exception.ProjectOverLimit(project_id, over_limit_list) + limit = 0 + project_limits.append((resource_name, limit)) return project_limits diff --git a/oslo_limit/tests/test_limit.py b/oslo_limit/tests/test_limit.py index 4c3d7ff..f38108d 100644 --- a/oslo_limit/tests/test_limit.py +++ b/oslo_limit/tests/test_limit.py @@ -143,6 +143,38 @@ class TestEnforcer(base.BaseTestCase): self.assertEqual(expected, enforcer.calculate_usage(project_id, ['a', 'b'])) + @mock.patch.object(limit._EnforcerUtils, "_get_project_limit") + @mock.patch.object(limit._EnforcerUtils, "_get_registered_limit") + def test_calculate_and_enforce_some_missing(self, mock_get_reglimit, + mock_get_limit): + # Registered and project limits for a and b, c is unregistered + reg_limits = {'a': mock.MagicMock(default_limit=10), + 'b': mock.MagicMock(default_limit=10)} + prj_limits = {('bar', 'b'): mock.MagicMock(resource_limit=6)} + mock_get_reglimit.side_effect = lambda r: reg_limits.get(r) + mock_get_limit.side_effect = lambda p, r: prj_limits.get((p, r)) + + # Regardless, we have usage for all three + mock_usage = mock.MagicMock() + mock_usage.return_value = {'a': 5, 'b': 5, 'c': 5} + + enforcer = limit.Enforcer(mock_usage) + + # When we calculate usage, we should expect the default limit + # of zero for the unregistered limit + expected = { + 'a': limit.ProjectUsage(10, 5), + 'b': limit.ProjectUsage(6, 5), + 'c': limit.ProjectUsage(0, 5), + } + self.assertEqual(expected, + enforcer.calculate_usage('bar', ['a', 'b', 'c'])) + + # Make sure that if we enforce, we get the expected behavior + # of c being considered to be zero + self.assertRaises(exception.ProjectOverLimit, + enforcer.enforce, 'bar', {'a': 1, 'b': 0, 'c': 1}) + def test_calculate_usage_bad_params(self): enforcer = limit.Enforcer(mock.MagicMock()) @@ -216,14 +248,17 @@ class TestFlatEnforcer(base.BaseTestCase): self.assertEqual(0, over_a.current_usage) self.assertEqual(2, over_a.delta) - @mock.patch.object(limit._EnforcerUtils, "get_project_limits") - def test_enforce_raises_on_missing_limit(self, mock_get_limits): - mock_usage = mock.MagicMock() + @mock.patch.object(limit._EnforcerUtils, "_get_project_limit") + @mock.patch.object(limit._EnforcerUtils, "_get_registered_limit") + def test_enforce_raises_on_missing_limit(self, mock_get_reglimit, + mock_get_limit): + def mock_usage(*a): + return {'a': 1, 'b': 1} project_id = uuid.uuid4().hex deltas = {"a": 0, "b": 0} - mock_get_limits.side_effect = exception.ProjectOverLimit( - project_id, [exception.OverLimitInfo("a", 0, 0, 0)]) + mock_get_reglimit.return_value = None + mock_get_limit.return_value = None enforcer = limit._FlatEnforcer(mock_usage) self.assertRaises(exception.ProjectOverLimit, enforcer.enforce, @@ -292,24 +327,8 @@ class TestEnforcerUtils(base.BaseTestCase): limits = utils.get_project_limits(project_id, ["a", "b"]) self.assertEqual([('a', 1), ('b', 2)], limits) - e = self.assertRaises(exception.ProjectOverLimit, - utils.get_project_limits, - project_id, ["c", "d"]) - expected = ("Project %s is over a limit for " - "[Resource c is over limit of 0 due to current usage 0 " - "and delta 0, " - "Resource d is over limit of 0 due to current usage 0 " - "and delta 0]") - self.assertEqual(expected % project_id, str(e)) - self.assertEqual(project_id, e.project_id) - self.assertEqual(2, len(e.over_limit_info_list)) - over_c = e.over_limit_info_list[0] - self.assertEqual("c", over_c.resource_name) - over_d = e.over_limit_info_list[1] - self.assertEqual("d", over_d.resource_name) - self.assertEqual(0, over_d.limit) - self.assertEqual(0, over_d.current_usage) - self.assertEqual(0, over_d.delta) + limits = utils.get_project_limits(project_id, ["c", "d"]) + self.assertEqual([('c', 0), ('d', 0)], limits) def test_get_limit_cache(self, cache=True): # No project limit and registered limit = 5 -- GitLab From 7e4f36abdb72cea991b1b99a67d30b088cc60d7d Mon Sep 17 00:00:00 2001 From: Dan Smith Date: Mon, 30 Aug 2021 13:42:30 -0700 Subject: [PATCH 8/9] Allow project_id=None for enforce/calculate This allows a caller to pass None for the project_id if it only wants it to check the registered limit for a given resource. This is useful for non-project-scoped resourced where we just want to make sure some global limit hasn't been exceeded. This would also be relevant for resources that are created by system-scoped users, such as host aggregates. Change-Id: I5fea0143b6a96b5f79bc273961e3e284a260e25e --- oslo_limit/limit.py | 31 +++++++++++++++++---------- oslo_limit/tests/test_limit.py | 38 +++++++++++++++++++++++++++++++--- 2 files changed, 55 insertions(+), 14 deletions(-) diff --git a/oslo_limit/limit.py b/oslo_limit/limit.py index 00260ee..1393eed 100644 --- a/oslo_limit/limit.py +++ b/oslo_limit/limit.py @@ -92,8 +92,8 @@ class Enforcer(object): From the deltas we extract the list of resource types that need to have limits enforced on them. - From keystone we fetch limits relating to this project_id and the - endpoint specified in the configuration. + From keystone we fetch limits relating to this project_id (if + not None) and the endpoint specified in the configuration. Using the usage_callback specified when creating the enforcer, we fetch the existing usage. @@ -107,8 +107,12 @@ class Enforcer(object): a limit of zero, i.e. do not allow any use of a resource type that does not have a registered limit. + Note that if a project_id of None is provided, we just compare + against the registered limits (i.e. use this for + non-project-scoped limits) + :param project_id: The project to check usage and enforce limits - against. + against (or None). :type project_id: string :param deltas: An dictionary containing resource names as keys and requests resource quantities as positive integers. @@ -117,9 +121,11 @@ class Enforcer(object): :type deltas: dictionary :raises exception.ClaimExceedsLimit: when over limits + """ - if not project_id or not isinstance(project_id, str): - msg = 'project_id must be a non-empty string.' + if project_id is not None and ( + not project_id or not isinstance(project_id, str)): + msg = 'project_id must be a non-empty string or None.' raise ValueError(msg) if not isinstance(deltas, dict) or len(deltas) == 0: msg = 'deltas must be a non-empty dictionary.' @@ -144,15 +150,17 @@ class Enforcer(object): This should *not* be used to conduct custom enforcement, but rather only for reporting. - :param project_id: The project for which to check usage and limits. + :param project_id: The project for which to check usage and limits, + or None. :type project_id: string :param resources_to_check: A list of resource names to query. :type resources_to_check: list :returns: A dictionary of name:limit.ProjectUsage for the requested names against the provided project. """ - if not project_id or not isinstance(project_id, str): - msg = 'project_id must be a non-empty string.' + if project_id is not None and ( + not project_id or not isinstance(project_id, str)): + msg = 'project_id must be a non-empty string or None.' raise ValueError(msg) msg = ('resources_to_check must be non-empty sequence of ' @@ -253,7 +261,7 @@ class _EnforcerUtils(object): def enforce_limits(project_id, limits, current_usage, deltas): """Check that proposed usage is not over given limits - :param project_id: project being checked + :param project_id: project being checked or None :param limits: list of (resource_name,limit) pairs :param current_usage: dict of resource name and current usage :param deltas: dict of resource name and proposed additional usage @@ -283,7 +291,7 @@ class _EnforcerUtils(object): If a limit is not found, it will be considered to be zero (i.e. no quota) - :param project_id: + :param project_id: project being checked or None :param resource_names: list of resource_name strings :return: list of (resource_name,limit) pairs """ @@ -308,7 +316,8 @@ class _EnforcerUtils(object): resource_name in self.plimit_cache[project_id]): return self.plimit_cache[project_id][resource_name].resource_limit - project_limit = self._get_project_limit(project_id, resource_name) + project_limit = (self._get_project_limit(project_id, resource_name) + if project_id is not None else None) if self.should_cache and project_limit: self.plimit_cache[project_id][resource_name] = project_limit diff --git a/oslo_limit/tests/test_limit.py b/oslo_limit/tests/test_limit.py index f38108d..2c9f151 100644 --- a/oslo_limit/tests/test_limit.py +++ b/oslo_limit/tests/test_limit.py @@ -179,9 +179,6 @@ class TestEnforcer(base.BaseTestCase): enforcer = limit.Enforcer(mock.MagicMock()) # Non-string project_id - self.assertRaises(ValueError, - enforcer.calculate_usage, - None, ['foo']) self.assertRaises(ValueError, enforcer.calculate_usage, 123, ['foo']) @@ -264,6 +261,9 @@ class TestFlatEnforcer(base.BaseTestCase): self.assertRaises(exception.ProjectOverLimit, enforcer.enforce, project_id, deltas) + self.assertRaises(exception.ProjectOverLimit, enforcer.enforce, + None, deltas) + class TestEnforcerUtils(base.BaseTestCase): def setUp(self): @@ -367,3 +367,35 @@ class TestEnforcerUtils(base.BaseTestCase): def test_get_limit_no_cache(self): self.test_get_limit_cache(cache=False) + + def test_get_limit(self): + utils = limit._EnforcerUtils(cache=False) + + mgpl = mock.MagicMock() + mgrl = mock.MagicMock() + with mock.patch.multiple(utils, _get_project_limit=mgpl, + _get_registered_limit=mgrl): + # With a project, we expect the project limit to be + # fetched. If present, we never check the registered limit. + utils._get_limit('project', 'foo') + mgrl.assert_not_called() + mgpl.assert_called_once_with('project', 'foo') + + mgrl.reset_mock() + mgpl.reset_mock() + + # With a project, we expect the project limit to be + # fetched. If absent, we check the registered limit. + mgpl.return_value = None + utils._get_limit('project', 'foo') + mgrl.assert_called_once_with('foo') + mgpl.assert_called_once_with('project', 'foo') + + mgrl.reset_mock() + mgpl.reset_mock() + + # With no project, we expect to get registered limit but + # not project limit + utils._get_limit(None, 'foo') + mgrl.assert_called_once_with('foo') + mgpl.assert_not_called() -- GitLab From bf9deb10c3af4c43f6952f0fcedb9d48dfd8340d Mon Sep 17 00:00:00 2001 From: melanie witt Date: Fri, 1 Oct 2021 01:03:43 +0000 Subject: [PATCH 9/9] Add interfaces for getting limits without enforcing We currently have a public Enforcer interface for getting limits and calculating usage but it is not yet possible to retrieve only limits without calculating usage. While working on unified limits support in nova, we realized we need a way to get limits only. In nova there are legacy APIs for showing quota limits and initially we will provide compat by proxying to keystone to get the limits. This adds public interfaces for getting limits to Enforcer. Related to blueprint unified-limits-nova Change-Id: I22234e0bb6b3a1cecb29a6b99a3afcd02ffdbf5f --- oslo_limit/limit.py | 31 +++++++++++++++ oslo_limit/tests/test_limit.py | 69 ++++++++++++++++++++++++++++++++++ 2 files changed, 100 insertions(+) diff --git a/oslo_limit/limit.py b/oslo_limit/limit.py index 1393eed..6fc3941 100644 --- a/oslo_limit/limit.py +++ b/oslo_limit/limit.py @@ -181,6 +181,13 @@ class Enforcer(object): return {resource: ProjectUsage(limit, usage[resource]) for resource, limit in limits} + def get_registered_limits(self, resources_to_check): + return self.model.get_registered_limits(resources_to_check) + + def get_project_limits(self, project_id, resources_to_check): + return self.model.get_project_limits(project_id, + resources_to_check) + class _FlatEnforcer(object): @@ -190,6 +197,9 @@ class _FlatEnforcer(object): self._usage_callback = usage_callback self._utils = _EnforcerUtils(cache=cache) + def get_registered_limits(self, resources_to_check): + return self._utils.get_registered_limits(resources_to_check) + def get_project_limits(self, project_id, resources_to_check): return self._utils.get_project_limits(project_id, resources_to_check) @@ -217,6 +227,9 @@ class _StrictTwoLevelEnforcer(object): def __init__(self, usage_callback, cache=True): self._usage_callback = usage_callback + def get_registered_limits(self, resources_to_check): + raise NotImplementedError() + def get_project_limits(self, project_id, resources_to_check): raise NotImplementedError() @@ -285,6 +298,24 @@ class _EnforcerUtils(object): LOG.debug("hit limit for project: %s", over_limit_list) raise exception.ProjectOverLimit(project_id, over_limit_list) + def get_registered_limits(self, resource_names): + """Get all the default limits for a given resource name list + + :param resource_names: list of resource_name strings + :return: list of (resource_name, limit) pairs + """ + # Using a list to preserve the resource_name order + registered_limits = [] + for resource_name in resource_names: + reg_limit = self._get_registered_limit(resource_name) + if reg_limit: + limit = reg_limit.default_limit + else: + limit = 0 + registered_limits.append((resource_name, limit)) + + return registered_limits + def get_project_limits(self, project_id, resource_names): """Get all the limits for given project a resource_name list diff --git a/oslo_limit/tests/test_limit.py b/oslo_limit/tests/test_limit.py index 2c9f151..87de31d 100644 --- a/oslo_limit/tests/test_limit.py +++ b/oslo_limit/tests/test_limit.py @@ -198,6 +198,27 @@ class TestEnforcer(base.BaseTestCase): enforcer.calculate_usage, 'project', ['a', 123, 'b']) + @mock.patch.object(limit._EnforcerUtils, "get_registered_limits") + def test_get_registered_limits(self, mock_get_limits): + mock_get_limits.return_value = [("a", 1), ("b", 0), ("c", 2)] + + enforcer = limit.Enforcer(lambda: None) + limits = enforcer.get_registered_limits(["a", "b", "c"]) + + mock_get_limits.assert_called_once_with(["a", "b", "c"]) + self.assertEqual(mock_get_limits.return_value, limits) + + @mock.patch.object(limit._EnforcerUtils, "get_project_limits") + def test_get_project_limits(self, mock_get_limits): + project_id = uuid.uuid4().hex + mock_get_limits.return_value = [("a", 1), ("b", 0), ("c", 2)] + + enforcer = limit.Enforcer(lambda: None) + limits = enforcer.get_project_limits(project_id, ["a", "b", "c"]) + + mock_get_limits.assert_called_once_with(project_id, ["a", "b", "c"]) + self.assertEqual(mock_get_limits.return_value, limits) + class TestFlatEnforcer(base.BaseTestCase): def setUp(self): @@ -205,6 +226,27 @@ class TestFlatEnforcer(base.BaseTestCase): self.mock_conn = mock.MagicMock() limit._SDK_CONNECTION = self.mock_conn + @mock.patch.object(limit._EnforcerUtils, "get_registered_limits") + def test_get_registered_limits(self, mock_get_limits): + mock_get_limits.return_value = [("a", 1), ("b", 0), ("c", 2)] + + enforcer = limit._FlatEnforcer(lambda: None) + limits = enforcer.get_registered_limits(["a", "b", "c"]) + + mock_get_limits.assert_called_once_with(["a", "b", "c"]) + self.assertEqual(mock_get_limits.return_value, limits) + + @mock.patch.object(limit._EnforcerUtils, "get_project_limits") + def test_get_project_limits(self, mock_get_limits): + project_id = uuid.uuid4().hex + mock_get_limits.return_value = [("a", 1), ("b", 0), ("c", 2)] + + enforcer = limit._FlatEnforcer(lambda: None) + limits = enforcer.get_project_limits(project_id, ["a", "b", "c"]) + + mock_get_limits.assert_called_once_with(project_id, ["a", "b", "c"]) + self.assertEqual(mock_get_limits.return_value, limits) + @mock.patch.object(limit._EnforcerUtils, "get_project_limits") def test_enforce(self, mock_get_limits): mock_usage = mock.MagicMock() @@ -298,6 +340,33 @@ class TestEnforcerUtils(base.BaseTestCase): self.assertEqual(foo, reg_limit) + def test_get_registered_limits(self): + fake_endpoint = endpoint.Endpoint() + fake_endpoint.service_id = "service_id" + fake_endpoint.region_id = "region_id" + self.mock_conn.get_endpoint.return_value = fake_endpoint + + # a and c have limits, b doesn't have one + empty_iterator = iter([]) + + a = registered_limit.RegisteredLimit() + a.resource_name = "a" + a.default_limit = 1 + a_iterator = iter([a]) + + c = registered_limit.RegisteredLimit() + c.resource_name = "c" + c.default_limit = 2 + c_iterator = iter([c]) + + self.mock_conn.registered_limits.side_effect = [a_iterator, + empty_iterator, + c_iterator] + + utils = limit._EnforcerUtils() + limits = utils.get_registered_limits(["a", "b", "c"]) + self.assertEqual([('a', 1), ('b', 0), ('c', 2)], limits) + def test_get_project_limits(self): fake_endpoint = endpoint.Endpoint() fake_endpoint.service_id = "service_id" -- GitLab