From b35ed2cfb88d86d0f714799336bbf01649df370e Mon Sep 17 00:00:00 2001 From: Luis Tomas Bolivar Date: Mon, 27 Mar 2023 12:41:35 +0200 Subject: [PATCH] Move base bgp configurations to a common bgp utils Some drivers are duplicating some base BGP configuration to later expose IPs. This patch is moving them to the bgp utils to avoid code duplication. It will also allow different base BGP configuration for different exposing modes within the same driver, once that is implemented. Change-Id: I0061b4598ad649c7492af3464a07d95a21eebc69 --- ovn_bgp_agent/constants.py | 7 +++++++ .../drivers/openstack/nb_ovn_bgp_driver.py | 21 ++++--------------- .../drivers/openstack/ovn_bgp_driver.py | 21 ++++--------------- .../openstack/ovn_stretched_l2_bgp_driver.py | 17 ++++----------- ovn_bgp_agent/drivers/openstack/utils/bgp.py | 19 +++++++++++++++++ .../openstack/test_nb_ovn_bgp_driver.py | 9 ++++++-- .../drivers/openstack/test_ovn_bgp_driver.py | 10 +++++---- 7 files changed, 51 insertions(+), 53 deletions(-) diff --git a/ovn_bgp_agent/constants.py b/ovn_bgp_agent/constants.py index 38e7cd6e..d6f9e940 100644 --- a/ovn_bgp_agent/constants.py +++ b/ovn_bgp_agent/constants.py @@ -67,3 +67,10 @@ EXPOSE = "expose" WITHDRAW = "withdraw" OVN_REQUESTED_CHASSIS = "requested-chassis" + +# Exposing method names +EXPOSE_METHOD_UNDERLAY = 'underlay' +EXPOSE_METHOD_L2VNI = 'l2vni' +EXPOSE_METHOD_VRF = 'vrf' +EXPOSE_METHOD_OVN = 'ovn' +EXPOSE_METHOD_DYNAMIC = 'dynamic' 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 f2eaed36..68c95df7 100644 --- a/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py +++ b/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py @@ -23,7 +23,6 @@ from oslo_log import log as logging from ovn_bgp_agent import constants from ovn_bgp_agent.drivers import driver_api from ovn_bgp_agent.drivers.openstack.utils import bgp as bgp_utils -from ovn_bgp_agent.drivers.openstack.utils import frr from ovn_bgp_agent.drivers.openstack.utils import ovn from ovn_bgp_agent.drivers.openstack.utils import ovs from ovn_bgp_agent.drivers.openstack.utils import wire as wire_utils @@ -83,16 +82,8 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): LOG.info("Loaded chassis %s.", self.chassis) LOG.info("Starting VRF configuration for advertising routes") - # Create VRF - linux_net.ensure_vrf(CONF.bgp_vrf, CONF.bgp_vrf_table_id) - - # Ensure FRR is configure to leak the routes - # NOTE: If we want to recheck this every X time, we should move it - # inside the sync function instead - frr.vrf_leak(CONF.bgp_vrf, CONF.bgp_AS, CONF.bgp_router_id) - - # Create OVN dummy device - linux_net.ensure_ovn_device(CONF.bgp_nic, CONF.bgp_vrf) + # Base BGP configuration + bgp_utils.ensure_base_bgp_configuration() # Clear vrf routing table if CONF.clear_vrf_routes_on_startup: @@ -138,12 +129,8 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): self.ovn_tenant_ls = {} LOG.debug("Ensuring VRF configuration for advertising routes") - # Create VRF - linux_net.ensure_vrf(CONF.bgp_vrf, - CONF.bgp_vrf_table_id) - # Create OVN dummy device - linux_net.ensure_ovn_device(CONF.bgp_nic, - CONF.bgp_vrf) + # Base BGP configuration + bgp_utils.ensure_base_bgp_configuration() LOG.debug("Configuring br-ex default rule and routing tables for " "each provider network") diff --git a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py index 4f2dd626..6729170c 100644 --- a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py +++ b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py @@ -25,7 +25,6 @@ from ovn_bgp_agent import constants from ovn_bgp_agent.drivers import driver_api from ovn_bgp_agent.drivers.openstack.utils import bgp as bgp_utils from ovn_bgp_agent.drivers.openstack.utils import driver_utils -from ovn_bgp_agent.drivers.openstack.utils import frr from ovn_bgp_agent.drivers.openstack.utils import ovn from ovn_bgp_agent.drivers.openstack.utils import ovs from ovn_bgp_agent.drivers.openstack.utils import wire as wire_utils @@ -78,14 +77,8 @@ class OVNBGPDriver(driver_api.AgentDriverBase): LOG.info("Loaded chassis %s.", self.chassis) LOG.info("Starting VRF configuration for advertising routes") - # Create VRF - linux_net.ensure_vrf(CONF.bgp_vrf, CONF.bgp_vrf_table_id) - - # Ensure FRR is configure to leak the routes - frr.vrf_leak(CONF.bgp_vrf, CONF.bgp_AS, CONF.bgp_router_id) - - # Create OVN dummy device - linux_net.ensure_ovn_device(CONF.bgp_nic, CONF.bgp_vrf) + # Base BGP configuration + bgp_utils.ensure_base_bgp_configuration() # Clear vrf routing table if CONF.clear_vrf_routes_on_startup: @@ -157,14 +150,8 @@ class OVNBGPDriver(driver_api.AgentDriverBase): self.ovn_lb_vips = collections.defaultdict() LOG.debug("Ensuring VRF configuration for advertising routes") - # Create VRF - linux_net.ensure_vrf(CONF.bgp_vrf, - CONF.bgp_vrf_table_id) - # Ensure FRR is configure to leak the routes - frr.vrf_leak(CONF.bgp_vrf, CONF.bgp_AS, CONF.bgp_router_id) - # Create OVN dummy device - linux_net.ensure_ovn_device(CONF.bgp_nic, - CONF.bgp_vrf) + # Base BGP configuration + bgp_utils.ensure_base_bgp_configuration() LOG.debug("Configuring br-ex default rule and routing tables for " "each provider network") diff --git a/ovn_bgp_agent/drivers/openstack/ovn_stretched_l2_bgp_driver.py b/ovn_bgp_agent/drivers/openstack/ovn_stretched_l2_bgp_driver.py index 57326e67..c1519dee 100644 --- a/ovn_bgp_agent/drivers/openstack/ovn_stretched_l2_bgp_driver.py +++ b/ovn_bgp_agent/drivers/openstack/ovn_stretched_l2_bgp_driver.py @@ -23,6 +23,7 @@ from oslo_log import log as logging from ovn_bgp_agent import constants from ovn_bgp_agent.drivers import driver_api +from ovn_bgp_agent.drivers.openstack.utils import bgp as bgp_utils from ovn_bgp_agent.drivers.openstack.utils import driver_utils from ovn_bgp_agent.drivers.openstack.utils import frr from ovn_bgp_agent.drivers.openstack.utils import ovn @@ -69,20 +70,10 @@ class OVNBGPStretchedL2Driver(driver_api.AgentDriverBase): self.ovs_idl = ovs.OvsIdl() self.ovs_idl.start(CONF.ovsdb_connection) + # Base BGP configuration # Ensure FRR is configured to leak the routes - # NOTE: If we want to recheck this every X time, we should move it - # inside the sync function instead - frr.vrf_leak( - CONF.bgp_vrf, - CONF.bgp_AS, - CONF.bgp_router_id, - template=frr.LEAK_VRF_KERNEL_TEMPLATE, - ) - - LOG.debug("Setting up VRF %s", CONF.bgp_vrf) - linux_net.ensure_vrf(CONF.bgp_vrf, CONF.bgp_vrf_table_id) - # Create OVN dummy device - linux_net.ensure_ovn_device(CONF.bgp_nic, CONF.bgp_vrf) + bgp_utils.ensure_base_bgp_configuration( + template=frr.LEAK_VRF_KERNEL_TEMPLATE) # Clear vrf routing table if CONF.clear_vrf_routes_on_startup: diff --git a/ovn_bgp_agent/drivers/openstack/utils/bgp.py b/ovn_bgp_agent/drivers/openstack/utils/bgp.py index 77cfc13c..ed6a151a 100644 --- a/ovn_bgp_agent/drivers/openstack/utils/bgp.py +++ b/ovn_bgp_agent/drivers/openstack/utils/bgp.py @@ -15,6 +15,8 @@ from oslo_config import cfg from oslo_log import log as logging +from ovn_bgp_agent import constants +from ovn_bgp_agent.drivers.openstack.utils import frr from ovn_bgp_agent.utils import linux_net @@ -28,3 +30,20 @@ def announce_ips(port_ips): def withdraw_ips(port_ips): linux_net.del_ips_from_dev(CONF.bgp_nic, port_ips) + + +def ensure_base_bgp_configuration(template=frr.LEAK_VRF_TEMPLATE): + if CONF.exposing_method not in [constants.EXPOSE_METHOD_UNDERLAY, + constants.EXPOSE_METHOD_DYNAMIC, + constants.EXPOSE_METHOD_OVN]: + return + + # Create VRF + linux_net.ensure_vrf(CONF.bgp_vrf, CONF.bgp_vrf_table_id) + + # Ensure FRR is configure to leak the routes + frr.vrf_leak(CONF.bgp_vrf, CONF.bgp_AS, CONF.bgp_router_id, + template=template) + + # Create OVN dummy device + linux_net.ensure_ovn_device(CONF.bgp_nic, CONF.bgp_vrf) 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 b1fe30f3..75da4af3 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 @@ -91,7 +91,8 @@ class TestNBOVNBGPDriver(test_base.TestCase): 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) + CONF.bgp_vrf, CONF.bgp_AS, CONF.bgp_router_id, + template=frr.LEAK_VRF_TEMPLATE) mock_ensure_ovn_device.assert_called_once_with(CONF.bgp_nic, CONF.bgp_vrf) mock_delete_routes_from_table.assert_called_once_with( @@ -109,8 +110,9 @@ class TestNBOVNBGPDriver(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_ovn_device') + @mock.patch.object(frr, 'vrf_leak') @mock.patch.object(linux_net, 'ensure_vrf') - def test_sync(self, mock_ensure_vrf, mock_ensure_ovn_dev, + def test_sync(self, mock_ensure_vrf, mock_vrf_leak, mock_ensure_ovn_dev, 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, @@ -139,6 +141,9 @@ class TestNBOVNBGPDriver(test_base.TestCase): 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(self.ovn_routing_tables, 'bridge0', diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/test_ovn_bgp_driver.py b/ovn_bgp_agent/tests/unit/drivers/openstack/test_ovn_bgp_driver.py index c647a8f1..53cfe39f 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/test_ovn_bgp_driver.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/test_ovn_bgp_driver.py @@ -83,7 +83,8 @@ class TestOVNBGPDriver(test_base.TestCase): self.bgp_driver.start() mock_vrf.assert_called_once_with( - CONF.bgp_vrf, CONF.bgp_AS, CONF.bgp_router_id) + CONF.bgp_vrf, CONF.bgp_AS, CONF.bgp_router_id, + template=frr.LEAK_VRF_TEMPLATE) # Assert connections were started self.mock_ovs_idl().start.assert_called_once_with( CONF.ovsdb_connection) @@ -100,10 +101,10 @@ class TestOVNBGPDriver(test_base.TestCase): @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(linux_net, 'ensure_vrf') @mock.patch.object(frr, 'vrf_leak') + @mock.patch.object(linux_net, 'ensure_vrf') def test_sync( - self, mock_vrf_leak, mock_ensure_vrf, mock_ensure_ovn_dev, + 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_riles, @@ -131,7 +132,8 @@ class TestOVNBGPDriver(test_base.TestCase): 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) + 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)