From d9c7de64650a8a0c31172a1fd50200ae41b6281f Mon Sep 17 00:00:00 2001 From: Luis Tomas Bolivar Date: Thu, 16 Mar 2023 16:03:03 +0100 Subject: [PATCH] Do not process external network that are not provider It seems it is posible to have external networks (with cr-lrp associated to them) that are not provider networks (e.g., vxlan type). In this case we are opting for not exposing through BGP their IPs as there is no physical infra handling it. This patch makes sure those networks are not considered when processing the events associated to them. Change-Id: Ib436894e2337248b4e37039a1d6690fef406753b --- .../drivers/openstack/ovn_bgp_driver.py | 49 +++++++++++++------ .../drivers/openstack/test_ovn_bgp_driver.py | 36 ++++++++++++-- 2 files changed, 66 insertions(+), 19 deletions(-) diff --git a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py index d0cd0a63..ebd14e3d 100644 --- a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py +++ b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py @@ -263,8 +263,9 @@ class OVNBGPDriver(driver_api.AgentDriverBase): cr_lrp_port, self.chassis, self.sb_idl) if not ips: return - self._expose_ip(ips, patch_port_row, associated_port=cr_lrp_port) - for ip in ips: + ips_adv = self._expose_ip(ips, patch_port_row, + associated_port=cr_lrp_port) + for ip in ips_adv: if exposed_ips and ip in exposed_ips: exposed_ips.remove(ip) if ovn_ip_rules: @@ -560,21 +561,30 @@ class OVNBGPDriver(driver_api.AgentDriverBase): # FIPs are only supported with IPv4 fip_address, fip_datapath = self.sb_idl.get_fip_associated( row.logical_port) - if fip_address: - LOG.debug("Adding BGP route for FIP with ip %s", - fip_address) - if self._expose_provider_port([fip_address], fip_datapath): - LOG.debug("Added BGP route for FIP with ip %s", - fip_address) - return [fip_address] - LOG.debug("Failure adding BGP route for FIP with ip %s", - fip_address) + + if not fip_address: return [] + if not self.sb_idl.is_provider_network(fip_datapath): + # Only exposing IPs if the associated network is a + # provider network + return [] + LOG.debug("Adding BGP route for FIP with ip %s", fip_address) + if self._expose_provider_port([fip_address], fip_datapath): + LOG.debug("Added BGP route for FIP with ip %s", + fip_address) + return [fip_address] + LOG.debug("Failure adding BGP route for FIP with ip %s", + fip_address) + return [] # FIP association to VM elif row.type == constants.OVN_PATCH_VIF_PORT_TYPE: if (associated_port and self.sb_idl.is_port_on_chassis( associated_port, self.chassis)): + if not self.ovn_local_cr_lrps.get(associated_port): + # Only exposing IPs if the associated network is a + # provider network + return [] LOG.debug("Adding BGP route for FIP with ip %s", ips) if self._expose_provider_port(ips, row.datapath): LOG.debug("Added BGP route for FIP with ip %s", ips) @@ -589,6 +599,10 @@ class OVNBGPDriver(driver_api.AgentDriverBase): row.logical_port) if not cr_lrp_datapath: return [] + if not self.sb_idl.is_provider_network(cr_lrp_datapath): + # Only exposing IPs if the associated network is a + # provider network + return [] bridge_device, bridge_vlan = self._get_bridge_for_datapath( cr_lrp_datapath) @@ -690,7 +704,10 @@ class OVNBGPDriver(driver_api.AgentDriverBase): row.logical_port) if not fip_address: return - + if not self.sb_idl.is_provider_network(fip_datapath): + # Only exposing IPs if the associated network is a + # provider network + return LOG.debug("Deleting BGP route for FIP with ip %s", fip_address) if not self._withdraw_provider_port([fip_address], fip_datapath): @@ -706,6 +723,10 @@ class OVNBGPDriver(driver_api.AgentDriverBase): self.sb_idl.is_port_on_chassis( associated_port, self.chassis) or self.sb_idl.is_port_deleted(associated_port))): + if not self.ovn_local_cr_lrps.get(associated_port): + # Only exposing IPs if the associated network is a + # provider network + return LOG.debug("Deleting BGP route for FIP with ip %s", ips) if not self._withdraw_provider_port(ips, row.datapath): LOG.debug("Failure deleting BGP route for FIP with ip %s", @@ -1036,7 +1057,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase): subnet_datapath = self.sb_idl.get_port_datapath( row.options['peer']) - if not cr_lrp: + if not cr_lrp or not self.ovn_local_cr_lrps.get(cr_lrp): return if not self._address_scope_allowed(ip, row.options['peer']): @@ -1072,7 +1093,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase): "triggering a subnet exposure.", row.logical_port) return - if not cr_lrp or cr_lrp not in self.ovn_local_cr_lrps.keys(): + if not cr_lrp or not self.ovn_local_cr_lrps.get(cr_lrp): # NOTE(ltomasbo) there is a chance the cr-lrp just got moved # to this node but was not yet processed. In that case there # is no need to withdraw the network as it was not exposed here 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 46d70085..62466822 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 @@ -188,6 +188,7 @@ class TestOVNBGPDriver(test_base.TestCase): [self.ipv4, self.ipv6], patch_port_row) exposed_ips = [self.ipv4, '192.168.1.20'] + mock_expose_ip.return_value = exposed_ips ip_rules = {"{}/128".format(self.ipv6): 'fake-rules'} self.bgp_driver._ensure_cr_lrp_associated_ports_exposed( 'fake-cr-lrp', exposed_ips, ip_rules) @@ -196,7 +197,6 @@ class TestOVNBGPDriver(test_base.TestCase): [self.ipv4, self.ipv6], patch_port_row, associated_port='fake-cr-lrp') self.assertEqual(['192.168.1.20'], exposed_ips) - self.assertEqual({}, ip_rules) def test__ensure_port_exposed(self): mock_expose_ip = mock.patch.object( @@ -1161,7 +1161,7 @@ class TestOVNBGPDriver(test_base.TestCase): @mock.patch.object(linux_net, 'add_ips_to_dev') def test__expose_ip_vm_with_fip( self, mock_add_ip_dev, mock_add_rule, mock_add_route): - self.sb_idl.is_provider_network.return_value = False + self.sb_idl.is_provider_network.side_effect = [False, True] self.sb_idl.get_fip_associated.return_value = ( self.fip, 'fake-dp') mock_get_bridge = mock.patch.object( @@ -1184,6 +1184,31 @@ class TestOVNBGPDriver(test_base.TestCase): mock_add_route.assert_called_once_with( mock.ANY, self.fip, 'fake-table', self.bridge, vlan=10) + @mock.patch.object(linux_net, 'add_ip_route') + @mock.patch.object(linux_net, 'add_ip_rule') + @mock.patch.object(linux_net, 'add_ips_to_dev') + def test__expose_ip_vm_with_fip_no_provider( + self, mock_add_ip_dev, mock_add_rule, mock_add_route): + self.sb_idl.is_provider_network.side_effect = [False, False] + self.sb_idl.get_fip_associated.return_value = ( + self.fip, 'fake-dp') + mock_get_bridge = mock.patch.object( + self.bgp_driver, '_get_bridge_for_datapath').start() + mock_get_bridge.return_value = (self.bridge, 10) + row = fakes.create_object({ + 'type': constants.OVN_VM_VIF_PORT_TYPE, + 'logical_port': 'fake-logical-port', + 'datapath': 'fake-dp'}) + + ips = [self.ipv4, self.ipv6] + ret = self.bgp_driver._expose_ip(ips, row) + + # Assert that the add methods were called + self.assertEqual([], ret) + mock_add_ip_dev.assert_not_called() + mock_add_rule.assert_not_called() + mock_add_route.assert_not_called() + @mock.patch.object(linux_net, 'add_ip_route') @mock.patch.object(linux_net, 'add_ip_rule') @mock.patch.object(linux_net, 'add_ips_to_dev') @@ -1221,7 +1246,8 @@ class TestOVNBGPDriver(test_base.TestCase): 'datapath': 'fake-dp'}) ips = [self.ipv4, self.ipv6] - ret = self.bgp_driver._expose_ip(ips, row, associated_port=True) + ret = self.bgp_driver._expose_ip(ips, row, + associated_port=self.cr_lrp0) # Assert that the add methods were called self.assertEqual(ips, ret) @@ -1417,7 +1443,7 @@ class TestOVNBGPDriver(test_base.TestCase): @mock.patch.object(linux_net, 'del_ips_from_dev') def test_withdraw_ip_vm_with_fip( self, mock_del_ip_dev, mock_del_rule, mock_del_route): - self.sb_idl.is_provider_network.return_value = False + self.sb_idl.is_provider_network.side_effect = [False, True] self.sb_idl.get_fip_associated.return_value = ( self.fip, 'fake-dp') mock_get_bridge = mock.patch.object( @@ -1475,7 +1501,7 @@ class TestOVNBGPDriver(test_base.TestCase): 'datapath': 'fake-dp'}) ips = [self.ipv4, self.ipv6] - self.bgp_driver.withdraw_ip(ips, row, associated_port=True) + self.bgp_driver.withdraw_ip(ips, row, associated_port=self.cr_lrp0) # Assert that the del methods were called mock_del_ip_dev.assert_called_once_with(