Add flat enforcer

Taking the Nova work as an example, looking to add a basic flat enforcer
that meets Nova's needs.

The user of the Enforce provides a callback. You can see an example of
the callback in the unit tests:

  def fake_callback(project_id, resources):
    return {
      "a": 5,
      "b": 10,
      "c": 0,
    }

In the code where you want to check if you increase the amount of
resources that are being consumed, you can make this call:

   enforcer = limit.Enforcer(fake_callback)
   enforcer.enforce(project_id, {"a": 5})

The enforce function looks up the limits that apply for the given
project_id, uses the callback to count the current usage. The proposed
usage is then calculated be adding the delta and the current usage
together. This is compared to any limits that apply.

If you later want to check if you have races that mean you are
over your limit, you can do this call:

   enforcer.enforce(project_id, {'a': 0})

Summary of key design points:

* single set of logic to enforce limits shared between all projects
  that adopt unified limits

* interface should work for both flat and strict-two-level enforcement

* it is assumed that oslo.limit will control which type of enforcement
  is being applied

* callback lists all resources that need counting
  for the given project_id, in Nova this helps limit
  the number of API calls made to placement

* allows to check if proposed additional usage means you are over
  your limit, and also double check if the current usages means
  you are over quota

* if the code is checking a resource where you do not have a
  registered limit, we default to a limit of zero, i.e. no
  resources can be created unless you set that registered limit
  There will be an appropriate warning logged to help the operator
  understand what needs to be setup in keystone.

This builds on various previous prototypes from:
Co-Authored-By: Lance Bragstad<lbragstad@gmail.com>
Co-Authored-By: wangxiyuan <wangxiyuan@huawei.com>

Change-Id: I294a922ea80af673291c991f07a4a203f25c289d

lower-constraints.txt

@@ -1,6 +1,7 @@
 bandit==1.4.0
 hacking==0.12.0
 keystoneauth1==3.9.0
+mock==3.0.0
 oslo.config==5.2.0
 oslo.i18n==3.15.3
 oslo.log==3.44.0
@@ -8,5 +9,6 @@ openstackdocstheme==1.20.0
 openstacksdk==0.31.1
 oslotest==3.2.0
 reno==2.5.0
+six==1.10.0
 Sphinx==1.8.0
 stestr==1.0.0

oslo_limit/exception.py

@@ -15,21 +15,49 @@
 from oslo_limit._i18n import _
 
 
+class ProjectOverLimit(Exception):
+    def __init__(self, project_id, over_limit_info_list):
+        """Exception raised when a project goes over one or more limits
+
+        :param project_id: the project id
+        :param over_limit_info_list: list of OverLimitInfo objects
+        """
+        if not isinstance(over_limit_info_list, list):
+            raise ValueError(over_limit_info_list)
+        if len(over_limit_info_list) == 0:
+            raise ValueError(over_limit_info_list)
+        for info in over_limit_info_list:
+            if not isinstance(info, OverLimitInfo):
+                raise ValueError(over_limit_info_list)
+
+        self.project_id = project_id
+        self.over_limit_info_list = over_limit_info_list
+        msg = _("Project %(project_id)s is over a limit for "
+                "%(limits)s") % {'project_id': project_id,
+                                 'limits': over_limit_info_list}
+        super(ProjectOverLimit, self).__init__(msg)
+
+
+class OverLimitInfo(object):
+    def __init__(self, resource_name, limit, current_usage, delta):
+        self.resource_name = resource_name
+        self.limit = int(limit)
+        self.current_usage = int(current_usage)
+        self.delta = int(delta)
+
+    def __str__(self):
+        template = ("Resource %s is over limit of %s due to "
+                    "current usage %s and delta %s")
+        return template % (self.resource_name, self.limit,
+                           self.current_usage, self.delta)
+
+    def __repr__(self):
+        return self.__str__()
+
+
 class SessionInitError(Exception):
     def __init__(self, reason):
         msg = _(
             "Can't initialise OpenStackSDK session: %(reason)s."
         ) % {'reason': reason}
         super(SessionInitError, self).__init__(msg)
-
-
-class LimitNotFound(Exception):
-    def __init__(self, resource, service, region):
-        msg = _("Can't find the limit for resource %(resource)s "
-                "for service %(service)s in region %(region)s."
-                ) % {
-            'resource': resource, 'service': service, 'region': region}
-        self.resource = resource
-        self.service = service
-        self.region = region
-        super(LimitNotFound, self).__init__(msg)

oslo_limit/limit.py

@@ -57,27 +57,25 @@ class Enforcer(object):
         :param usage_callback: A callable function that accepts a project_id
                                string as a parameter and calculates the current
                                usage of a resource.
-        :type callable function:
+        :type usage_callback: callable function
-
         """
         if not callable(usage_callback):
             msg = 'usage_callback must be a callable function.'
             raise ValueError(msg)
 
-        self.usage_callback = usage_callback
         self.connection = _get_keystone_connection()
-        self.model = self._get_model_impl()
+        self.model = self._get_model_impl(usage_callback)
 
     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):
+    def _get_model_impl(self, usage_callback):
         """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()
+                return impl(usage_callback)
         raise ValueError("enforcement model %s is not supported" % model)
 
     def enforce(self, project_id, deltas):
@@ -110,16 +108,17 @@ class Enforcer(object):
                        Specify a quantity of zero to check current usage.
         :type deltas: dictionary
 
+        :raises exception.ClaimExceedsLimit: when over limits
         """
-        if not isinstance(project_id, six.text_type):
+        if not project_id or not isinstance(project_id, six.string_types):
-            msg = 'project_id must be a string.'
+            msg = 'project_id must be a non-empty string.'
             raise ValueError(msg)
-        if not isinstance(deltas, dict):
+        if not isinstance(deltas, dict) or len(deltas) == 0:
-            msg = 'deltas must be a dictionary.'
+            msg = 'deltas must be a non-empty dictionary.'
             raise ValueError(msg)
 
-        for k, v in iter(deltas.items()):
+        for k, v in deltas.items():
-            if not isinstance(k, six.text_type):
+            if not isinstance(k, six.string_types):
                 raise ValueError('resource name is not a string.')
             elif not isinstance(v, int):
                 raise ValueError('resource limit is not an integer.')
@@ -131,14 +130,30 @@ class _FlatEnforcer(object):
 
     name = 'flat'
 
+    def __init__(self, usage_callback):
+        self._usage_callback = usage_callback
+        self._utils = _EnforcerUtils()
+
     def enforce(self, project_id, deltas):
-        raise NotImplementedError()
+        resources_to_check = list(deltas.keys())
+        # Always check the limits in the same order, for predictable errors
+        resources_to_check.sort()
+
+        project_limits = self._utils.get_project_limits(project_id,
+                                                        resources_to_check)
+        current_usage = self._usage_callback(project_id, resources_to_check)
+
+        self._utils.enforce_limits(project_id, project_limits,
+                                   current_usage, deltas)
 
 
 class _StrictTwoLevelEnforcer(object):
 
     name = 'strict-two-level'
 
+    def __init__(self, usage_callback):
+        self._usage_callback = usage_callback
+
     def enforce(self, project_id, deltas):
         raise NotImplementedError()
 
@@ -146,6 +161,14 @@ class _StrictTwoLevelEnforcer(object):
 _MODELS = [_FlatEnforcer, _StrictTwoLevelEnforcer]
 
 
+class _LimitNotFound(Exception):
+    def __init__(self, resource):
+        msg = "Can't find the limit for resource %(resource)s" % {
+            'resource': resource}
+        self.resource = resource
+        super(_LimitNotFound, self).__init__(msg)
+
+
 class _EnforcerUtils(object):
     """Logic common used by multiple enforcers"""
 
@@ -160,21 +183,61 @@ class _EnforcerUtils(object):
         self._service_id = self._endpoint.service_id
         self._region_id = self._endpoint.region_id
 
+    @staticmethod
+    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 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
+
+        :raises exception.ClaimExceedsLimit: raise if over limit
+        """
+        over_limit_list = []
+        for resource_name, limit in limits:
+            if resource_name not in current_usage:
+                msg = "unable to get current usage for %s" % resource_name
+                raise ValueError(msg)
+
+            current = int(current_usage[resource_name])
+            delta = int(deltas[resource_name])
+            proposed_usage_total = current + delta
+            if proposed_usage_total > limit:
+                over_limit_list.append(exception.OverLimitInfo(
+                    resource_name, limit, current, delta))
+
+        if len(over_limit_list) > 0:
+            LOG.debug("hit limit for project: %s", over_limit_list)
+            raise exception.ProjectOverLimit(project_id, over_limit_list)
+
     def get_project_limits(self, project_id, resource_names):
         """Get all the limits for given project a resource_name list
 
-        We will raise
+        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.
+
         :param project_id:
         :param resource_names: list of resource_name strings
         :return: list of (resource_name,limit) pairs
 
-        :raises exception.LimitNotFound if no limit is found
+        :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:
-            limit = self._get_limit(project_id, resource_name)
+            try:
-            project_limits.append((resource_name, limit))
+                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)
+
         return project_limits
 
     def _get_limit(self, project_id, resource_name):
@@ -187,8 +250,12 @@ class _EnforcerUtils(object):
         if registered_limit:
             return registered_limit.default_limit
 
-        raise exception.LimitNotFound(
+        LOG.error(
-            resource_name, self._service_id, self._region_id)
+            "Unable to find registered limit for resource "
+            "%(resource)s for %(service)s in region %(region)s.",
+            {"resource": resource_name, "service": self._service_id,
+             "region": self._region_id}, exec_info=False)
+        raise _LimitNotFound(resource_name)
 
     def _get_project_limit(self, project_id, resource_name):
         limit = self.connection.limits(

oslo_limit/tests/test_limit.py

@@ -15,7 +15,6 @@
 """
 test_limit
 ----------------------------------
-
 Tests for `limit` module.
 """
 
@@ -57,12 +56,8 @@ class TestEnforcer(base.BaseTestCase):
         json.json.return_value = {"model": {"name": "flat"}}
         limit._SDK_CONNECTION.get.return_value = json
 
-    def _get_usage_for_project(self, project_id):
+    def _get_usage_for_project(self, project_id, resource_names):
-        return 8
+        return {"a": 1}
-
-    def test_required_parameters(self):
-        enforcer = limit.Enforcer(self._get_usage_for_project)
-        self.assertEqual(self._get_usage_for_project, enforcer.usage_callback)
 
     def test_usage_callback_must_be_callable(self):
         invalid_callback_types = [uuid.uuid4().hex, 5, 5.1]
@@ -76,7 +71,7 @@ class TestEnforcer(base.BaseTestCase):
 
     def test_deltas_must_be_a_dictionary(self):
         project_id = uuid.uuid4().hex
-        invalid_delta_types = [uuid.uuid4().hex, 5, 5.1, True, False, []]
+        invalid_delta_types = [uuid.uuid4().hex, 5, 5.1, True, [], None, {}]
         enforcer = limit.Enforcer(self._get_usage_for_project)
 
         for invalid_delta in invalid_delta_types:
@@ -87,6 +82,17 @@ class TestEnforcer(base.BaseTestCase):
                 invalid_delta
             )
 
+    def test_project_id_must_be_a_string(self):
+        enforcer = limit.Enforcer(self._get_usage_for_project)
+        invalid_delta_types = [{}, 5, 5.1, True, False, [], None, ""]
+        for invalid_project_id in invalid_delta_types:
+            self.assertRaises(
+                ValueError,
+                enforcer.enforce,
+                invalid_project_id,
+                {}
+            )
+
     def test_set_model_impl(self):
         enforcer = limit.Enforcer(self._get_usage_for_project)
         self.assertIsInstance(enforcer.model, limit._FlatEnforcer)
@@ -97,17 +103,88 @@ class TestEnforcer(base.BaseTestCase):
 
         json.json.return_value = {"model": {"name": "flat"}}
         enforcer = limit.Enforcer(self._get_usage_for_project)
-        flat_impl = enforcer._get_model_impl()
+        flat_impl = enforcer._get_model_impl(self._get_usage_for_project)
         self.assertIsInstance(flat_impl, limit._FlatEnforcer)
 
         json.json.return_value = {"model": {"name": "strict-two-level"}}
-        flat_impl = enforcer._get_model_impl()
+        flat_impl = enforcer._get_model_impl(self._get_usage_for_project)
         self.assertIsInstance(flat_impl, limit._StrictTwoLevelEnforcer)
 
         json.json.return_value = {"model": {"name": "foo"}}
-        e = self.assertRaises(ValueError, enforcer._get_model_impl)
+        e = self.assertRaises(ValueError, enforcer._get_model_impl,
+                              self._get_usage_for_project)
         self.assertEqual("enforcement model foo is not supported", str(e))
 
+    @mock.patch.object(limit._FlatEnforcer, "enforce")
+    def test_enforce(self, mock_enforce):
+        enforcer = limit.Enforcer(self._get_usage_for_project)
+        project_id = uuid.uuid4().hex
+        deltas = {"a": 1}
+
+        enforcer.enforce(project_id, deltas)
+
+        mock_enforce.assert_called_once_with(project_id, deltas)
+
+
+class TestFlatEnforcer(base.BaseTestCase):
+    def setUp(self):
+        super(TestFlatEnforcer, self).setUp()
+        self.mock_conn = mock.MagicMock()
+        limit._SDK_CONNECTION = self.mock_conn
+
+    @mock.patch.object(limit._EnforcerUtils, "get_project_limits")
+    def test_enforce(self, mock_get_limits):
+        mock_usage = mock.MagicMock()
+
+        project_id = uuid.uuid4().hex
+        deltas = {"a": 1, "b": 1}
+        mock_get_limits.return_value = [("a", 1), ("b", 2)]
+        mock_usage.return_value = {"a": 0, "b": 1}
+
+        enforcer = limit._FlatEnforcer(mock_usage)
+        enforcer.enforce(project_id, deltas)
+
+        self.mock_conn.get_endpoint.assert_called_once_with(None)
+        mock_get_limits.assert_called_once_with(project_id, ["a", "b"])
+        mock_usage.assert_called_once_with(project_id, ["a", "b"])
+
+    @mock.patch.object(limit._EnforcerUtils, "get_project_limits")
+    def test_enforce_raises_on_over(self, mock_get_limits):
+        mock_usage = mock.MagicMock()
+
+        project_id = uuid.uuid4().hex
+        deltas = {"a": 2, "b": 1}
+        mock_get_limits.return_value = [("a", 1), ("b", 2)]
+        mock_usage.return_value = {"a": 0, "b": 1}
+
+        enforcer = limit._FlatEnforcer(mock_usage)
+        e = self.assertRaises(exception.ProjectOverLimit, enforcer.enforce,
+                              project_id, deltas)
+        expected = ("Project %s is over a limit for "
+                    "[Resource a is over limit of 1 due to current usage 0 "
+                    "and delta 2]")
+        self.assertEqual(expected % project_id, str(e))
+        self.assertEqual(project_id, e.project_id)
+        self.assertEqual(1, len(e.over_limit_info_list))
+        over_a = e.over_limit_info_list[0]
+        self.assertEqual("a", over_a.resource_name)
+        self.assertEqual(1, over_a.limit)
+        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()
+
+        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)])
+
+        enforcer = limit._FlatEnforcer(mock_usage)
+        self.assertRaises(exception.ProjectOverLimit, enforcer.enforce,
+                          project_id, deltas)
+
 
 class TestEnforcerUtils(base.BaseTestCase):
     def setUp(self):
@@ -149,30 +226,43 @@ class TestEnforcerUtils(base.BaseTestCase):
         self.mock_conn.get_endpoint.return_value = fake_endpoint
         project_id = uuid.uuid4().hex
 
-        # a is a project limit, b and c don't have one
+        # a is a project limit, b, c and d don't have one
         empty_iterator = iter([])
         a = klimit.Limit()
         a.resource_name = "a"
         a.resource_limit = 1
         a_iterator = iter([a])
         self.mock_conn.limits.side_effect = [a_iterator, empty_iterator,
-                                             empty_iterator]
+                                             empty_iterator, empty_iterator]
 
-        # b has a limit, but c doesn't, a isn't ever checked
+        # b has a limit, but c and d doesn't, a isn't ever checked
         b = registered_limit.RegisteredLimit()
         b.resource_name = "b"
         b.default_limit = 2
         b_iterator = iter([b])
         self.mock_conn.registered_limits.side_effect = [b_iterator,
+                                                        empty_iterator,
                                                         empty_iterator]
 
         utils = limit._EnforcerUtils()
         limits = utils.get_project_limits(project_id, ["a", "b"])
         self.assertEqual([('a', 1), ('b', 2)], limits)
 
-        e = self.assertRaises(exception.LimitNotFound,
+        e = self.assertRaises(exception.ProjectOverLimit,
                               utils.get_project_limits,
-                              project_id, ["c"])
+                              project_id, ["c", "d"])
-        self.assertEqual("c", e.resource)
+        expected = ("Project %s is over a limit for "
-        self.assertEqual("service_id", e.service)
+                    "[Resource c is over limit of 0 due to current usage 0 "
-        self.assertEqual("region_id", e.region)
+                    "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)

requirements.txt

@@ -2,6 +2,7 @@
 # of appearance. Changing the order has an impact on the overall integration
 # process, which may cause wedges in the gate later.
 keystoneauth1>=3.9.0 # Apache-2.0
+six>=1.10.0 # MIT
 oslo.config>=5.2.0 # Apache-2.0
 oslo.i18n>=3.15.3 # Apache-2.0
 oslo.log>=3.44.0 # Apache-2.0

test-requirements.txt

@@ -2,6 +2,7 @@
 # of appearance. Changing the order has an impact on the overall integration
 # process, which may cause wedges in the gate later.
 
+mock>=3.0.0 # BSD
 hacking>=1.1.0,<1.2.0 # Apache-2.0
 oslotest>=3.2.0 # Apache-2.0
 stestr>=1.0.0 # Apache-2.0