From a859760323a878ced58c2c01b6da8d0f803028be Mon Sep 17 00:00:00 2001
From: Rodolfo Alonso Hernandez <ralonsoh@redhat.com>
Date: Thu, 8 Aug 2024 18:23:28 +0000
Subject: [PATCH] Neutron quota engine checks the resource usage by default

Now the Neutron quota engine always checks the current resource usage
before updating the quota limits. Only when the CLI "--force"
parameter is passed, this check is skipped. That aligns the Neutron
quota engine behaviour with other projects.

The Neutron quota commands now always check the resource limits. The
CLI parameter "--check-limits" is no longer needed, as this is the
default behaviour.

Depends-On: https://review.opendev.org/c/openstack/neutron-tempest-plugin/+/928770

Closes-Bug: #1953170
Change-Id: I2a9cd89cfe40ef635892cefeb61264272fe7bf16
---
 neutron/common/ovn/extensions.py              |  2 +
 .../extensions/quota_check_limit_default.py   | 66 +++++++++++++++++++
 neutron/extensions/quotasv2.py                | 25 +++----
 neutron/plugins/ml2/plugin.py                 |  2 +
 .../tests/contrib/hooks/api_all_extensions    |  1 +
 .../tests/unit/extensions/test_quotasv2.py    | 10 +--
 ...hecks-resource-usage-6e5e18f5c8f4b725.yaml | 12 ++++
 7 files changed, 99 insertions(+), 19 deletions(-)
 create mode 100644 neutron/extensions/quota_check_limit_default.py
 create mode 100644 releasenotes/notes/quota-always-checks-resource-usage-6e5e18f5c8f4b725.yaml

diff --git a/neutron/common/ovn/extensions.py b/neutron/common/ovn/extensions.py
index 41bda33b25d..0ad6ccf25da 100644
--- a/neutron/common/ovn/extensions.py
+++ b/neutron/common/ovn/extensions.py
@@ -98,6 +98,7 @@ from neutron_lib.api.definitions import vpn_endpoint_groups
 from neutron_lib import constants
 
 from neutron.extensions import port_trusted_vif
+from neutron.extensions import quota_check_limit_default
 from neutron.extensions import quotasv2_detail
 from neutron.extensions import security_groups_default_rules
 
@@ -171,6 +172,7 @@ ML2_SUPPORTED_API_EXTENSIONS = [
     qos_rules_alias.ALIAS,
     'quotas',
     quota_check_limit.ALIAS,
+    quota_check_limit_default.ALIAS,
     quotasv2_detail.ALIAS,
     rbac_address_scope.ALIAS,
     'rbac-policies',
diff --git a/neutron/extensions/quota_check_limit_default.py b/neutron/extensions/quota_check_limit_default.py
new file mode 100644
index 00000000000..b603c60568b
--- /dev/null
+++ b/neutron/extensions/quota_check_limit_default.py
@@ -0,0 +1,66 @@
+# Copyright (c) 2024 Red Hat, Inc.
+#
+#    Licensed under the Apache License, Version 2.0 (the "License"); you may
+#    not use this file except in compliance with the License. You may obtain
+#    a copy of the License at
+#
+#         http://www.apache.org/licenses/LICENSE-2.0
+#
+#    Unless required by applicable law or agreed to in writing, software
+#    distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+#    WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+#    License for the specific language governing permissions and limitations
+#    under the License.
+
+from neutron_lib.api.definitions import quota_check_limit
+from neutron_lib.api import extensions
+
+
+# NOTE(ralonsoh): once [1] is merged, use
+# ``neutron_lib.api.definitions.quota_check_limit_default`` instead.
+# [1] https://review.opendev.org/c/openstack/neutron-lib/+/926777
+ALIAS = 'quota-check-limit-default'
+IS_SHIM_EXTENSION = True
+IS_STANDARD_ATTR_EXTENSION = False
+NAME = 'Quota engine limit check by default'
+DESCRIPTION = ('By default, the Neutron quota engine checks the resource '
+               'usage before applying a new quota limit')
+UPDATED_TIMESTAMP = '2024-08-21T16:00:00-00:00'
+RESOURCE_ATTRIBUTE_MAP = {}
+SUB_RESOURCE_ATTRIBUTE_MAP = {}
+ACTION_MAP = {}
+REQUIRED_EXTENSIONS = [quota_check_limit.ALIAS]
+OPTIONAL_EXTENSIONS = []
+ACTION_STATUS = {}
+
+
+class Quota_check_limit_default(extensions.APIExtensionDescriptor):
+
+    @classmethod
+    def get_name(cls):
+        return NAME
+
+    @classmethod
+    def get_alias(cls):
+        return ALIAS
+
+    @classmethod
+    def get_description(cls):
+        return DESCRIPTION
+
+    @classmethod
+    def get_updated(cls):
+        return UPDATED_TIMESTAMP
+
+    def get_required_extensions(self):
+        return REQUIRED_EXTENSIONS
+
+    def get_optional_extensions(self):
+        return []
+
+    @classmethod
+    def get_extended_resources(cls, version):
+        if version == "2.0":
+            return RESOURCE_ATTRIBUTE_MAP
+        else:
+            return {}
diff --git a/neutron/extensions/quotasv2.py b/neutron/extensions/quotasv2.py
index b18f37e7368..accf5438a8a 100644
--- a/neutron/extensions/quotasv2.py
+++ b/neutron/extensions/quotasv2.py
@@ -131,20 +131,11 @@ class QuotaSetsController(wsgi.Controller):
     def update(self, request, id, body=None):
         validate_policy(request.context, "update_quota")
         force = body[self._resource_name].pop('force', None)
-        check_limit = body[self._resource_name].pop('check_limit', None)
-        # NOTE(ralonsoh): these warning messages will be removed once
-        # LP#1953170 is completed and Neutron quota engine accepts "--force" or
-        # nothing (by default, Neutron quota engine will check the resource
-        # usage before setting the quota limit).
-        if force is None and check_limit is None:
-            warnings.warn('Neutron quota engine will require "--force" '
-                          'parameter to set a quota limit without checking '
-                          'the resource usage.')
-        elif check_limit:
-            warnings.warn('"--check-limit" parameter will not be needed in '
-                          'Z+. By default, Neutron quota engine will check '
-                          'the resource usage before setting a new quota '
-                          'limit. Use "--force" to skip this check.')
+        if body[self._resource_name].pop('check_limit', None):
+            warnings.warn('"--check-limit" parameter is no longer needed '
+                          'since Epoxy (2025.1) release. By default, '
+                          'Neutron quota engine checks the resource usage '
+                          'before updating the limits')
 
         if self._update_extended_attributes:
             self._update_attributes()
@@ -158,9 +149,13 @@ class QuotaSetsController(wsgi.Controller):
                 "body. The exception message is [%s].", e)
             raise e
 
-        if check_limit:
+        if force is not True:
             resources = resource_registry.get_all_resources()
             for resource_name, limit in body[self._resource_name].items():
+                if limit == -1:
+                    # limit=-1 is disabling the quota thus it is not needed to
+                    # check the resource usage.
+                    continue
                 resource_usage = self._driver.get_resource_usage(
                     request.context, id, resources, resource_name)
                 if resource_usage > limit:
diff --git a/neutron/plugins/ml2/plugin.py b/neutron/plugins/ml2/plugin.py
index 5a1bd7b370f..99ca3d93b73 100644
--- a/neutron/plugins/ml2/plugin.py
+++ b/neutron/plugins/ml2/plugin.py
@@ -134,6 +134,7 @@ from neutron.db import subnet_service_type_mixin
 from neutron.db import vlantransparent_db
 from neutron.extensions import dhcpagentscheduler as dhcp_ext
 from neutron.extensions import filter_validation
+from neutron.extensions import quota_check_limit_default
 from neutron.extensions import security_groups_default_rules as \
         sg_default_rules_ext
 from neutron.extensions import vlantransparent
@@ -247,6 +248,7 @@ class Ml2Plugin(db_base_plugin_v2.NeutronDbPluginV2,
                                     pnaps_def.ALIAS,
                                     pdp_def.ALIAS,
                                     quota_check_limit.ALIAS,
+                                    quota_check_limit_default.ALIAS,
                                     port_mac_address_override.ALIAS,
                                     sg_default_rules_ext.ALIAS,
                                     sg_rules_default_sg.ALIAS,
diff --git a/neutron/tests/contrib/hooks/api_all_extensions b/neutron/tests/contrib/hooks/api_all_extensions
index 6bbb6266405..7cbaae210e2 100644
--- a/neutron/tests/contrib/hooks/api_all_extensions
+++ b/neutron/tests/contrib/hooks/api_all_extensions
@@ -46,6 +46,7 @@ NETWORK_API_EXTENSIONS+=",qos-fip"
 NETWORK_API_EXTENSIONS+=",qos-gateway-ip"
 NETWORK_API_EXTENSIONS+=",quotas"
 NETWORK_API_EXTENSIONS+=",quota-check-limit"
+NETWORK_API_EXTENSIONS+=",quota-check-limit-default"
 NETWORK_API_EXTENSIONS+=",quota_details"
 NETWORK_API_EXTENSIONS+=",rbac-policies"
 NETWORK_API_EXTENSIONS+=",rbac-address-group"
diff --git a/neutron/tests/unit/extensions/test_quotasv2.py b/neutron/tests/unit/extensions/test_quotasv2.py
index c5d914a944f..d7072fd44db 100644
--- a/neutron/tests/unit/extensions/test_quotasv2.py
+++ b/neutron/tests/unit/extensions/test_quotasv2.py
@@ -291,7 +291,7 @@ class QuotaExtensionDbTestCase(QuotaExtensionTestCase):
     def test_update_attributes(self):
         project_id = 'project_id1'
         env = test_base._get_neutron_env(project_id + '2', as_admin=True)
-        quotas = {'quota': {'extra1': 100}}
+        quotas = {'quota': {'extra1': 100, 'force': True}}
         res = self.api.put(_get_path('quotas', id=project_id, fmt=self.fmt),
                            self.serialize(quotas), extra_environ=env)
         self.assertEqual(200, res.status_int)
@@ -302,16 +302,18 @@ class QuotaExtensionDbTestCase(QuotaExtensionTestCase):
         self.assertEqual(100, quota['quota']['extra1'])
 
     @mock.patch.object(driver_nolock.DbQuotaNoLockDriver, 'get_resource_usage')
-    def test_update_quotas_check_limit(self, mock_get_resource_usage):
+    def test_update_quotas_force(self, mock_get_resource_usage):
         tenant_id = 'tenant_id1'
         env = test_base._get_neutron_env(tenant_id, as_admin=True)
-        quotas = {'quota': {'network': 100, 'check_limit': False}}
+        # force=True; no resource usage check
+        quotas = {'quota': {'network': 100, 'force': True}}
         res = self.api.put(_get_path('quotas', id=tenant_id, fmt=self.fmt),
                            self.serialize(quotas), extra_environ=env,
                            expect_errors=False)
         self.assertEqual(200, res.status_int)
 
-        quotas = {'quota': {'network': 50, 'check_limit': True}}
+        # force=False; before the quota is set, there is a resource usage check
+        quotas = {'quota': {'network': 50}}  # force=False by default
         mock_get_resource_usage.return_value = 51
         res = self.api.put(_get_path('quotas', id=tenant_id, fmt=self.fmt),
                            self.serialize(quotas), extra_environ=env,
diff --git a/releasenotes/notes/quota-always-checks-resource-usage-6e5e18f5c8f4b725.yaml b/releasenotes/notes/quota-always-checks-resource-usage-6e5e18f5c8f4b725.yaml
new file mode 100644
index 00000000000..c56ed52a47d
--- /dev/null
+++ b/releasenotes/notes/quota-always-checks-resource-usage-6e5e18f5c8f4b725.yaml
@@ -0,0 +1,12 @@
+---
+features:
+  - |
+    Since Epoxy (2025.1) release, the Neutron quota engine always checks
+    the current resource usage before updating the quota limits. Only when the
+    CLI "--force" parameter is passed, this check is skipped. That aligns the
+    Neutron quota engine behaviour with other projects.
+deprecations:
+  - |
+    The Neutron quota commands now always check the resource limits. The CLI
+    parameter "--check-limits" is no longer needed, as this is the default
+    behaviour.