diff --git a/neutron/common/utils.py b/neutron/common/utils.py index 84f5d9be062..ec6067eb911 100644 --- a/neutron/common/utils.py +++ b/neutron/common/utils.py @@ -38,6 +38,7 @@ from neutron_lib.api.definitions import availability_zone as az_def from neutron_lib import constants as n_const from neutron_lib import context as n_context from neutron_lib.db import api as db_api +from neutron_lib.services.qos import constants as qos_consts from neutron_lib.services.trunk import constants as trunk_constants from neutron_lib.utils import helpers from oslo_config import cfg @@ -1046,3 +1047,15 @@ def is_session_active(session): if not (session.dirty or session.deleted or session.new): return False return True + + +def effective_qos_policy_id(resource): + """Return the resource effective QoS policy + + If the resource does not have any QoS policy reference, returns the + QoS policy inherited from the network (if exists). + :param resource: [dict] resource with QoS policies + :return: [str] resource QoS policy ID or network QoS policy ID + """ + return (resource.get(qos_consts.QOS_POLICY_ID) or + resource.get(qos_consts.QOS_NETWORK_POLICY_ID)) diff --git a/neutron/db/l3_gateway_ip_qos.py b/neutron/db/l3_gateway_ip_qos.py index 3d95de54b54..0d90bd4090b 100644 --- a/neutron/db/l3_gateway_ip_qos.py +++ b/neutron/db/l3_gateway_ip_qos.py @@ -39,11 +39,17 @@ class L3_gw_ip_qos_dbonly_mixin(l3_gwmode_db.L3_NAT_dbonly_mixin): @staticmethod @resource_extend.extends([l3_apidef.ROUTERS]) def _extend_router_dict_gw_qos(router_res, router_db): - if router_db.gw_port_id and router_db.get('qos_policy_binding'): - policy_id = router_db.qos_policy_binding.policy_id - router_res[l3_apidef.EXTERNAL_GW_INFO].update( - {qos_consts.QOS_POLICY_ID: policy_id}) + if router_db.gw_port and ( + router_db.qos_policy_binding or + router_db.gw_port.qos_network_policy_binding): + qos_bind = router_db.qos_policy_binding + qos_net_bind = router_db.gw_port.qos_network_policy_binding + policy_id = qos_bind.policy_id if qos_bind else None + net_policy_id = qos_net_bind.policy_id if qos_net_bind else None + router_res[l3_apidef.EXTERNAL_GW_INFO].update({ + qos_consts.QOS_POLICY_ID: policy_id}) router_res[qos_consts.QOS_POLICY_ID] = policy_id + router_res[qos_consts.QOS_NETWORK_POLICY_ID] = net_policy_id @property def _is_gw_ip_qos_supported(self): diff --git a/neutron/objects/qos/binding.py b/neutron/objects/qos/binding.py index 61c332d6797..8cf0c926664 100644 --- a/neutron/objects/qos/binding.py +++ b/neutron/objects/qos/binding.py @@ -141,3 +141,26 @@ class QosPolicyRouterGatewayIPBinding(base.NeutronDbObject, primary_keys = ['policy_id', 'router_id'] fields_no_update = ['policy_id', 'router_id'] _bound_model_id = db_model.router_id + + @classmethod + @db_api.CONTEXT_READER + def get_routers_by_network_id(cls, context, network_id, policy_id=None): + """Return the routers that have a network as GW, filtered by QoS policy + + This method returns the routers that have a gateway port on this + network. If "policy_id" is defined, it will return those routers that + have a gateway IP QoS policy associated. If "policy_id" is None, this + method will return only those routers that doesn't have any gateway + IP QoS policy associated. + """ + query = context.session.query(models_l3.Router).filter( + models_l3.Router.gw_port_id == models_v2.Port.id, + models_v2.Port.network_id == network_id) + if policy_id: + query = query.filter(exists().where(and_( + cls.db_model.router_id == models_l3.Router.id, + cls.db_model.policy_id == policy_id))) + else: + query = query.filter(~exists().where( + cls.db_model.router_id == models_l3.Router.id)) + return query.all() diff --git a/neutron/objects/qos/policy.py b/neutron/objects/qos/policy.py index aa4974add8d..9dcd255fe08 100644 --- a/neutron/objects/qos/policy.py +++ b/neutron/objects/qos/policy.py @@ -337,7 +337,7 @@ class QosPolicy(rbac_db.NeutronRbacObject): self.obj_context, self.id) def get_bound_routers(self): - return binding.QosPolicyRouterGatewayIPBinding.get_objects( + return binding.QosPolicyRouterGatewayIPBinding.get_bound_ids( self.obj_context, policy_id=self.id) @classmethod diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/qos.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/qos.py index 194a9b2ebc4..edac787735c 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/qos.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/qos.py @@ -25,6 +25,7 @@ from oslo_log import log as logging from neutron.common.ovn import constants as ovn_const from neutron.common.ovn import utils +from neutron.common import utils as n_utils from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf @@ -199,9 +200,8 @@ class OVNClientQosExtension(object): If the port does not have any QoS policy reference or is a network device, then return None. """ - policy_exists = bool(port.get('qos_policy_id') or - port.get('qos_network_policy_id')) - if not policy_exists or utils.is_network_device_port(port): + policy_id = n_utils.effective_qos_policy_id(port) + if not policy_id or utils.is_network_device_port(port): return None, None if port.get('qos_policy_id'): @@ -282,17 +282,18 @@ class OVNClientQosExtension(object): qos_rules=None): updated_port_ids = set([]) updated_fip_ids = set([]) + updated_router_ids = set([]) if not reset and not original_network: # If there is no information about the previous QoS policy, do not # make any change. - return updated_port_ids, updated_fip_ids + return updated_port_ids, updated_fip_ids, updated_router_ids qos_policy_id = network.get('qos_policy_id') if not reset: original_qos_policy_id = original_network.get('qos_policy_id') if qos_policy_id == original_qos_policy_id: # No QoS policy change - return updated_port_ids, updated_fip_ids + return updated_port_ids, updated_fip_ids, updated_router_ids # NOTE(ralonsoh): we don't use the transaction context because some # ports can belong to other projects. @@ -315,7 +316,13 @@ class OVNClientQosExtension(object): self.update_floatingip(txn, floatingip) updated_fip_ids.add(floatingip['id']) - return updated_port_ids, updated_fip_ids + for router in (qos_binding.QosPolicyRouterGatewayIPBinding. + get_routers_by_network_id(admin_context, network['id'])): + router_dict = self._plugin_l3._make_router_dict(router) + self.update_router(txn, router_dict) + updated_router_ids.add(router.id) + + return updated_port_ids, updated_fip_ids, updated_router_ids def _delete_fip_qos_rules(self, txn, fip_id, network_id): if network_id: @@ -385,7 +392,7 @@ class OVNClientQosExtension(object): def update_router(self, txn, router): gw_info = router.get(l3_api.EXTERNAL_GW_INFO) or {} - qos_policy_id = router.get('qos_policy_id') + qos_policy_id = n_utils.effective_qos_policy_id(router) router_id = router.get('id') gw_port_id = router.get('gw_port_id') gw_network_id = gw_info.get('network_id') @@ -393,7 +400,8 @@ class OVNClientQosExtension(object): # NOTE(ralonsoh): when the gateway network is detached, the gateway # port is deleted. Any QoS policy related to this port_id is # deleted in "self.update_port()". - LOG.debug('Router %s does not have ID or gateway assigned', router) + LOG.debug('Router %s does not have ID or gateway assigned', + router_id) return admin_context = n_context.get_admin_context() @@ -419,9 +427,11 @@ class OVNClientQosExtension(object): def update_policy(self, context, policy): updated_port_ids = set([]) updated_fip_ids = set([]) + updated_router_ids = set([]) bound_networks = policy.get_bound_networks() bound_ports = policy.get_bound_ports() bound_fips = policy.get_bound_floatingips() + bound_routers = policy.get_bound_routers() qos_rules = self._qos_rules(context, policy.id) # TODO(ralonsoh): we need to benchmark this transaction in systems with # a huge amount of ports. This can take a while and could block other @@ -429,10 +439,11 @@ class OVNClientQosExtension(object): with self.nb_idl.transaction(check_error=True) as txn: for network_id in bound_networks: network = {'qos_policy_id': policy.id, 'id': network_id} - port_ids, fip_ids = self.update_network( + port_ids, fip_ids, router_ids = self.update_network( txn, network, {}, reset=True, qos_rules=qos_rules) updated_port_ids.update(port_ids) updated_fip_ids.update(fip_ids) + updated_router_ids.update(router_ids) # Update each port bound to this policy, not handled previously in # the network update loop @@ -451,7 +462,9 @@ class OVNClientQosExtension(object): context, filters={'id': fip_ids}): self.update_floatingip(txn, fip) - for router_binding in policy.get_bound_routers(): - router = self._plugin_l3.get_router(context, - router_binding.router_id) - self.update_router(txn, router) + router_ids = [r for r in bound_routers if + r not in updated_router_ids] + if router_ids: + for router in self._plugin_l3.get_routers( + context, filters={'id': router_ids}): + self.update_router(txn, router) diff --git a/neutron/tests/unit/extensions/test_l3_ext_gw_mode.py b/neutron/tests/unit/extensions/test_l3_ext_gw_mode.py index 3e2e617b02a..9c941738ab9 100644 --- a/neutron/tests/unit/extensions/test_l3_ext_gw_mode.py +++ b/neutron/tests/unit/extensions/test_l3_ext_gw_mode.py @@ -471,7 +471,7 @@ class ExtGwModeIntTestCase(test_db_base_plugin_v2.NeutronDbPluginV2TestCase, external_gateway_info=input_value) as router: res = self._show('routers', router['router']['id']) for k, v in expected_value: - self.assertEqual(res['router'][k], v) + self.assertEqual(v, res['router'][k]) def test_router_create_show_ext_gwinfo_default(self): self._test_router_create_show_ext_gwinfo(None, True) diff --git a/neutron/tests/unit/objects/qos/test_binding.py b/neutron/tests/unit/objects/qos/test_binding.py index b8be633fcb5..f3bbb44387a 100644 --- a/neutron/tests/unit/objects/qos/test_binding.py +++ b/neutron/tests/unit/objects/qos/test_binding.py @@ -11,6 +11,7 @@ # under the License. from neutron.objects.qos import binding +from neutron.objects import router from neutron.tests.unit.objects import test_base from neutron.tests.unit import testlib_api @@ -102,3 +103,76 @@ class QosPolicyRouterGatewayIPBindingDbObjectTestCase( for db_obj in self.db_objs: self._create_test_qos_policy(id=db_obj['policy_id']) self._create_test_router_id(router_id=db_obj['router_id']) + + def test_get_routers_by_network_id(self): + qos_policy_router_obj = self._create_test_qos_policy() + qos_policy_net_obj = self._create_test_qos_policy() + # External network 1, no QoS policy + ext_network_id_1 = self._create_external_network_id() + gw_port_id_1a = self._create_test_port_id(network_id=ext_network_id_1) + gw_port_id_1b = self._create_test_port_id(network_id=ext_network_id_1) + # External network 2, "qos_policy_network" assigned + ext_network_id_2 = self._create_external_network_id( + qos_policy_id=qos_policy_net_obj.id) + gw_port_id_2a = self._create_test_port_id(network_id=ext_network_id_2) + gw_port_id_2b = self._create_test_port_id(network_id=ext_network_id_2) + + # Router 1: no GW + self._create_test_router_id(name='router1') + + # Router 2: GW assigned, no router QoS, not public network QoS + router2 = self._create_test_router_id(name='router2') + router2_obj = router.Router.get_object(self.context, id=router2) + router2_obj.gw_port_id = gw_port_id_1a + router2_obj.update() + + # Router 3: GW assigned, router QoS, not public network QoS + router3 = self._create_test_router_id(name='router3') + router3_obj = router.Router.get_object(self.context, id=router3) + router3_obj.gw_port_id = gw_port_id_1b + router3_obj.qos_policy_id = qos_policy_router_obj.id + router3_obj.update() + + # Router 4: GW assigned, no router QoS, public network with QoS + router4 = self._create_test_router_id(name='router4') + router4_obj = router.Router.get_object(self.context, id=router4) + router4_obj.gw_port_id = gw_port_id_2a + router4_obj.update() + + # Router 5: GW assigned, router QoS, public network with QoS + router5 = self._create_test_router_id(name='router5') + router5_obj = router.Router.get_object(self.context, id=router5) + router5_obj.gw_port_id = gw_port_id_2b + router5_obj.qos_policy_id = qos_policy_router_obj.id + router5_obj.update() + + # Check that only router3 and router5 have + # "QosPolicyRouterGatewayIPBinding" related registers. + qos_gw_binds = self._test_class.get_objects(self.context) + self.assertEqual(2, len(qos_gw_binds)) + router_ids = [qos_gw_bind.router_id for qos_gw_bind in qos_gw_binds] + self.assertEqual(sorted([router3, router5]), sorted(router_ids)) + + result = self._test_class.get_routers_by_network_id( + self.context, ext_network_id_1, policy_id=None) + self.assertEqual([router2], [r.id for r in result]) + + result = self._test_class.get_routers_by_network_id( + self.context, ext_network_id_1, policy_id=qos_policy_router_obj.id) + self.assertEqual([router3], [r.id for r in result]) + + result = self._test_class.get_routers_by_network_id( + self.context, ext_network_id_1, policy_id=qos_policy_net_obj.id) + self.assertEqual([], [r.id for r in result]) + + result = self._test_class.get_routers_by_network_id( + self.context, ext_network_id_2, policy_id=None) + self.assertEqual([router4], [r.id for r in result]) + + result = self._test_class.get_routers_by_network_id( + self.context, ext_network_id_2, policy_id=qos_policy_router_obj.id) + self.assertEqual([router5], [r.id for r in result]) + + result = self._test_class.get_routers_by_network_id( + self.context, ext_network_id_2, policy_id=qos_policy_net_obj.id) + self.assertEqual([], [r.id for r in result]) diff --git a/neutron/tests/unit/objects/test_base.py b/neutron/tests/unit/objects/test_base.py index 64d7852c495..bb560a1207b 100644 --- a/neutron/tests/unit/objects/test_base.py +++ b/neutron/tests/unit/objects/test_base.py @@ -1555,12 +1555,14 @@ class BaseDbObjectTestCase(_BaseObjectTestCase, _network.create() return _network - def _create_test_network_id(self): + def _create_test_network_id(self, qos_policy_id=None): return self._create_test_network( - "test-network-%s" % helpers.get_random_string(4)).id + name="test-network-%s" % helpers.get_random_string(4), + qos_policy_id=qos_policy_id).id - def _create_external_network_id(self): - test_network_id = self._create_test_network_id() + def _create_external_network_id(self, qos_policy_id=None): + test_network_id = self._create_test_network_id( + qos_policy_id=qos_policy_id) ext_net = net_obj.ExternalNetwork(self.context, network_id=test_network_id) ext_net.create() @@ -1639,9 +1641,9 @@ class BaseDbObjectTestCase(_BaseObjectTestCase, segment.create() return segment.id - def _create_test_router_id(self, router_id=None): + def _create_test_router_id(self, router_id=None, name=None): attrs = { - 'name': 'test_router', + 'name': name or 'test_router', } if router_id: attrs['id'] = router_id diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py index 4174f2b1ee9..3f1534716d5 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/extensions/test_qos.py @@ -128,10 +128,12 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): router.create() return router, network - def _update_router_qos(self, router_id, qos_policy_id, attach=True): + @db_api.CONTEXT_WRITER + def _update_router_qos(self, context, router_id, qos_policy_id, + attach=True): # NOTE(ralonsoh): router QoS policy is not yet implemented in Router # OVO. Once we have this feature, this method can be removed. - qos = policy_obj.QosPolicy.get_policy_obj(self.ctx, qos_policy_id) + qos = policy_obj.QosPolicy.get_policy_obj(context, qos_policy_id) if attach: qos.attach_router(router_id) else: @@ -375,7 +377,7 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): mock.ANY, self.ports[1].id, self.ports[1].network_id, None, None) def test_update_network(self): - """Test update network. + """Test update network (internal ports). net1: [(1) from qos_policy0 to no QoS policy, (2) from qos_policy0 to qos_policy1] @@ -395,7 +397,7 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): self.networks[0].qos_policy_id = qos_policy_id self.networks[0].update() original_network = {'qos_policy_id': self.qos_policies[0]} - reviewed_port_ids, _ = self.qos_driver.update_network( + reviewed_port_ids, _, _ = self.qos_driver.update_network( mock.ANY, self.networks[0], original_network) self.assertEqual(reference_ports, reviewed_port_ids) calls = [mock.call(mock.ANY, self.ports[0].id, @@ -405,24 +407,30 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): self.mock_rules.reset_mock() def test_update_external_network(self): - """Test update external network (floating IPs). + """Test update external network (floating IPs and GW IPs). - fip0: qos_policy0 - fip1: no QoS FIP policy (inherits from external network QoS) + - router_fips: no QoS FIP policy (inherits from external network QoS) """ - network_policies = [ - (self.qos_policies[1].id, {self.fips[1].id}), - (None, {self.fips[1].id})] + network_policies = [(self.qos_policies[1].id, + {self.fips[1].id}, + {self.router_fips.id}), + (None, + {self.fips[1].id}, + {self.router_fips.id})] self.fips[0].qos_policy_id = self.qos_policies[0].id self.fips[0].update() - for qos_policy_id, reference_fips in network_policies: + for qos_policy_id, ref_fips, ref_routers in network_policies: self.fips_network.qos_policy_id = qos_policy_id self.fips_network.update() original_network = {'qos_policy_id': self.qos_policies[0]} - _, reviewed_fips_ids = self.qos_driver.update_network( - mock.Mock(), self.fips_network, original_network) - self.assertEqual(reference_fips, reviewed_fips_ids) + _, reviewed_fips_ids, reviewed_router_ids = ( + self.qos_driver.update_network( + mock.Mock(), self.fips_network, original_network)) + self.assertEqual(ref_fips, reviewed_fips_ids) + self.assertEqual(ref_routers, reviewed_router_ids) def test_update_network_no_policy_change(self): """Test update network if the QoS policy is the same. @@ -434,10 +442,11 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): self.networks[0].qos_policy_id = qos_policy_id self.networks[0].update() original_network = {'qos_policy_id': qos_policy_id} - port_ids, fip_ids = self.qos_driver.update_network( + port_ids, fip_ids, router_ids = self.qos_driver.update_network( mock.ANY, self.networks[0], original_network) self.assertEqual(set([]), port_ids) self.assertEqual(set([]), fip_ids) + self.assertEqual(set([]), router_ids) self.mock_rules.assert_not_called() def test_update_network_reset(self): @@ -461,7 +470,7 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): self.networks[0].qos_policy_id = qos_policy_id self.networks[0].update() original_network = {'qos_policy_id': self.qos_policies[0]} - reviewed_port_ids, _ = self.qos_driver.update_network( + reviewed_port_ids, _, _ = self.qos_driver.update_network( mock.ANY, self.networks[0], original_network, reset=True) self.assertEqual(reference_ports, reviewed_port_ids) calls = [mock.call(mock.ANY, self.ports[0].id, @@ -491,7 +500,7 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): self.networks[0].qos_policy_id = qos_policy_id self.networks[0].update() original_network = {'qos_policy_id': self.qos_policies[0]} - reviewed_port_ids, _ = self.qos_driver.update_network( + reviewed_port_ids, _, _ = self.qos_driver.update_network( mock.ANY, self.networks[0], original_network, reset=True) self.assertEqual(reference_ports, reviewed_port_ids) calls = [mock.call( @@ -533,8 +542,10 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): self.fips[0].update() self.fips[1].qos_policy_id = self.qos_policies[1].id self.fips[1].update() - self._update_router_qos(self.routers[0].id, self.qos_policies[0].id) - self._update_router_qos(self.routers[1].id, self.qos_policies[1].id) + self._update_router_qos(self.ctx, self.routers[0].id, + self.qos_policies[0].id) + self._update_router_qos(self.ctx, self.routers[1].id, + self.qos_policies[1].id) mock_qos_rules = mock.Mock() with mock.patch.object(self.qos_driver, '_qos_rules', return_value=mock_qos_rules), \ @@ -665,7 +676,8 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): nb_idl.reset_mock() # Add QoS policy. - self._update_router_qos(router['id'], self.qos_policies[0].id) + self._update_router_qos(self.ctx, router['id'], + self.qos_policies[0].id) router = self._get_router(self.routers[0].id) self.qos_driver.update_router(txn, router) nb_idl.qos_add.assert_called_once() @@ -673,9 +685,20 @@ class TestOVNClientQosExtension(test_plugin.Ml2PluginV2TestCase): nb_idl.reset_mock() # Remove QoS - self._update_router_qos(router['id'], self.qos_policies[0].id, - attach=False) + self._update_router_qos(self.ctx, router['id'], + self.qos_policies[0].id, attach=False) router = self._get_router(self.routers[0].id) self.qos_driver.update_router(txn, router) nb_idl.qos_add.assert_not_called() self.assertEqual(2, nb_idl.qos_del.call_count) + nb_idl.reset_mock() + + # Add network QoS policy + ext_net = self.router_networks[0] + ext_net.qos_policy_id = self.qos_policies[1].id + ext_net.update() + router = self._get_router(self.routers[0].id) + self.qos_driver.update_router(txn, router) + nb_idl.qos_add.assert_called_once() + nb_idl.qos_del.assert_called_once() + nb_idl.reset_mock() diff --git a/releasenotes/notes/qos-gwip-network-inheritance-support-in-ovn-95a53b1dbb5cbac0.yaml b/releasenotes/notes/qos-gwip-network-inheritance-support-in-ovn-95a53b1dbb5cbac0.yaml new file mode 100644 index 00000000000..3c58e0550f9 --- /dev/null +++ b/releasenotes/notes/qos-gwip-network-inheritance-support-in-ovn-95a53b1dbb5cbac0.yaml @@ -0,0 +1,6 @@ +--- +features: + - | + Gateway IP QoS network inheritance is now available for OVN L3 plugin + QoS extension. If the router external network (gateway network) has a QoS + policy associated, the gateway IP port will inherit the network QoS policy.