openstack/ovn-bgp-agent
Code Issues Proposed changes

Better protect from FRR restarts

Browse Source 
It is possible that FRR gets restarted, loosing the configuration
added by the agent (for instance the vrf leaking). As the sync
action is kind of heavy to run it more frequently, this patch adds
a new periodic task that is in charge of just ensuring the FRR
configuration and not about the routes being exposed. That way
it can be executed more frequently.

Change-Id: I19c4a295eac6454d5aa465aa7b90ecf258701850
This commit is contained in:
Luis Tomas Bolivar 2023-04-14 10:03:13 +02:00
parent ed43b17904
commit db9d9aee9f
6 changed files with 63 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
ovn_bgp_agent/agent.py
View File

@ -63,6 +63,17 @@ class BGPAgent(service.Service, periodic_task.PeriodicTasks,
except Exception as e:
LOG.exception("Unexpected exception while running the sync: %s", e)
@periodic_task.periodic_task(spacing=CONF.frr_reconcile_interval,
run_immediately=False)
def frr_sync(self, context):
LOG.info("Running reconciliation loop to ensure frr configuration is "
"in place.")
try:
self.agent_driver.frr_sync()
except Exception as e:
LOG.exception("Unexpected exception while running the frr sync: "
"%s", e)
def wait(self):
super(BGPAgent, self).wait()
LOG.info("Service '%s' stopped", self.__class__.__name__)

6
ovn_bgp_agent/config.py
View File

@ -22,8 +22,12 @@ LOG = logging.getLogger(__name__)
agent_opts = [
cfg.IntOpt('reconcile_interval',
help='Time between re-sync actions.',
help='Time (seconds) between re-sync actions.',
default=120),
cfg.IntOpt('frr_reconcile_interval',
help='Time (seconds) between re-sync actions to ensure frr '
'configuration is correct, in case frr is restart.',
default=15),
cfg.BoolOpt('expose_tenant_networks',
help='Expose VM IPs on tenant networks. '
'If this flag is enabled, it takes precedence over '

10
ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py
View File

@ -123,16 +123,18 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase):
events.update([])
return events
@lockutils.synchronized('nbbgp')
def frr_sync(self):
LOG.debug("Ensuring VRF configuration for advertising routes")
# Base BGP configuration
bgp_utils.ensure_base_bgp_configuration()
@lockutils.synchronized('nbbgp')
def sync(self):
self._expose_tenant_networks = (CONF.expose_tenant_networks or
CONF.expose_ipv6_gua_tenant_networks)
self._init_vars()
LOG.debug("Ensuring VRF configuration for advertising routes")
# Base BGP configuration
bgp_utils.ensure_base_bgp_configuration()
LOG.debug("Configuring br-ex default rule and routing tables for "
"each provider network")
flows_info = {}

10
ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py
View File

@ -140,6 +140,12 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
"OVNLBVIPPortEvent"])
return events
@lockutils.synchronized('bgp')
def frr_sync(self):
LOG.debug("Ensuring VRF configuration for advertising routes")
# Base BGP configuration
bgp_utils.ensure_base_bgp_configuration()
@lockutils.synchronized('bgp')
def sync(self):
self._expose_tenant_networks = (CONF.expose_tenant_networks or
@ -151,10 +157,6 @@ class OVNBGPDriver(driver_api.AgentDriverBase):
self.ovn_routing_tables_routes = collections.defaultdict()
self.ovn_lb_vips = collections.defaultdict()
LOG.debug("Ensuring VRF configuration for advertising routes")
# Base BGP configuration
bgp_utils.ensure_base_bgp_configuration()
LOG.debug("Configuring br-ex default rule and routing tables for "
"each provider network")
flows_info = {}

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

@ -99,6 +99,21 @@ class TestNBOVNBGPDriver(test_base.TestCase):
CONF.bgp_vrf_table_id)
self.mock_nbdb().start.assert_called_once_with()
@mock.patch.object(linux_net, 'ensure_ovn_device')
@mock.patch.object(frr, 'vrf_leak')
@mock.patch.object(linux_net, 'ensure_vrf')
def test_frr_sync(self, mock_ensure_vrf, mock_vrf_leak,
mock_ensure_ovn_dev):
self.nb_bgp_driver.frr_sync()
mock_ensure_vrf.assert_called_once_with(
CONF.bgp_vrf, CONF.bgp_vrf_table_id)
mock_vrf_leak.assert_called_once_with(
CONF.bgp_vrf, CONF.bgp_AS, CONF.bgp_router_id,
template=frr.LEAK_VRF_TEMPLATE)
mock_ensure_ovn_dev.assert_called_once_with(
CONF.bgp_nic, CONF.bgp_vrf)
@mock.patch.object(linux_net, 'delete_bridge_ip_routes')
@mock.patch.object(linux_net, 'delete_ip_rules')
@mock.patch.object(linux_net, 'delete_exposed_ips')
@ -109,11 +124,7 @@ class TestNBOVNBGPDriver(test_base.TestCase):
@mock.patch.object(linux_net, 'ensure_arp_ndp_enabled_for_bridge')
@mock.patch.object(linux_net, 'ensure_vlan_device_for_network')
@mock.patch.object(linux_net, 'ensure_routing_table_for_bridge')
@mock.patch.object(linux_net, 'ensure_ovn_device')
@mock.patch.object(frr, 'vrf_leak')
@mock.patch.object(linux_net, 'ensure_vrf')
def test_sync(self, mock_ensure_vrf, mock_vrf_leak, mock_ensure_ovn_dev,
mock_routing_bridge, mock_ensure_vlan_network,
def test_sync(self, mock_routing_bridge, mock_ensure_vlan_network,
mock_ensure_arp, mock_flows_info, mock_remove_flows,
mock_exposed_ips, mock_get_ip_rules, mock_del_exposed_ips,
mock_del_ip_riles, moock_del_ip_routes):
@ -140,13 +151,6 @@ class TestNBOVNBGPDriver(test_base.TestCase):
self.nb_bgp_driver.sync()
mock_ensure_vrf.assert_called_once_with(
CONF.bgp_vrf, CONF.bgp_vrf_table_id)
mock_vrf_leak.assert_called_once_with(
CONF.bgp_vrf, CONF.bgp_AS, CONF.bgp_router_id,
template=frr.LEAK_VRF_TEMPLATE)
mock_ensure_ovn_dev.assert_called_once_with(
CONF.bgp_nic, CONF.bgp_vrf)
expected_calls = [mock.call({}, 'bridge0', CONF.bgp_vrf_table_id),
mock.call({}, 'bridge1', CONF.bgp_vrf_table_id)]
mock_routing_bridge.assert_has_calls(expected_calls)

35
ovn_bgp_agent/tests/unit/drivers/openstack/test_ovn_bgp_driver.py
View File

@ -90,6 +90,21 @@ class TestOVNBGPDriver(test_base.TestCase):
CONF.ovsdb_connection)
self.mock_sbdb().start.assert_called_once_with()
@mock.patch.object(linux_net, 'ensure_ovn_device')
@mock.patch.object(frr, 'vrf_leak')
@mock.patch.object(linux_net, 'ensure_vrf')
def test_frr_sync(self, mock_ensure_vrf, mock_vrf_leak,
mock_ensure_ovn_dev):
self.bgp_driver.frr_sync()
mock_ensure_vrf.assert_called_once_with(
CONF.bgp_vrf, CONF.bgp_vrf_table_id)
mock_vrf_leak.assert_called_once_with(
CONF.bgp_vrf, CONF.bgp_AS, CONF.bgp_router_id,
template=frr.LEAK_VRF_TEMPLATE)
mock_ensure_ovn_dev.assert_called_once_with(
CONF.bgp_nic, CONF.bgp_vrf)
@mock.patch.object(linux_net, 'delete_bridge_ip_routes')
@mock.patch.object(linux_net, 'delete_ip_rules')
@mock.patch.object(linux_net, 'delete_exposed_ips')
@ -100,15 +115,11 @@ class TestOVNBGPDriver(test_base.TestCase):
@mock.patch.object(linux_net, 'ensure_vlan_device_for_network')
@mock.patch.object(linux_net, 'ensure_routing_table_for_bridge')
@mock.patch.object(linux_net, 'ensure_arp_ndp_enabled_for_bridge')
@mock.patch.object(linux_net, 'ensure_ovn_device')
@mock.patch.object(frr, 'vrf_leak')
@mock.patch.object(linux_net, 'ensure_vrf')
def test_sync(
self, mock_ensure_vrf, mock_vrf_leak, mock_ensure_ovn_dev,
mock_ensure_arp, mock_routing_bridge, mock_ensure_vlan_network,
mock_exposed_ips, mock_get_ip_rules, mock_flows_info,
mock_remove_flows, mock_del_exposed_ips, mock_del_ip_rules,
mock_del_ip_routes):
self, mock_ensure_arp, mock_routing_bridge,
mock_ensure_vlan_network, mock_exposed_ips, mock_get_ip_rules,
mock_flows_info, mock_remove_flows, mock_del_exposed_ips,
mock_del_ip_rules, mock_del_ip_routes):
self.mock_ovs_idl.get_ovn_bridge_mappings.return_value = [
'net0:bridge0', 'net1:bridge1']
self.sb_idl.get_network_vlan_tag_by_network_name.side_effect = (
@ -130,14 +141,6 @@ class TestOVNBGPDriver(test_base.TestCase):
self.bgp_driver.sync()
mock_ensure_vrf.assert_called_once_with(
CONF.bgp_vrf, CONF.bgp_vrf_table_id)
mock_vrf_leak.assert_called_once_with(
CONF.bgp_vrf, CONF.bgp_AS, CONF.bgp_router_id,
template=frr.LEAK_VRF_TEMPLATE)
mock_ensure_ovn_dev.assert_called_once_with(
CONF.bgp_nic, CONF.bgp_vrf)
expected_calls = [mock.call('bridge0', 1, 10),
mock.call('bridge1', 2, 11)]
mock_ensure_arp.assert_has_calls(expected_calls)