openstack/ovn-bgp-agent
Code Issues Proposed changes

Add BGP NIC IP cleanup logic for the OVN driver

Browse Source 
Currently IPs present on the bgp_nic are not cleaned up during regular
reconciliation syncs so if ovn-bgp-agent misses an update from OVN NB
or that update is not delivered correctly for whatever reason, the IPs
on bgp_nic remain uncleared.

Usually this leads to issues whenever an IP got used and then released
for future use: the fabric keeps using a route advertised by FRR
pointing to the old host that contains the residual IP and not to the
new host that is supposed to be the next hop for this IP.

The underlying reasons for leftover IPs being present still need to be
debugged and fixed so a warning is logged (the warning level is used
as normally the IPs are cleaned up based on NB DB events and this
usually not a normal situation).

Closes-Bug: #2096736
Change-Id: I6f55b5a77f1f9d6a50ef00be48b6c41d00d54b24
This commit is contained in:
Dmitrii Shcherbakov 2025-01-26 05:30:12 +03:00
parent 37655792e1
commit 019605c97f
3 changed files with 43 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
ovn_bgp_agent/drivers/openstack/utils/wire.py
View File

@ -512,19 +512,37 @@ def cleanup_wiring(idl, bridge_mappings, ovs_flows, exposed_ips,
elif CONF.exposing_method == constants.EXPOSE_METHOD_VRF:
return _cleanup_wiring_evpn(ovs_flows, routing_tables_routes)
elif CONF.exposing_method == constants.EXPOSE_METHOD_OVN:
# TODO(ltomasbo): clean up old policies, routes and proxy_arps cidrs
return True
return _cleanup_wiring_ovn(exposed_ips)
def _cleanup_extra_ips(exposed_ips: list[str]) -> set[str]:
current_ips = set(linux_net.get_exposed_ips(CONF.bgp_nic))
expected_ips = {ip for ip_dict in exposed_ips.values()
for ip in ip_dict.keys()}
ips_to_delete = current_ips - expected_ips
if ips_to_delete:
LOG.warning(f"Removing IPs {ips_to_delete} from {CONF.bgp_nic} as "
"they are not expected to be exposed.")
linux_net.delete_exposed_ips(ips_to_delete, CONF.bgp_nic)
return expected_ips
def _cleanup_wiring_ovn(exposed_ips):
"""Cleanup IPs that are not expected to be exposed on the bgp_nic.
This may be needed in case ovn-bgp-agent misses updates from the OVN NB
database due to temporary downtime, networking issues between the
ovn-bgp-agent and the NB DB or otherwise.
"""
_cleanup_extra_ips(exposed_ips)
# TODO(dmitriis): clean up other state: in the local OVN NB: logical
# router policies, logical router routes, ARP proxies in logical switch
# ports, static mac bindings.
def _cleanup_wiring_underlay(idl, bridge_mappings, ovs_flows, exposed_ips,
routing_tables, routing_tables_routes):
current_ips = linux_net.get_exposed_ips(CONF.bgp_nic)
expected_ips = [ip for ip_dict in exposed_ips.values()
for ip in ip_dict.keys()]
ips_to_delete = [ip for ip in current_ips if ip not in expected_ips]
linux_net.delete_exposed_ips(ips_to_delete, CONF.bgp_nic)
expected_ips = _cleanup_extra_ips(exposed_ips)
extra_routes = {}
for bridge in bridge_mappings.values():
extra_routes[bridge] = (

2
ovn_bgp_agent/tests/unit/drivers/openstack/test_nb_ovn_bgp_driver.py
View File

@ -230,7 +230,7 @@ class TestNBOVNBGPDriver(test_base.TestCase):
mock_expose_ovn_lb_vip.assert_called_once_with(lb1)
mock_expose_ovn_lb_fip.assert_called_once_with(lb1)
mock_del_exposed_ips.assert_called_once_with(
ips, CONF.bgp_nic)
set(ips), CONF.bgp_nic)
mock_del_ip_rules.assert_called_once_with(fake_ip_rules)
mock_del_ip_routes.assert_called_once()
bridge = set(self.nb_bgp_driver.ovn_bridge_mappings.values()).pop()

21
ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_wire.py
View File

@ -328,18 +328,27 @@ class TestWire(test_base.TestCase):
self.sb_idl, self.bridge_mappings, ovs_flows, exposed_ips,
routing_tables, routing_tables_routes)
def test_cleanup_wiring_ovn(self):
@mock.patch.object(linux_net, 'get_exposed_ips')
@mock.patch.object(linux_net, 'delete_exposed_ips')
def test_cleanup_wiring_ovn(self, _delete_exposed_ips, _get_exposed_ips):
CONF.set_override('exposing_method', 'ovn')
self.addCleanup(CONF.clear_override, 'exposing_method')
# Both IPs are present on the NIC but .42 is not expected
# to be exposed.
_get_exposed_ips.return_value = ['192.0.2.42', '192.0.2.24']
ovs_flows = {}
exposed_ips = {}
exposed_ips = {
"fakels": {"192.0.2.24": {'bridge_device': "br-example"}}}
routing_tables = {}
routing_tables_routes = {}
ret = wire.cleanup_wiring(self.sb_idl, self.bridge_mappings, ovs_flows,
exposed_ips, routing_tables,
routing_tables_routes)
self.assertTrue(ret)
wire.cleanup_wiring(self.sb_idl, self.bridge_mappings, ovs_flows,
exposed_ips, routing_tables,
routing_tables_routes)
# Make sure we only delete the IP that isn't supposed to be exposed.
_delete_exposed_ips.assert_called_once_with({"192.0.2.42"},
CONF.bgp_nic)
def test_cleanup_wiring_evpn(self):
CONF.set_override('exposing_method', 'vrf')