From c923bd9c799e7480228e8757047bb93e123b3c94 Mon Sep 17 00:00:00 2001 From: Fernando Royo Date: Fri, 12 Jan 2024 20:04:33 +0100 Subject: [PATCH] Add support to PF OVN LBs for NB Driver This patch adds support to the OVN LBs created when a port forwarding (PF) is configured over a FIP for the NB driver. Closes-Bug: #2049415 Change-Id: I95d132172d54329306f41fee2cc631e27ccfd8c4 --- ovn_bgp_agent/constants.py | 6 ++ .../drivers/openstack/nb_ovn_bgp_driver.py | 70 ++++++++++++-- .../drivers/openstack/utils/driver_utils.py | 11 +++ ovn_bgp_agent/drivers/openstack/utils/ovn.py | 14 ++- .../openstack/watchers/base_watcher.py | 5 +- .../openstack/watchers/nb_bgp_watcher.py | 40 ++++++++ .../openstack/test_nb_ovn_bgp_driver.py | 96 +++++++++++++++++++ .../unit/drivers/openstack/utils/test_ovn.py | 13 +++ .../openstack/watchers/test_base_watcher.py | 6 ++ 9 files changed, 245 insertions(+), 16 deletions(-) diff --git a/ovn_bgp_agent/constants.py b/ovn_bgp_agent/constants.py index 559f36a8..7285bcf7 100644 --- a/ovn_bgp_agent/constants.py +++ b/ovn_bgp_agent/constants.py @@ -33,11 +33,17 @@ OVN_DEVICE_OWNER_EXT_ID_KEY = 'neutron:device_owner' OVN_FIP_EXT_ID_KEY = 'neutron:port_fip' OVN_FIP_NET_EXT_ID_KEY = 'neutron:fip_network_id' LB_VIP_PORT_PREFIX = "ovn-lb-vip-" +OVN_LB_PF_NAME_PREFIX = "pf-floatingip-" OVN_LB_VIP_IP_EXT_ID_KEY = 'neutron:vip' OVN_LB_VIP_FIP_EXT_ID_KEY = 'neutron:vip_fip' OVN_LB_VIP_PORT_EXT_ID_KEY = 'neutron:vip_port_id' OVN_LB_LR_REF_EXT_ID_KEY = 'lr_ref' +OVN_LB_EXT_ID_ROUTER_KEY = [ + OVN_LB_LR_REF_EXT_ID_KEY, + OVN_LR_NAME_EXT_ID_KEY +] + OVS_RULE_COOKIE = "999" OVS_VRF_RULE_COOKIE = "998" diff --git a/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py b/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py index 184f6f96..76ed4d15 100644 --- a/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py +++ b/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py @@ -145,7 +145,9 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): watcher.LogicalSwitchPortFIPDeleteEvent(self), watcher.LocalnetCreateDeleteEvent(self), watcher.OVNLBCreateEvent(self), - watcher.OVNLBDeleteEvent(self)} + watcher.OVNLBDeleteEvent(self), + watcher.OVNPFCreateEvent(self), + watcher.OVNPFDeleteEvent(self)} if self._expose_tenant_networks: events.update({watcher.ChassisRedirectCreateEvent(self), watcher.ChassisRedirectDeleteEvent(self), @@ -332,18 +334,24 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): def _expose_lbs(self, router_list): lbs = self.nb_idl.get_active_local_lbs(router_list) for lb in lbs: - self._expose_ovn_lb_vip(lb) - # if vip-fip expose fip too - if lb.external_ids.get(constants.OVN_LB_VIP_FIP_EXT_ID_KEY): - self._expose_ovn_lb_fip(lb) + if driver_utils.is_pf_lb(lb): + self._expose_ovn_pf_lb_fip(lb) + else: + self._expose_ovn_lb_vip(lb) + # if vip-fip expose fip too + if lb.external_ids.get(constants.OVN_LB_VIP_FIP_EXT_ID_KEY): + self._expose_ovn_lb_fip(lb) def _withdraw_lbs(self, router_list): lbs = self.nb_idl.get_active_local_lbs(router_list) for lb in lbs: - self._withdraw_ovn_lb_vip(lb) - # if vip-fip withdraw fip too - if lb.external_ids.get(constants.OVN_LB_VIP_FIP_EXT_ID_KEY): - self._withdraw_ovn_lb_fip(lb) + if driver_utils.is_pf_lb(lb): + self._withdraw_ovn_pf_lb_fip(lb) + else: + self._withdraw_ovn_lb_vip(lb) + # if vip-fip withdraw fip too + if lb.external_ids.get(constants.OVN_LB_VIP_FIP_EXT_ID_KEY): + self._withdraw_ovn_lb_fip(lb) @lockutils.synchronized('nbbgp') def expose_ip(self, ips, ips_info): @@ -841,6 +849,50 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): return self._expose_fip(external_ip, external_mac, ls_name, vip_lsp) + def _get_parameters_from_lb(self, lb, include_mac_and_localnet=False): + for fipport in lb.vips.keys(): + fip, port = fipport.split(':') + break + else: + return + + router = lb.external_ids.get( + constants.OVN_LR_NAME_EXT_ID_KEY, '').replace('neutron-', "", 1) + if not router: + return + cr_lrp_info = self.ovn_local_cr_lrps.get(router) + if not cr_lrp_info: + return + net, bridge_device, bridge_vlan = self._get_ls_localnet_info( + cr_lrp_info['provider_switch']) + kwargs = { + 'port_ips': [fip], + 'logical_switch': cr_lrp_info['provider_switch'], + 'bridge_device': bridge_device, + 'bridge_vlan': bridge_vlan} + + if include_mac_and_localnet: + kwargs['mac'] = None + kwargs['localnet'] = net + + return kwargs + + @lockutils.synchronized('nbbgp') + def expose_ovn_pf_lb_fip(self, lb): + self._expose_ovn_pf_lb_fip(lb) + + @lockutils.synchronized('nbbgp') + def withdraw_ovn_pf_lb_fip(self, lb): + self._withdraw_ovn_pf_lb_fip(lb) + + def _withdraw_ovn_pf_lb_fip(self, lb): + kwargs = self._get_parameters_from_lb(lb) + self._withdraw_provider_port(**kwargs) if kwargs else None + + def _expose_ovn_pf_lb_fip(self, lb): + kwargs = self._get_parameters_from_lb(lb, True) + self._expose_provider_port(**kwargs) if kwargs else None + @lockutils.synchronized('nbbgp') def withdraw_ovn_lb_fip(self, lb): self._withdraw_ovn_lb_fip(lb) diff --git a/ovn_bgp_agent/drivers/openstack/utils/driver_utils.py b/ovn_bgp_agent/drivers/openstack/utils/driver_utils.py index 226477a5..86e239de 100644 --- a/ovn_bgp_agent/drivers/openstack/utils/driver_utils.py +++ b/ovn_bgp_agent/drivers/openstack/utils/driver_utils.py @@ -38,3 +38,14 @@ def get_addr_scopes(port): constants.IP_VERSION_6: port.external_ids.get( constants.SUBNET_POOL_ADDR_SCOPE6), } + + +def check_name_prefix(entity, prefix): + try: + return entity.name.startswith(prefix) + except AttributeError: + return False + + +def is_pf_lb(lb): + return check_name_prefix(lb, constants.OVN_LB_PF_NAME_PREFIX) diff --git a/ovn_bgp_agent/drivers/openstack/utils/ovn.py b/ovn_bgp_agent/drivers/openstack/utils/ovn.py index 0f13b099..504794ea 100644 --- a/ovn_bgp_agent/drivers/openstack/utils/ovn.py +++ b/ovn_bgp_agent/drivers/openstack/utils/ovn.py @@ -470,11 +470,17 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend): def get_active_local_lbs(self, local_gateway_ports): lbs = [] cmd = self.db_find_rows('Load_Balancer', ('vips', '!=', {})) + for row in cmd.execute(check_error=True): - if (row.vips and row.external_ids[ - constants.OVN_LB_LR_REF_EXT_ID_KEY].replace( - 'neutron-', "", 1) in local_gateway_ports): - lbs.append(row) + for ext_id_key in constants.OVN_LB_EXT_ID_ROUTER_KEY: + try: + router_name = row.external_ids[ext_id_key].replace( + 'neutron-', '', 1) + if router_name in local_gateway_ports: + lbs.append(row) + break + except KeyError: + continue return lbs # FIXME(ltomasbo): This can be removed once ovsdbapp version is >=2.3.0 diff --git a/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py b/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py index 29d1ecb0..287644a6 100644 --- a/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py +++ b/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py @@ -50,10 +50,9 @@ class OVNLBEvent(Event): events, table, None) self.event_name = self.__class__.__name__ - def _get_router(self, row): + def _get_router(self, row, key=constants.OVN_LB_LR_REF_EXT_ID_KEY): try: - return row.external_ids[ - constants.OVN_LB_LR_REF_EXT_ID_KEY].replace('neutron-', "", 1) + return row.external_ids[key].replace('neutron-', "", 1) except (AttributeError, KeyError): return diff --git a/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py b/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py index 4b3f9dec..ae44bf59 100644 --- a/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py +++ b/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py @@ -738,3 +738,43 @@ class OVNLBDeleteEvent(base_watcher.OVNLBEvent): if hasattr(old, 'vips'): self.agent.withdraw_ovn_lb_vip(row) + + +class OVNPFBaseEvent(base_watcher.OVNLBEvent): + + event = None + + def __init__(self, bgp_agent): + super(OVNPFBaseEvent, self).__init__( + bgp_agent, (self.event,)) + + def match_fn(self, event, row, old): + # The ovn port forwarding are manage as OVN lb balancers and they are + # exposed through the cr-lrp, so if the local agent does not have the + # matching router there is no need to process the event + if not driver_utils.check_name_prefix(row, + constants.OVN_LB_PF_NAME_PREFIX): + return False + + if not row.vips: + return False + lb_router = self._get_router(row, constants.OVN_LR_NAME_EXT_ID_KEY) + return lb_router in self.agent.ovn_local_cr_lrps.keys() + + +class OVNPFCreateEvent(OVNPFBaseEvent): + + event = OVNPFBaseEvent.ROW_CREATE + + def _run(self, event, row, old): + with _SYNC_STATE_LOCK.read_lock(): + self.agent.expose_ovn_pf_lb_fip(row) + + +class OVNPFDeleteEvent(OVNPFBaseEvent): + + event = OVNPFBaseEvent.ROW_DELETE + + def _run(self, event, row, old): + with _SYNC_STATE_LOCK.read_lock(): + self.agent.withdraw_ovn_pf_lb_fip(row) diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/test_nb_ovn_bgp_driver.py b/ovn_bgp_agent/tests/unit/drivers/openstack/test_nb_ovn_bgp_driver.py index 405ddb87..45d33fc5 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/test_nb_ovn_bgp_driver.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/test_nb_ovn_bgp_driver.py @@ -1286,6 +1286,55 @@ class TestNBOVNBGPDriver(test_base.TestCase): mock_withdraw_remote_ip.assert_not_called() mock_withdraw_provider_port.assert_not_called() + def test_expose_ovn_pf_lb_fip(self): + lb = utils.create_row( + external_ids={ + constants.OVN_LR_NAME_EXT_ID_KEY: 'neutron-router1'}, + name='pf-floatingip-uuid-tcp', + vips={'fip:port': 'member:port'}) + + mock_get_ls_localnet_info = mock.patch.object( + self.nb_bgp_driver, '_get_ls_localnet_info').start() + mock_get_ls_localnet_info.return_value = ('provider-ls', 'br-ex', + 100) + mock_expose_provider_port = mock.patch.object( + self.nb_bgp_driver, '_expose_provider_port').start() + + self.nb_bgp_driver.expose_ovn_pf_lb_fip(lb) + kwargs = { + 'port_ips': ['fip'], + 'mac': None, + 'logical_switch': 'provider-ls', + 'bridge_device': 'br-ex', + 'bridge_vlan': 100, + 'localnet': 'provider-ls'} + mock_expose_provider_port.assert_called_once_with(**kwargs) + + def test_expose_ovn_pf_lb_fip_no_router(self): + lb = utils.create_row( + external_ids={}, + name='pf-floatingip-uuid-tcp', + vips={'fip:port': 'member:port'}) + + mock_expose_provider_port = mock.patch.object( + self.nb_bgp_driver, '_expose_provider_port').start() + + self.nb_bgp_driver.expose_ovn_pf_lb_fip(lb) + mock_expose_provider_port.assert_not_called() + + def test_expose_ovn_pf_lb_fip_no_router_cr_lrp(self): + lb = utils.create_row( + external_ids={ + constants.OVN_LR_NAME_EXT_ID_KEY: 'neutron-router2'}, + name='pf-floatingip-uuid-tcp', + vips={'fip:port': 'member:port'}) + + mock_expose_provider_port = mock.patch.object( + self.nb_bgp_driver, '_expose_provider_port').start() + + self.nb_bgp_driver.expose_ovn_pf_lb_fip(lb) + mock_expose_provider_port.assert_not_called() + def test_expose_ovn_lb_fip(self): lb = utils.create_row( external_ids={ @@ -1391,3 +1440,50 @@ class TestNBOVNBGPDriver(test_base.TestCase): self.nb_bgp_driver.withdraw_ovn_lb_fip(lb) mock_withdraw_provider_port.assert_not_called() + + def test_withdraw_ovn_pf_lb_fip(self): + lb = utils.create_row( + external_ids={ + constants.OVN_LR_NAME_EXT_ID_KEY: 'neutron-router1'}, + name='pf-floatingip-uuid-tcp', + vips={'fip:port': 'member:port'}) + + mock_get_ls_localnet_info = mock.patch.object( + self.nb_bgp_driver, '_get_ls_localnet_info').start() + mock_get_ls_localnet_info.return_value = ('provider-ls', 'br-ex', 100) + + mock_withdraw_provider_port = mock.patch.object( + self.nb_bgp_driver, '_withdraw_provider_port').start() + + self.nb_bgp_driver.withdraw_ovn_pf_lb_fip(lb) + kwargs = { + 'port_ips': ['fip'], + 'logical_switch': 'provider-ls', + 'bridge_device': 'br-ex', + 'bridge_vlan': 100} + mock_withdraw_provider_port.assert_called_once_with(**kwargs) + + def test_withdraw_ovn_pf_lb_fip_no_router(self): + lb = utils.create_row( + external_ids={}, + name='pf-floatingip-uuid-tcp', + vips={'fip:port': 'member:port'}) + + mock_withdraw_provider_port = mock.patch.object( + self.nb_bgp_driver, '_withdraw_provider_port').start() + + self.nb_bgp_driver.withdraw_ovn_pf_lb_fip(lb) + mock_withdraw_provider_port.assert_not_called() + + def test_withdraw_ovn_pf_lb_fip_no_cr_lrp(self): + lb = utils.create_row( + external_ids={ + constants.OVN_LR_NAME_EXT_ID_KEY: 'neutron-router2'}, + name='pf-floatingip-uuid-tcp', + vips={'fip:port': 'member:port'}) + + mock_withdraw_provider_port = mock.patch.object( + self.nb_bgp_driver, '_withdraw_provider_port').start() + + self.nb_bgp_driver.withdraw_ovn_pf_lb_fip(lb) + mock_withdraw_provider_port.assert_not_called() diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovn.py b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovn.py index dde08bc6..8b435c82 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovn.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovn.py @@ -221,6 +221,19 @@ class TestOvsdbNbOvnIdl(test_base.TestCase): self.nb_idl.db_find_rows.assert_called_once_with( 'Load_Balancer', ('vips', '!=', {})) + self.nb_idl.db_find_rows.reset_mock() + + lb3 = fakes.create_object({ + 'vips': {'fip': 'member1'}, + 'external_ids': { + constants.OVN_LR_NAME_EXT_ID_KEY: "neutron-router1"}}) + self.nb_idl.db_find_rows.return_value.execute.return_value = [ + lb1, lb2, lb3] + ret = self.nb_idl.get_active_local_lbs(local_gateway_ports) + self.assertEqual([lb1, lb3], ret) + self.nb_idl.db_find_rows.assert_called_once_with( + 'Load_Balancer', ('vips', '!=', {})) + class TestOvsdbSbOvnIdl(test_base.TestCase): diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_base_watcher.py b/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_base_watcher.py index d96650d0..e444a9bc 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_base_watcher.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_base_watcher.py @@ -59,7 +59,13 @@ class TestOVNLBEvent(test_base.TestCase): def test__get_router(self): row = utils.create_row( external_ids={constants.OVN_LB_LR_REF_EXT_ID_KEY: 'neutron-net'}) + self.assertEqual('net', self.ovnlb_event._get_router( + row, constants.OVN_LB_LR_REF_EXT_ID_KEY)) self.assertEqual('net', self.ovnlb_event._get_router(row)) + row = utils.create_row( + external_ids={constants.OVN_LR_NAME_EXT_ID_KEY: 'neutron-router1'}) + self.assertEqual('router1', self.ovnlb_event._get_router( + row, constants.OVN_LR_NAME_EXT_ID_KEY)) row = utils.create_row(external_ids={}) self.assertEqual(None, self.ovnlb_event._get_router(row))