From f73a2515cbf5ffecdcb0de1598c890da8d58c5cc Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Fri, 16 Feb 2024 20:50:16 +0000 Subject: [PATCH] [OVN] Identify the LR GW port with "external_ids:neutron:is_ext_gw" Since [1], any "Logical_Router_Port" has a key in "external_ids" to identify if it is an external gateway or not. This key is "neutron:is_ext_gw". This patch makes use of this key to identify them in the ``get_lrouter_gw_ports`` method. This implementation will be useful when the "Logical_Router_Port" would be bound to an external tunnelled networks. These ports won't be bound to a "Gateway_Chassis" and the previous logic in this method will fail. [1]https://review.opendev.org/c/openstack/neutron/+/907402 Partial-Bug: #2052821 Change-Id: Ieac603b0c7a940693a12170b453eea39dd0eaa47 --- .../ovn/mech_driver/ovsdb/impl_idl_ovn.py | 16 ++--- .../ovn/mech_driver/test_mech_driver.py | 64 ++++++++++++++++--- .../mech_driver/ovsdb/test_impl_idl_ovn.py | 16 ++--- 3 files changed, 72 insertions(+), 24 deletions(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py index c87214dad04..45082113410 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py @@ -19,6 +19,7 @@ from neutron_lib import constants from neutron_lib import exceptions as n_exc from neutron_lib.utils import helpers from oslo_log import log +from oslo_utils import strutils from oslo_utils import uuidutils from ovs import socket_util from ovs import stream @@ -775,19 +776,18 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend): return result[0] if result else None def get_lrouter_gw_ports(self, lrouter_name): + r_name = ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY + is_gw = ovn_const.OVN_ROUTER_IS_EXT_GW lr = self.get_lrouter(lrouter_name) gw_ports = [] for lrp in getattr(lr, 'ports', []): lrp_ext_ids = getattr(lrp, 'external_ids', {}) - if (ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY not in lrp_ext_ids or - utils.ovn_name(lrp_ext_ids[ - ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY]) != lr.name): + if (r_name not in lrp_ext_ids or + utils.ovn_name(lrp_ext_ids[r_name]) != lr.name or + not strutils.bool_from_string(lrp_ext_ids.get(is_gw))): continue - lrp_ha_cfg = (getattr(lrp, 'gateway_chassis', None) or - getattr(lrp, 'options', {}).get( - ovn_const.OVN_GATEWAY_CHASSIS_KEY)) - if lrp_ha_cfg: - gw_ports.append(lrp) + + gw_ports.append(lrp) return gw_ports def get_lrouter_by_lrouter_port(self, lrp_name): diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 8c85a9ed4b3..25d9730b759 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -1269,17 +1269,12 @@ class TestAgentApi(base.TestOVNFunctionalBase): self.context, metadata_id) -class TestNATRuleGatewayPort(base.TestOVNFunctionalBase): +class _TestRouter(base.TestOVNFunctionalBase): - def setUp(self): - super().setUp() + def setUp(self, **kwargs): + super().setUp(**kwargs) self._ovn_client = self.mech_driver._ovn_client - def deserialize(self, content_type, response): - ctype = 'application/%s' % content_type - data = self._deserializers[ctype].deserialize(response.body)['body'] - return data - def _create_router(self, name, external_gateway_info=None): data = {'router': {'name': name, 'tenant_id': self._tenant_id, 'external_gateway_info': external_gateway_info}} @@ -1289,6 +1284,20 @@ class TestNATRuleGatewayPort(base.TestOVNFunctionalBase): res = req.get_response(self.api) return self.deserialize(self.fmt, res)['router'] + def _update_router(self, router_id, router_dict): + data = {'router': router_dict} + req = self.new_update_request('routers', data, router_id, self.fmt) + res = req.get_response(self.api) + return self.deserialize(self.fmt, res)['router'] + + +class TestNATRuleGatewayPort(_TestRouter): + + def deserialize(self, content_type, response): + ctype = 'application/%s' % content_type + data = self._deserializers[ctype].deserialize(response.body)['body'] + return data + def _process_router_interface(self, action, router_id, subnet_id): req = self.new_action_request( 'routers', {'subnet_id': subnet_id}, router_id, @@ -1369,3 +1378,42 @@ class TestNATRuleGatewayPort(base.TestOVNFunctionalBase): self.assertNotEqual([], fip_rule['gateway_port']) else: self.assertNotIn('gateway_port', fip_rule) + + +class TestRouterGWPort(_TestRouter): + + def test_create_and_delete_router_gw_port(self): + ext_net = self._make_network( + self.fmt, 'ext_networktest', True, as_admin=True, + arg_list=('router:external', + 'provider:network_type', + 'provider:physical_network'), + **{'router:external': True, + 'provider:network_type': 'flat', + 'provider:physical_network': 'public'})['network'] + res = self._create_subnet(self.fmt, ext_net['id'], '100.0.0.0/24') + ext_subnet = self.deserialize(self.fmt, res)['subnet'] + external_gateway_info = { + 'enable_snat': True, + 'network_id': ext_net['id'], + 'external_fixed_ips': [ + {'ip_address': '100.0.0.2', 'subnet_id': ext_subnet['id']}]} + router = self._create_router( + uuidutils.generate_uuid(), + external_gateway_info=external_gateway_info) + + # Check GW LRP. + lr = self._ovn_client._nb_idl.lookup('Logical_Router', + utils.ovn_name(router['id'])) + for lrp in lr.ports: + if lrp.external_ids[ovn_const.OVN_ROUTER_IS_EXT_GW] == str(True): + break + else: + self.fail('Logical Router %s does not have a gateway port' % + utils.ovn_name(router['id'])) + + # Remove LR GW port and check. + self._update_router(router['id'], {'external_gateway_info': {}}) + lr = self._ovn_client._nb_idl.lookup('Logical_Router', + utils.ovn_name(router['id'])) + self.assertEqual([], lr.ports) diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py index e0456f2b5d8..c66b67ef2ed 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py @@ -168,18 +168,18 @@ class TestNBImplIdlOvn(TestDBImplIdlOvn): 'lr-name-f'}}], 'lrouter_ports': [ {'name': utils.ovn_lrouter_port_name('orp-id-a1'), - 'external_ids': {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: - 'lr-id-a'}, + 'external_ids': {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'lr-id-a', + ovn_const.OVN_ROUTER_IS_EXT_GW: str(True)}, 'networks': ['10.0.1.0/24'], 'options': {ovn_const.OVN_GATEWAY_CHASSIS_KEY: 'host-1'}}, {'name': utils.ovn_lrouter_port_name('orp-id-a2'), - 'external_ids': {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: - 'lr-id-a'}, + 'external_ids': {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'lr-id-a', + ovn_const.OVN_ROUTER_IS_EXT_GW: str(True)}, 'networks': ['10.0.2.0/24'], 'options': {ovn_const.OVN_GATEWAY_CHASSIS_KEY: 'host-1'}}, {'name': utils.ovn_lrouter_port_name('orp-id-a3'), - 'external_ids': {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: - 'lr-id-a'}, + 'external_ids': {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'lr-id-a', + ovn_const.OVN_ROUTER_IS_EXT_GW: str(True)}, 'networks': ['10.0.3.0/24'], 'options': {ovn_const.OVN_GATEWAY_CHASSIS_KEY: ovn_const.OVN_GATEWAY_INVALID_CHASSIS}}, @@ -192,8 +192,8 @@ class TestNBImplIdlOvn(TestDBImplIdlOvn): 'external_ids': {}, 'networks': ['20.0.3.0/24'], 'options': {}}, {'name': utils.ovn_lrouter_port_name('gwc'), - 'external_ids': {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: - 'lr-id-f'}, + 'external_ids': {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'lr-id-f', + ovn_const.OVN_ROUTER_IS_EXT_GW: str(True)}, 'networks': ['10.0.4.0/24'], 'options': {}}], 'gateway_chassis': [