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 46a5b309..165a0603 100644 --- a/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py +++ b/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py @@ -554,10 +554,10 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): return None, None, None net_id = nat_entry.external_ids.get(constants.OVN_FIP_NET_EXT_ID_KEY) if not net_id: - return nat_entry.external_ip, nat_entry.external_mac, None + return nat_entry.external_ip, nat_entry.external_mac[0], None else: ls_name = "neutron-{}".format(net_id) - return nat_entry.external_ip, nat_entry.external_mac, ls_name + return nat_entry.external_ip, nat_entry.external_mac[0], ls_name @lockutils.synchronized('nbbgp') def expose_fip(self, ip, mac, logical_switch, row): diff --git a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py index b037d671..2c705966 100644 --- a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py +++ b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py @@ -335,7 +335,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase): if wire_utils.wire_provider_port( self.ovn_routing_tables_routes, self.ovs_flows, port_ips, bridge_device, bridge_vlan, localnet, - self.ovn_routing_tables, proxy_cidrs, lladdr=lladdr): + self.ovn_routing_tables, proxy_cidrs, mac=lladdr): # Expose the IP now that it is connected bgp_utils.announce_ips(port_ips) return True @@ -413,7 +413,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase): return wire_utils.unwire_provider_port( self.ovn_routing_tables_routes, port_ips, bridge_device, bridge_vlan, self.ovn_routing_tables, proxy_cidrs, - lladdr=lladdr) + mac=lladdr) except Exception as e: LOG.exception("Unexpected exception while unwiring provider port: " "%s", e) diff --git a/ovn_bgp_agent/drivers/openstack/utils/wire.py b/ovn_bgp_agent/drivers/openstack/utils/wire.py index 38894aa8..69f39791 100644 --- a/ovn_bgp_agent/drivers/openstack/utils/wire.py +++ b/ovn_bgp_agent/drivers/openstack/utils/wire.py @@ -555,13 +555,13 @@ def _cleanup_wiring_evpn(ovs_flows, routing_tables_routes): def wire_provider_port(routing_tables_routes, ovs_flows, port_ips, bridge_device, bridge_vlan, localnet, routing_table, - proxy_cidrs, lladdr=None, mac=None, ovn_idl=None): + proxy_cidrs, mac=None, ovn_idl=None): if CONF.exposing_method == constants.EXPOSE_METHOD_UNDERLAY: return _wire_provider_port_underlay(routing_tables_routes, ovs_flows, port_ips, bridge_device, bridge_vlan, localnet, routing_table, proxy_cidrs, - lladdr=lladdr) + lladdr=mac) elif CONF.exposing_method == constants.EXPOSE_METHOD_VRF: return _wire_provider_port_evpn(routing_tables_routes, ovs_flows, port_ips, bridge_device, @@ -576,17 +576,17 @@ def wire_provider_port(routing_tables_routes, ovs_flows, port_ips, def unwire_provider_port(routing_tables_routes, port_ips, bridge_device, - bridge_vlan, routing_table, proxy_cidrs, lladdr=None, + bridge_vlan, routing_table, proxy_cidrs, mac=None, ovn_idl=None): if CONF.exposing_method == constants.EXPOSE_METHOD_UNDERLAY: return _unwire_provider_port_underlay(routing_tables_routes, port_ips, bridge_device, bridge_vlan, routing_table, proxy_cidrs, - lladdr=lladdr) + lladdr=mac) elif CONF.exposing_method == constants.EXPOSE_METHOD_VRF: return _unwire_provider_port_evpn(routing_tables_routes, port_ips, bridge_device, bridge_vlan, - lladdr) + mac) elif CONF.exposing_method == constants.EXPOSE_METHOD_OVN: # We need to remove thestatic mac binding added due to proxy-arp issue # in core ovn that would reply on the incomming traffic from the LR, @@ -618,7 +618,8 @@ def _wire_provider_port_underlay(routing_tables_routes, ovs_flows, port_ips, linux_net.add_ip_rule(ip, routing_table[bridge_device], dev=dev, lladdr=lladdr) else: - linux_net.add_ip_rule(ip, routing_table[bridge_device]) + linux_net.add_ip_rule(ip, routing_table[bridge_device], + dev=bridge_device) except agent_exc.InvalidPortIP: LOG.exception("Invalid IP to create a rule for port on the " "provider network: %s", ip) @@ -699,7 +700,8 @@ def _unwire_provider_port_underlay(routing_tables_routes, port_ips, return False else: try: - linux_net.del_ip_rule(ip, routing_table[bridge_device]) + linux_net.del_ip_rule(ip, routing_table[bridge_device], + dev=bridge_device) except agent_exc.InvalidPortIP: LOG.exception("Invalid IP to delete a rule for the " "provider port: %s", ip) 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 6264eb4d..6a56e133 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 @@ -728,12 +728,12 @@ class TestNBOVNBGPDriver(test_base.TestCase): nat_entry = fakes.create_object({ 'external_ids': {constants.OVN_FIP_NET_EXT_ID_KEY: 'net1'}, 'external_ip': 'fake-ip', - 'external_mac': 'fake-mac'}) + 'external_mac': ['fake-mac']}) self.nb_idl.get_nat_by_logical_port.return_value = nat_entry ret = self.nb_bgp_driver.get_port_external_ip_and_ls('fake-port') - expected_result = (nat_entry.external_ip, nat_entry.external_mac, + expected_result = (nat_entry.external_ip, nat_entry.external_mac[0], "neutron-net1") self.assertEqual(ret, expected_result) @@ -748,13 +748,13 @@ class TestNBOVNBGPDriver(test_base.TestCase): nat_entry = fakes.create_object({ 'external_ids': {}, 'external_ip': 'fake-ip', - 'external_mac': 'fake-mac'}) + 'external_mac': ['fake-mac']}) self.nb_idl.get_nat_by_logical_port.return_value = nat_entry ret = self.nb_bgp_driver.get_port_external_ip_and_ls('fake-port') - self.assertEqual(ret, - (nat_entry.external_ip, nat_entry.external_mac, None)) + self.assertEqual( + ret, (nat_entry.external_ip, nat_entry.external_mac[0], None)) def test_expose_fip(self): ip = '10.0.0.1' @@ -773,7 +773,8 @@ class TestNBOVNBGPDriver(test_base.TestCase): ret = self.nb_bgp_driver.expose_fip(ip, mac, logical_switch, row) mock_get_ls_localnet_info.assert_called_once_with(logical_switch) - mock_expose_provider_port.assert_called_once_with([ip], mac, 'test-ls', + mock_expose_provider_port.assert_called_once_with([ip], mac, + 'test-ls', 'br-ex', 100, 'fake-localnet') self.assertTrue(ret) 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 64d4b747..dbf38769 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 @@ -298,7 +298,7 @@ class TestOVNBGPDriver(test_base.TestCase): mock_add_ips_dev.assert_called_once_with( CONF.bgp_nic, [self.ipv4]) mock_add_rule.assert_called_once_with( - self.ipv4, 'fake-table') + self.ipv4, 'fake-table', dev='fake-bridge') mock_add_route.assert_called_once_with( mock.ANY, self.ipv4, 'fake-table', self.bridge, vlan=10) @@ -340,7 +340,7 @@ class TestOVNBGPDriver(test_base.TestCase): self.assertEqual(False, ret) mock_add_ips_dev.assert_not_called() mock_add_rule.assert_called_once_with( - self.ipv4, 'fake-table') + self.ipv4, 'fake-table', dev='fake-bridge') mock_add_route.assert_not_called() mock_ensure_mac_tweak.assert_not_called() @@ -518,7 +518,7 @@ class TestOVNBGPDriver(test_base.TestCase): mock_del_ips_dev.assert_called_once_with( CONF.bgp_nic, [self.ipv4]) mock_del_rule.assert_called_once_with( - self.ipv4, 'fake-table') + self.ipv4, 'fake-table', dev='fake-bridge') mock_del_route.assert_called_once_with( mock.ANY, self.ipv4, 'fake-table', self.bridge, vlan=10) @@ -1009,8 +1009,10 @@ class TestOVNBGPDriver(test_base.TestCase): mock.call(CONF.bgp_nic, [self.ipv6])] mock_del_ip_dev.assert_has_calls(expected_calls) - expected_calls = [mock.call(self.ipv4, 'fake-table'), - mock.call(self.ipv6, 'fake-table')] + expected_calls = [mock.call(self.ipv4, 'fake-table', + dev='fake-bridge'), + mock.call(self.ipv6, 'fake-table', + dev='fake-bridge')] mock_del_rule.assert_has_calls(expected_calls) expected_calls = [mock.call(mock.ANY, self.ipv4, 'fake-table', @@ -1066,8 +1068,10 @@ class TestOVNBGPDriver(test_base.TestCase): mock_add_ip_dev.assert_called_once_with( CONF.bgp_nic, ips) - expected_calls = [mock.call(self.ipv4, 'fake-table'), - mock.call(self.ipv6, 'fake-table')] + expected_calls = [mock.call(self.ipv4, 'fake-table', + dev='fake-bridge'), + mock.call(self.ipv6, 'fake-table', + dev='fake-bridge')] mock_add_rule.assert_has_calls(expected_calls) expected_calls = [mock.call(mock.ANY, self.ipv4, 'fake-table', @@ -1149,8 +1153,10 @@ class TestOVNBGPDriver(test_base.TestCase): mock_add_ip_dev.assert_called_once_with( CONF.bgp_nic, ips) - expected_calls = [mock.call(self.ipv4, 'fake-table'), - mock.call(self.ipv6, 'fake-table')] + expected_calls = [mock.call(self.ipv4, 'fake-table', + dev='fake-bridge'), + mock.call(self.ipv6, 'fake-table', + dev='fake-bridge')] mock_add_rule.assert_has_calls(expected_calls) expected_calls = [mock.call(mock.ANY, self.ipv4, 'fake-table', @@ -1218,7 +1224,7 @@ class TestOVNBGPDriver(test_base.TestCase): mock_add_ip_dev.assert_called_once_with( CONF.bgp_nic, [self.fip]) mock_add_rule.assert_called_once_with( - self.fip, 'fake-table') + self.fip, 'fake-table', dev='fake-bridge') mock_add_route.assert_called_once_with( mock.ANY, self.fip, 'fake-table', self.bridge, vlan=10) @@ -1296,8 +1302,10 @@ class TestOVNBGPDriver(test_base.TestCase): mock_add_ip_dev.assert_called_once_with( CONF.bgp_nic, ips) - expected_calls = [mock.call(self.ipv4, 'fake-table'), - mock.call(self.ipv6, 'fake-table')] + expected_calls = [mock.call(self.ipv4, 'fake-table', + dev='fake-bridge'), + mock.call(self.ipv6, 'fake-table', + dev='fake-bridge')] mock_add_rule.assert_has_calls(expected_calls) expected_calls = [mock.call(mock.ANY, self.ipv4, 'fake-table', @@ -1436,8 +1444,10 @@ class TestOVNBGPDriver(test_base.TestCase): mock_del_ip_dev.assert_called_once_with( CONF.bgp_nic, ips) - expected_calls = [mock.call(self.ipv4, 'fake-table'), - mock.call(self.ipv6, 'fake-table')] + expected_calls = [mock.call(self.ipv4, 'fake-table', + dev='fake-bridge'), + mock.call(self.ipv6, 'fake-table', + dev='fake-bridge')] mock_del_rule.assert_has_calls(expected_calls) expected_calls = [mock.call(mock.ANY, self.ipv4, 'fake-table', @@ -1474,8 +1484,10 @@ class TestOVNBGPDriver(test_base.TestCase): mock_del_ip_dev.assert_called_once_with( CONF.bgp_nic, ips) - expected_calls = [mock.call(self.ipv4, 'fake-table'), - mock.call(self.ipv6, 'fake-table')] + expected_calls = [mock.call(self.ipv4, 'fake-table', + dev='fake-bridge'), + mock.call(self.ipv6, 'fake-table', + dev='fake-bridge')] mock_del_rule.assert_has_calls(expected_calls) expected_calls = [mock.call(mock.ANY, self.ipv4, 'fake-table', @@ -1509,7 +1521,7 @@ class TestOVNBGPDriver(test_base.TestCase): mock_del_ip_dev.assert_called_once_with( CONF.bgp_nic, [self.fip]) mock_del_rule.assert_called_once_with( - self.fip, 'fake-table') + self.fip, 'fake-table', dev='fake-bridge') mock_del_route.assert_called_once_with( mock.ANY, self.fip, 'fake-table', self.bridge, vlan=10) @@ -1555,8 +1567,10 @@ class TestOVNBGPDriver(test_base.TestCase): mock_del_ip_dev.assert_called_once_with( CONF.bgp_nic, ips) - expected_calls = [mock.call(self.ipv4, 'fake-table'), - mock.call(self.ipv6, 'fake-table')] + expected_calls = [mock.call(self.ipv4, 'fake-table', + dev='fake-bridge'), + mock.call(self.ipv6, 'fake-table', + dev='fake-bridge')] mock_del_rule.assert_has_calls(expected_calls) expected_calls = [mock.call(mock.ANY, self.ipv4, 'fake-table', diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_wire.py b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_wire.py index dc89e4bb..fd82b1d6 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_wire.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_wire.py @@ -574,7 +574,7 @@ class TestWire(test_base.TestCase): wire.unwire_provider_port(routing_tables_routes, port_ips, bridge_device, bridge_vlan, routing_table, - proxy_cidrs, lladdr='boo') + proxy_cidrs, mac='boo') mock_evpn.assert_called_once_with(routing_tables_routes, port_ips, bridge_device, bridge_vlan, 'boo') @@ -757,6 +757,9 @@ class TestWire(test_base.TestCase): CONF.set_override( 'advertisement_method_tenant_networks', constants.ADVERTISEMENT_METHOD_SUBNET) + self.addCleanup( + CONF.clear_override, + 'advertisement_method_tenant_networks') routing_tables_routes = {} ip = '10.0.0.1/24' bridge_device = 'fake-bridge' diff --git a/ovn_bgp_agent/utils/linux_net.py b/ovn_bgp_agent/utils/linux_net.py index a1b01b1c..4dc25740 100644 --- a/ovn_bgp_agent/utils/linux_net.py +++ b/ovn_bgp_agent/utils/linux_net.py @@ -701,8 +701,10 @@ def del_ip_rule(ip, table, dev=None, lladdr=None): ovn_bgp_agent.privileged.linux_net.rule_delete(rule) - if lladdr: - del_ip_nei(ip, lladdr, dev) + # TODO(jayjahns): The lladdr in all delete operations is blank, so + # we need to investigate how to ensure that exists in order to + # add the if comparison. + del_ip_nei(ip, lladdr, dev) def del_ip_nei(ip, lladdr, dev):