From 8ad6c3bf60081ebf19c1cadd9c46e1f5bf504892 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Tue, 22 Mar 2022 16:39:42 +0000 Subject: [PATCH] [L3][QoS] Remove router QoS binding when GW is removed Now when the external gateway of a router is removed, the QoS policy assigned to this router is removed too. A router can only have a QoS policy assigned if a gateway port is attached; if the port is removed, the QoS binding must be removed too. Closes-Bug: #1968269 Change-Id: Iced254259170972a9699aa35ae8062776e11285d --- neutron/db/l3_gateway_ip_qos.py | 13 +++++++----- neutron/objects/qos/policy.py | 4 ++-- .../unit/extensions/test_qos_gateway_ip.py | 21 +++++++++++-------- 3 files changed, 22 insertions(+), 16 deletions(-) diff --git a/neutron/db/l3_gateway_ip_qos.py b/neutron/db/l3_gateway_ip_qos.py index 3eeccf82ba0..3d95de54b54 100644 --- a/neutron/db/l3_gateway_ip_qos.py +++ b/neutron/db/l3_gateway_ip_qos.py @@ -59,7 +59,7 @@ class L3_gw_ip_qos_dbonly_mixin(l3_gwmode_db.L3_NAT_dbonly_mixin): def _delete_gw_ip_qos_db(self, context, router_id, policy_id): policy = policy_object.QosPolicy.get_policy_obj(context, policy_id) - policy.detach_router(router_id) + policy.detach_router(router_id, if_exists=True) def _update_router_gw_info(self, context, router_id, info, request_body, router=None): @@ -68,10 +68,13 @@ class L3_gw_ip_qos_dbonly_mixin(l3_gwmode_db.L3_NAT_dbonly_mixin): self)._update_router_gw_info( context, router_id, info, request_body, router) + if not self._is_gw_ip_qos_supported: + return router + with db_api.CONTEXT_WRITER.using(context): - if self._is_gw_ip_qos_supported and router.gw_port: - self._update_router_gw_qos_policy(context, router_id, - info, router) + if not router.gw_port: + info = {qos_consts.QOS_POLICY_ID: None} + self._update_router_gw_qos_policy(context, router_id, info, router) return router @@ -81,7 +84,7 @@ class L3_gw_ip_qos_dbonly_mixin(l3_gwmode_db.L3_NAT_dbonly_mixin): def _update_router_gw_qos_policy(self, context, router_id, info, router): if not info or qos_consts.QOS_POLICY_ID not in info: - # An explicit 'None' for `qos_polcy_id` indicates to clear + # An explicit 'None' for `qos_policy_id` indicates to clear # the router gateway IP policy. So if info does not have # the key `qos_polcy_id`, we can not decide what behavior # to be done, then directly return here. diff --git a/neutron/objects/qos/policy.py b/neutron/objects/qos/policy.py index abb0318f6ec..aa4974add8d 100644 --- a/neutron/objects/qos/policy.py +++ b/neutron/objects/qos/policy.py @@ -295,10 +295,10 @@ class QosPolicy(rbac_db.NeutronRbacObject): raise qos_exc.FloatingIPQosBindingNotFound(fip_id=fip_id, policy_id=self.id) - def detach_router(self, router_id): + def detach_router(self, router_id, if_exists=False): deleted = binding.QosPolicyRouterGatewayIPBinding.delete_objects( self.obj_context, router_id=router_id) - if not deleted: + if not deleted and not if_exists: raise qos_exc.RouterQosBindingNotFound(router_id=router_id, policy_id=self.id) diff --git a/neutron/tests/unit/extensions/test_qos_gateway_ip.py b/neutron/tests/unit/extensions/test_qos_gateway_ip.py index a3d9ded44cd..246ed917b34 100644 --- a/neutron/tests/unit/extensions/test_qos_gateway_ip.py +++ b/neutron/tests/unit/extensions/test_qos_gateway_ip.py @@ -27,6 +27,7 @@ from oslo_utils import uuidutils from neutron.conf.db import extraroute_db from neutron.db import l3_gateway_ip_qos from neutron.extensions import l3 +from neutron.objects.qos import binding from neutron.objects.qos import policy from neutron.tests.unit.extensions import test_l3 @@ -110,8 +111,7 @@ class GatewayIPQoSDBTestCaseBase(object): res['router']['external_gateway_info'].get( qos_consts.QOS_POLICY_ID)) - def test_clear_router_gateway_and_create_with_old_qos_policy_implicitly( - self): + def test_clear_router_gateway_and_create_again(self): ctx = context.get_admin_context() policy_obj = policy.QosPolicy(ctx, id=uuidutils.generate_uuid(), @@ -129,22 +129,25 @@ class GatewayIPQoSDBTestCaseBase(object): policy_obj.id, res['router']['external_gateway_info'].get( qos_consts.QOS_POLICY_ID)) + gw_binding = binding.QosPolicyRouterGatewayIPBinding.get_object( + ctx, router_id=r['router']['id']) + self.assertEqual(r['router']['id'], gw_binding.router_id) - # Clear router gateway + # Clear router gateway, the QoS policy must be removed. self._remove_external_gateway_from_router( r['router']['id'], public_sub['subnet']['network_id'], external_gw_info={}) + gw_binding = binding.QosPolicyRouterGatewayIPBinding.get_object( + ctx, router_id=r['router']['id']) + self.assertIsNone(gw_binding) - # Create router gateway again, then the qos policy binding will be - # reused here. + # Create router gateway again. res = self._add_external_gateway_to_router( r['router']['id'], public_sub['subnet']['network_id']) - self.assertEqual( - policy_obj.id, - res['router']['external_gateway_info'].get( - qos_consts.QOS_POLICY_ID)) + self.assertIsNone(res['router']['external_gateway_info'].get( + qos_consts.QOS_POLICY_ID)) def test_clear_router_gateway_qos_policy(self): ctx = context.get_admin_context()