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(