From e8a22767424a34050c9f8921c4c5d0c3557f9520 Mon Sep 17 00:00:00 2001 From: Luis Tomas Bolivar Date: Tue, 29 Nov 2022 12:13:49 +0100 Subject: [PATCH] Code reshape to avoid code duplication This patch moves the logic about exposing/withdrawing an IP on the provider network to common functions to avoid code duplication and make their modifications easier. Also fixes a typo on ensure_arp_ndp_enabled_for_bridge function name Change-Id: I70d86c12fa860a8850b6bcceb7b7374d9702aaee --- .../drivers/openstack/ovn_bgp_driver.py | 251 ++++++------------ .../drivers/openstack/ovn_evpn_driver.py | 2 +- .../drivers/openstack/test_ovn_bgp_driver.py | 162 ++++++++--- .../drivers/openstack/test_ovn_evpn_driver.py | 2 +- .../tests/unit/utils/test_linux_net.py | 12 +- ovn_bgp_agent/utils/linux_net.py | 2 +- 6 files changed, 227 insertions(+), 204 deletions(-) diff --git a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py index f3092ca0..9877cbfa 100644 --- a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py +++ b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py @@ -161,9 +161,9 @@ class OVNBGPDriver(driver_api.AgentDriverBase): linux_net.ensure_vlan_device_for_network(bridge, vlan_tag) - linux_net.ensure_arp_ndp_enabed_for_bridge(bridge, - bridge_index, - vlan_tag) + linux_net.ensure_arp_ndp_enabled_for_bridge(bridge, + bridge_index, + vlan_tag) if flows_info.get(bridge): continue @@ -286,6 +286,33 @@ class OVNBGPDriver(driver_api.AgentDriverBase): exposed_ips.remove(ip_address) ovn_ip_rules.pop(ip_dst, None) + def _expose_provider_port(self, port_ips, provider_datapath, + bridge_device=None, bridge_vlan=None, + lladdr=None): + linux_net.add_ips_to_dev(CONF.bgp_nic, port_ips) + + if not bridge_device and not bridge_vlan: + bridge_device, bridge_vlan = self._get_bridge_for_datapath( + provider_datapath) + for ip in port_ips: + try: + if lladdr: + linux_net.add_ip_rule( + ip, self.ovn_routing_tables[bridge_device], + bridge_device, lladdr=lladdr) + else: + linux_net.add_ip_rule( + ip, self.ovn_routing_tables[bridge_device], + bridge_device) + except agent_exc.InvalidPortIP: + LOG.exception("Invalid IP to create a rule for port" + " on the provider network: %s", ip) + return [] + linux_net.add_ip_route( + self.ovn_routing_tables_routes, ip, + self.ovn_routing_tables[bridge_device], bridge_device, + vlan=bridge_vlan) + def _expose_tenant_port(self, port, ip_version, exposed_ips=[], ovn_ip_rules={}): # specific case for ovn-lb vips on tenant networks @@ -382,6 +409,32 @@ class OVNBGPDriver(driver_api.AgentDriverBase): port, ip_version=router_port_ip_version, exposed_ips=exposed_ips, ovn_ip_rules=ovn_ip_rules) + def _withdraw_provider_port(self, port_ips, provider_datapath, + bridge_device=None, bridge_vlan=None, + lladdr=None): + linux_net.del_ips_from_dev(CONF.bgp_nic, port_ips) + + # assuming either you pass both or none + if not bridge_device and not bridge_vlan: + bridge_device, bridge_vlan = self._get_bridge_for_datapath( + provider_datapath) + for ip in port_ips: + if lladdr: + if linux_net.get_ip_version(ip) == constants.IP_VERSION_6: + cr_lrp_ip = '{}/128'.format(ip) + else: + cr_lrp_ip = '{}/32'.format(ip) + linux_net.del_ip_rule( + cr_lrp_ip, self.ovn_routing_tables[bridge_device], + bridge_device, lladdr=lladdr) + else: + linux_net.del_ip_rule( + ip, self.ovn_routing_tables[bridge_device], bridge_device) + linux_net.del_ip_route( + self.ovn_routing_tables_routes, ip, + self.ovn_routing_tables[bridge_device], bridge_device, + vlan=bridge_vlan) + def _remove_network_exposed(self, subnet_cidr, gateway): gateway_ips = [ip.split('/')[0] for ip in gateway['ips']] subnet_ip = subnet_cidr.split('/')[0] @@ -434,39 +487,24 @@ class OVNBGPDriver(driver_api.AgentDriverBase): def _expose_ovn_lb_on_provider(self, ovn_lb, ip, cr_lrp): self.ovn_local_cr_lrps[cr_lrp]['ovn_lbs'].append(ovn_lb) self.ovn_lb_vips.setdefault(ovn_lb, []).append(ip) - - LOG.debug("Adding BGP route for loadbalancer VIP %s", ip) - linux_net.add_ips_to_dev(CONF.bgp_nic, [ip]) - bridge_device = self.ovn_local_cr_lrps[cr_lrp]['bridge_device'] bridge_vlan = self.ovn_local_cr_lrps[cr_lrp]['bridge_vlan'] - try: - linux_net.add_ip_rule( - ip, self.ovn_routing_tables[bridge_device], bridge_device) - except agent_exc.InvalidPortIP: - LOG.exception("Invalid IP to create a rule for the VM ip" - " on the provider network: %s", ip) - return - linux_net.add_ip_route( - self.ovn_routing_tables_routes, ip, - self.ovn_routing_tables[bridge_device], bridge_device, - vlan=bridge_vlan) + + LOG.debug("Adding BGP route for loadbalancer VIP %s", ip) + self._expose_provider_port([ip], None, bridge_device=bridge_device, + bridge_vlan=bridge_vlan) LOG.debug("Added BGP route for loadbalancer VIP %s", ip) @lockutils.synchronized('bgp') def withdraw_ovn_lb_on_provider(self, ovn_lb, cr_lrp): + bridge_device = self.ovn_local_cr_lrps[cr_lrp]['bridge_device'] + bridge_vlan = self.ovn_local_cr_lrps[cr_lrp]['bridge_vlan'] + for ip in self.ovn_lb_vips[ovn_lb].copy(): LOG.debug("Deleting BGP route for loadbalancer VIP %s", ip) - linux_net.del_ips_from_dev(CONF.bgp_nic, [ip]) - - bridge_device = self.ovn_local_cr_lrps[cr_lrp]['bridge_device'] - bridge_vlan = self.ovn_local_cr_lrps[cr_lrp]['bridge_vlan'] - - linux_net.del_ip_rule(ip, self.ovn_routing_tables[bridge_device], - bridge_device) - linux_net.del_ip_route(self.ovn_routing_tables_routes, ip, - self.ovn_routing_tables[bridge_device], - bridge_device, vlan=bridge_vlan) + self._withdraw_provider_port([ip], None, + bridge_device=bridge_device, + bridge_vlan=bridge_vlan) if ip in self.ovn_lb_vips[ovn_lb]: self.ovn_lb_vips[ovn_lb].remove(ip) LOG.debug("Deleted BGP route for loadbalancer VIP %s", ip) @@ -497,23 +535,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase): row.type == constants.OVN_VIRTUAL_VIF_PORT_TYPE) and self.sb_idl.is_provider_network(row.datapath)): LOG.debug("Adding BGP route for logical port with ip %s", ips) - linux_net.add_ips_to_dev(CONF.bgp_nic, ips) - - bridge_device, bridge_vlan = self._get_bridge_for_datapath( - row.datapath) - for ip in ips: - try: - linux_net.add_ip_rule( - ip, self.ovn_routing_tables[bridge_device], - bridge_device) - except agent_exc.InvalidPortIP: - LOG.exception("Invalid IP to create a rule for the VM ip" - " on the provider network: %s", ip) - return [] - linux_net.add_ip_route( - self.ovn_routing_tables_routes, ip, - self.ovn_routing_tables[bridge_device], bridge_device, - vlan=bridge_vlan) + self._expose_provider_port(ips, row.datapath) LOG.debug("Added BGP route for logical port with ip %s", ips) return ips @@ -525,24 +547,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase): row.logical_port) if fip_address: LOG.debug("Adding BGP route for FIP with ip %s", fip_address) - linux_net.add_ips_to_dev(CONF.bgp_nic, - [fip_address]) - - bridge_device, bridge_vlan = self._get_bridge_for_datapath( - fip_datapath) - try: - linux_net.add_ip_rule( - fip_address, self.ovn_routing_tables[bridge_device], - bridge_device) - except agent_exc.InvalidPortIP: - LOG.exception("Invalid IP to create a rule for the VM " - "floating IP: %s", fip_address) - return [] - - linux_net.add_ip_route( - self.ovn_routing_tables_routes, fip_address, - self.ovn_routing_tables[bridge_device], bridge_device, - vlan=bridge_vlan) + self._expose_provider_port([fip_address], fip_datapath) LOG.debug("Added BGP route for FIP with ip %s", fip_address) return [fip_address] else: @@ -554,24 +559,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase): if (associated_port and self.sb_idl.is_port_on_chassis( associated_port, self.chassis)): LOG.debug("Adding BGP route for FIP with ip %s", ips) - linux_net.add_ips_to_dev(CONF.bgp_nic, ips) - - bridge_device, bridge_vlan = self._get_bridge_for_datapath( - row.datapath) - for ip in ips: - try: - linux_net.add_ip_rule( - ip, self.ovn_routing_tables[bridge_device], - bridge_device) - except agent_exc.InvalidPortIP: - LOG.exception("Invalid IP to create a rule for the " - "floating IP associated to the VM: %s", - ip) - return [] - linux_net.add_ip_route( - self.ovn_routing_tables_routes, ip, - self.ovn_routing_tables[bridge_device], bridge_device, - vlan=bridge_vlan) + self._expose_provider_port(ips, row.datapath) LOG.debug("Added BGP route for FIP with ip %s", ips) return ips @@ -586,6 +574,8 @@ class OVNBGPDriver(driver_api.AgentDriverBase): LOG.debug("Adding BGP route for CR-LRP Port %s", ips) # Keeping information about the associated network for # tenant network advertisement + bridge_device, bridge_vlan = self._get_bridge_for_datapath( + cr_lrp_datapath) self.ovn_local_cr_lrps[row.logical_port] = { 'router_datapath': row.datapath, 'provider_datapath': cr_lrp_datapath, @@ -593,40 +583,17 @@ class OVNBGPDriver(driver_api.AgentDriverBase): 'subnets_datapath': {}, 'subnets_cidr': [], 'ovn_lbs': [], - 'bridge_vlan': None, - 'bridge_device': None + 'bridge_vlan': bridge_vlan, + 'bridge_device': bridge_device } ips_without_mask = [ip.split("/")[0] for ip in ips] - linux_net.add_ips_to_dev(CONF.bgp_nic, ips_without_mask) - - bridge_device, bridge_vlan = self._get_bridge_for_datapath( - cr_lrp_datapath) - - self.ovn_local_cr_lrps[row.logical_port]['bridge_vlan'] = ( - bridge_vlan) - self.ovn_local_cr_lrps[row.logical_port]['bridge_device'] = ( - bridge_device) - + self._expose_provider_port(ips_without_mask, cr_lrp_datapath, + bridge_device, bridge_vlan, + lladdr=row.mac[0].split(' ')[0]) + # add proxy ndp config for ipv6 for ip in ips: - ip_without_mask = ip.split("/")[0] - try: - linux_net.add_ip_rule( - ip_without_mask, - self.ovn_routing_tables[bridge_device], bridge_device, - lladdr=row.mac[0].split(' ')[0]) - except agent_exc.InvalidPortIP: - LOG.exception("Invalid IP to create a rule for the " - "router gateway port: %s", ip_without_mask) - return [] - linux_net.add_ip_route( - self.ovn_routing_tables_routes, ip_without_mask, - self.ovn_routing_tables[bridge_device], bridge_device, - vlan=bridge_vlan) - # add proxy ndp config for ipv6 - if (linux_net.get_ip_version(ip_without_mask) == - constants.IP_VERSION_6): + if linux_net.get_ip_version(ip) == constants.IP_VERSION_6: linux_net.add_ndp_proxy(ip, bridge_device, bridge_vlan) - LOG.debug("Added BGP route for CR-LRP Port %s", ips) # Check if there are networks attached to the router, @@ -691,18 +658,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase): row.type == constants.OVN_VIRTUAL_VIF_PORT_TYPE) and self.sb_idl.is_provider_network(row.datapath)): LOG.debug("Deleting BGP route for logical port with ip %s", ips) - linux_net.del_ips_from_dev(CONF.bgp_nic, ips) - - bridge_device, bridge_vlan = self._get_bridge_for_datapath( - row.datapath) - for ip in ips: - linux_net.del_ip_rule(ip, - self.ovn_routing_tables[bridge_device], - bridge_device) - linux_net.del_ip_route( - self.ovn_routing_tables_routes, ip, - self.ovn_routing_tables[bridge_device], bridge_device, - vlan=bridge_vlan) + self._withdraw_provider_port(ips, row.datapath) LOG.debug("Deleted BGP route for logical port with ip %s", ips) # VM with FIP @@ -714,19 +670,8 @@ class OVNBGPDriver(driver_api.AgentDriverBase): if not fip_address: return - LOG.debug("Delete BGP route for FIP with ip %s", fip_address) - linux_net.del_ips_from_dev(CONF.bgp_nic, - [fip_address]) - - bridge_device, bridge_vlan = self._get_bridge_for_datapath( - fip_datapath) - linux_net.del_ip_rule(fip_address, - self.ovn_routing_tables[bridge_device], - bridge_device) - linux_net.del_ip_route( - self.ovn_routing_tables_routes, fip_address, - self.ovn_routing_tables[bridge_device], bridge_device, - vlan=bridge_vlan) + LOG.debug("Deleting BGP route for FIP with ip %s", fip_address) + self._withdraw_provider_port([fip_address], fip_datapath) LOG.debug("Deleted BGP route for FIP with ip %s", fip_address) # FIP association to VM @@ -736,18 +681,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase): associated_port, self.chassis) or self.sb_idl.is_port_deleted(associated_port))): LOG.debug("Deleting BGP route for FIP with ip %s", ips) - linux_net.del_ips_from_dev(CONF.bgp_nic, ips) - - bridge_device, bridge_vlan = self._get_bridge_for_datapath( - row.datapath) - for ip in ips: - linux_net.del_ip_rule( - ip, self.ovn_routing_tables[bridge_device], - bridge_device) - linux_net.del_ip_route( - self.ovn_routing_tables_routes, ip, - self.ovn_routing_tables[bridge_device], bridge_device, - vlan=bridge_vlan) + self._withdraw_provider_port(ips, row.datapath) LOG.debug("Deleted BGP route for FIP with ip %s", ips) # CR-LRP Port @@ -762,27 +696,16 @@ class OVNBGPDriver(driver_api.AgentDriverBase): # Removing information about the associated network for # tenant network advertisement ips_without_mask = [ip.split("/")[0] for ip in ips] - linux_net.del_ips_from_dev(CONF.bgp_nic, - ips_without_mask) - bridge_vlan = self.ovn_local_cr_lrps[row.logical_port].get( 'bridge_vlan') bridge_device = self.ovn_local_cr_lrps[row.logical_port].get( 'bridge_device') - + self._withdraw_provider_port(ips_without_mask, cr_lrp_datapath, + bridge_device=bridge_device, + bridge_vlan=bridge_vlan, + lladdr=row.mac[0].split(' ')[0]) + # del proxy ndp config for ipv6 for ip in ips_without_mask: - if linux_net.get_ip_version(ip) == constants.IP_VERSION_6: - cr_lrp_ip = '{}/128'.format(ip) - else: - cr_lrp_ip = '{}/32'.format(ip) - linux_net.del_ip_rule( - cr_lrp_ip, self.ovn_routing_tables[bridge_device], - bridge_device, lladdr=row.mac[0].split(' ')[0]) - linux_net.del_ip_route( - self.ovn_routing_tables_routes, ip, - self.ovn_routing_tables[bridge_device], bridge_device, - vlan=bridge_vlan) - # del proxy ndp config for ipv6 if linux_net.get_ip_version(ip) == constants.IP_VERSION_6: cr_lrps_on_same_provider = [ p for p in self.ovn_local_cr_lrps.values() diff --git a/ovn_bgp_agent/drivers/openstack/ovn_evpn_driver.py b/ovn_bgp_agent/drivers/openstack/ovn_evpn_driver.py index be547f25..16e94754 100644 --- a/ovn_bgp_agent/drivers/openstack/ovn_evpn_driver.py +++ b/ovn_bgp_agent/drivers/openstack/ovn_evpn_driver.py @@ -121,7 +121,7 @@ class OVNEVPNDriver(driver_api.AgentDriverBase): bridge = bridge_mapping.split(":")[1] self.ovn_bridge_mappings[network] = bridge - linux_net.ensure_arp_ndp_enabed_for_bridge(bridge, bridge_index) + linux_net.ensure_arp_ndp_enabled_for_bridge(bridge, bridge_index) # TO DO # add missing routes/ips for fips/provider VMs 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 a1183e5d..2a447937 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 @@ -95,7 +95,7 @@ class TestOVNBGPDriver(test_base.TestCase): @mock.patch.object(linux_net, 'get_exposed_ips') @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_enabed_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') def test_sync( @@ -244,6 +244,63 @@ class TestOVNBGPDriver(test_base.TestCase): # the port type is not OVN_VIF_PORT_TYPES mock_expose_ip.assert_not_called() + @mock.patch.object(linux_net, 'add_ips_to_dev') + @mock.patch.object(linux_net, 'add_ip_route') + @mock.patch.object(linux_net, 'add_ip_rule') + def test__expose_provider_port(self, mock_add_rule, mock_add_route, + mock_add_ips_dev): + port_ips = [self.ipv4] + provider_datapath = 'fake-provider-dp' + mock_get_bridge = mock.patch.object( + self.bgp_driver, '_get_bridge_for_datapath').start() + mock_get_bridge.return_value = (self.bridge, 10) + self.bgp_driver._expose_provider_port(port_ips, provider_datapath) + + 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.bridge) + mock_add_route.assert_called_once_with( + mock.ANY, self.ipv4, 'fake-table', self.bridge, vlan=10) + + @mock.patch.object(linux_net, 'add_ips_to_dev') + @mock.patch.object(linux_net, 'add_ip_route') + @mock.patch.object(linux_net, 'add_ip_rule') + def test__expose_provider_port_invalid_ip( + self, mock_add_rule, mock_add_route, mock_add_ips_dev): + port_ips = [self.ipv4] + provider_datapath = 'fake-provider-dp' + mock_get_bridge = mock.patch.object( + self.bgp_driver, '_get_bridge_for_datapath').start() + mock_get_bridge.return_value = (self.bridge, 10) + mock_add_rule.side_effect = agent_exc.InvalidPortIP(ip=self.ipv4) + self.bgp_driver._expose_provider_port(port_ips, provider_datapath) + + 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.bridge) + mock_add_route.assert_not_called() + + @mock.patch.object(linux_net, 'add_ips_to_dev') + @mock.patch.object(linux_net, 'add_ip_route') + @mock.patch.object(linux_net, 'add_ip_rule') + def test__expose_provider_port_with_lladdr( + self, mock_add_rule, mock_add_route, mock_add_ips_dev): + port_ips = [self.ipv4] + provider_datapath = 'fake-provider-dp' + mock_get_bridge = mock.patch.object( + self.bgp_driver, '_get_bridge_for_datapath').start() + mock_get_bridge.return_value = (self.bridge, 10) + self.bgp_driver._expose_provider_port(port_ips, provider_datapath, + lladdr='fake-mac') + 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.bridge, lladdr='fake-mac') + mock_add_route.assert_called_once_with( + mock.ANY, self.ipv4, 'fake-table', self.bridge, vlan=10) + @mock.patch.object(linux_net, 'add_ips_to_dev') @mock.patch.object(linux_net, 'get_ip_version') def test__expose_tenant_port(self, mock_ip_version, mock_add_ips_dev): @@ -359,6 +416,73 @@ class TestOVNBGPDriver(test_base.TestCase): mock_ip_version.assert_not_called() mock_add_ips_dev.assert_not_called() + @mock.patch.object(linux_net, 'del_ips_from_dev') + @mock.patch.object(linux_net, 'del_ip_route') + @mock.patch.object(linux_net, 'del_ip_rule') + def test__withdraw_provider_port(self, mock_del_rule, mock_del_route, + mock_del_ips_dev,): + port_ips = [self.ipv4] + provider_datapath = 'fake-provider-dp' + mock_get_bridge = mock.patch.object( + self.bgp_driver, '_get_bridge_for_datapath').start() + mock_get_bridge.return_value = (self.bridge, 10) + self.bgp_driver._withdraw_provider_port(port_ips, provider_datapath) + + 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.bridge) + mock_del_route.assert_called_once_with( + mock.ANY, self.ipv4, 'fake-table', self.bridge, vlan=10) + + @mock.patch.object(linux_net, 'get_ip_version') + @mock.patch.object(linux_net, 'del_ips_from_dev') + @mock.patch.object(linux_net, 'del_ip_route') + @mock.patch.object(linux_net, 'del_ip_rule') + def test__withdraw_provider_port_lladdr( + self, mock_del_rule, mock_del_route, mock_del_ips_dev, + mock_ip_version): + port_ips = [self.ipv4] + provider_datapath = 'fake-provider-dp' + mock_get_bridge = mock.patch.object( + self.bgp_driver, '_get_bridge_for_datapath').start() + mock_get_bridge.return_value = (self.bridge, 10) + mock_ip_version.return_value = constants.IP_VERSION_4 + self.bgp_driver._withdraw_provider_port(port_ips, provider_datapath, + lladdr='fake-mac') + + mock_del_ips_dev.assert_called_once_with( + CONF.bgp_nic, [self.ipv4]) + mock_del_rule.assert_called_once_with( + '{}/32'.format(self.ipv4), 'fake-table', self.bridge, + lladdr='fake-mac') + mock_del_route.assert_called_once_with( + mock.ANY, self.ipv4, 'fake-table', self.bridge, vlan=10) + + @mock.patch.object(linux_net, 'get_ip_version') + @mock.patch.object(linux_net, 'del_ips_from_dev') + @mock.patch.object(linux_net, 'del_ip_route') + @mock.patch.object(linux_net, 'del_ip_rule') + def test__withdraw_provider_port_lladdr_ipv6( + self, mock_del_rule, mock_del_route, mock_del_ips_dev, + mock_ip_version): + port_ips = [self.ipv6] + provider_datapath = 'fake-provider-dp' + mock_get_bridge = mock.patch.object( + self.bgp_driver, '_get_bridge_for_datapath').start() + mock_get_bridge.return_value = (self.bridge, 10) + mock_ip_version.return_value = constants.IP_VERSION_6 + self.bgp_driver._withdraw_provider_port(port_ips, provider_datapath, + lladdr='fake-mac') + + mock_del_ips_dev.assert_called_once_with( + CONF.bgp_nic, [self.ipv6]) + mock_del_rule.assert_called_once_with( + '{}/128'.format(self.ipv6), 'fake-table', self.bridge, + lladdr='fake-mac') + mock_del_route.assert_called_once_with( + mock.ANY, self.ipv6, 'fake-table', self.bridge, vlan=10) + @mock.patch.object(linux_net, 'add_ips_to_dev') @mock.patch.object(linux_net, 'add_ip_route') @mock.patch.object(linux_net, 'add_ip_rule') @@ -630,39 +754,15 @@ class TestOVNBGPDriver(test_base.TestCase): ret = self.bgp_driver._get_bridge_for_datapath('fake-dp') self.assertEqual((None, None), ret) - @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_ovn_lb_on_provider( - self, mock_add_ip_dev, mock_add_rule, mock_add_route): + def test_expose_ovn_lb_on_provider(self): + mock_expose_provider_port = mock.patch.object( + self.bgp_driver, '_expose_provider_port').start() self.bgp_driver.expose_ovn_lb_on_provider( self.loadbalancer, self.ipv4, self.cr_lrp0) # Assert that the add methods were called - mock_add_ip_dev.assert_called_once_with( - CONF.bgp_nic, [self.ipv4]) - mock_add_rule.assert_called_once_with( - self.ipv4, 'fake-table', self.bridge) - mock_add_route.assert_called_once_with( - mock.ANY, self.ipv4, 'fake-table', self.bridge, vlan=None) - - @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_ovn_lb_on_provider_invalid_ip( - self, mock_add_ip_dev, mock_add_rule, mock_add_route): - # Raise an exception on add_ip_rule() - mock_add_rule.side_effect = agent_exc.InvalidPortIP(ip=self.ipv4) - - self.bgp_driver.expose_ovn_lb_on_provider( - self.loadbalancer, self.ipv4, self.cr_lrp0) - - # Assert that the add methods were called - mock_add_ip_dev.assert_called_once_with( - CONF.bgp_nic, [self.ipv4]) - mock_add_rule.assert_called_once_with( - self.ipv4, 'fake-table', self.bridge) - mock_add_route.assert_not_called() + mock_expose_provider_port.assert_called_once_with( + [self.ipv4], None, bridge_device=self.bridge, bridge_vlan=None) @mock.patch.object(linux_net, 'del_ip_route') @mock.patch.object(linux_net, 'del_ip_rule') @@ -1015,8 +1115,8 @@ class TestOVNBGPDriver(test_base.TestCase): self.bgp_driver, '_remove_network_exposed').start() mock_ip_version.side_effect = (constants.IP_VERSION_4, - constants.IP_VERSION_4, constants.IP_VERSION_6, + constants.IP_VERSION_4, constants.IP_VERSION_6) row = fakes.create_object({ 'name': 'fake-row', diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/test_ovn_evpn_driver.py b/ovn_bgp_agent/tests/unit/drivers/openstack/test_ovn_evpn_driver.py index efbbbfab..9c3e1b85 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/test_ovn_evpn_driver.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/test_ovn_evpn_driver.py @@ -110,7 +110,7 @@ class TestOVNEVPNDriver(test_base.TestCase): CONF.ovsdb_connection) self.mock_sbdb().start.assert_called_once_with() - @mock.patch.object(linux_net, 'ensure_arp_ndp_enabed_for_bridge') + @mock.patch.object(linux_net, 'ensure_arp_ndp_enabled_for_bridge') def test_sync(self, mock_ensure_ndp): self.mock_ovs_idl.get_ovn_bridge_mappings.return_value = [ 'net0:bridge0', 'net1:bridge1'] diff --git a/ovn_bgp_agent/tests/unit/utils/test_linux_net.py b/ovn_bgp_agent/tests/unit/utils/test_linux_net.py index c42c07ae..3b289261 100644 --- a/ovn_bgp_agent/tests/unit/utils/test_linux_net.py +++ b/ovn_bgp_agent/tests/unit/utils/test_linux_net.py @@ -120,9 +120,9 @@ class TestLinuxNet(test_base.TestCase): @mock.patch.object(linux_net, 'enable_proxy_arp') @mock.patch.object(linux_net, 'enable_proxy_ndp') @mock.patch('ovn_bgp_agent.privileged.linux_net.add_ip_to_dev') - def test_ensure_arp_ndp_enabed_for_bridge(self, mock_add_ip_to_dev, - mock_ndp, mock_arp): - linux_net.ensure_arp_ndp_enabed_for_bridge('fake-bridge', 511) + def test_ensure_arp_ndp_enabled_for_bridge(self, mock_add_ip_to_dev, + mock_ndp, mock_arp): + linux_net.ensure_arp_ndp_enabled_for_bridge('fake-bridge', 511) # NOTE(ltomasbo): hardoced starting ipv4 is 192.168.0.0, and ipv6 is # fd53:d91e:400:7f17::0 ipv4 = '192.168.1.255' # base + 511 offset @@ -136,9 +136,9 @@ class TestLinuxNet(test_base.TestCase): @mock.patch.object(linux_net, 'enable_proxy_arp') @mock.patch.object(linux_net, 'enable_proxy_ndp') @mock.patch('ovn_bgp_agent.privileged.linux_net.add_ip_to_dev') - def test_ensure_arp_ndp_enabed_for_bridge_vlan(self, mock_add_ip_to_dev, - mock_ndp, mock_arp): - linux_net.ensure_arp_ndp_enabed_for_bridge('fake-bridge', 511, 11) + def test_ensure_arp_ndp_enabled_for_bridge_vlan(self, mock_add_ip_to_dev, + mock_ndp, mock_arp): + linux_net.ensure_arp_ndp_enabled_for_bridge('fake-bridge', 511, 11) # NOTE(ltomasbo): hardoced starting ipv4 is 192.168.0.0, and ipv6 is # fd53:d91e:400:7f17::0 ipv4 = '192.168.1.255' # base + 511 offset diff --git a/ovn_bgp_agent/utils/linux_net.py b/ovn_bgp_agent/utils/linux_net.py index de8d98f5..0a4802b1 100644 --- a/ovn_bgp_agent/utils/linux_net.py +++ b/ovn_bgp_agent/utils/linux_net.py @@ -79,7 +79,7 @@ def delete_device(device): ovn_bgp_agent.privileged.linux_net.delete_device(device) -def ensure_arp_ndp_enabed_for_bridge(bridge, offset, vlan_tag=None): +def ensure_arp_ndp_enabled_for_bridge(bridge, offset, vlan_tag=None): ipv4 = "192.168." + str(int(offset / 256)) + "." + str(offset % 256) ipv6 = "fd53:d91e:400:7f17::%x" % offset try: