diff --git a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py index 2b935b55..0041c086 100644 --- a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py +++ b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py @@ -320,6 +320,8 @@ class OVNBGPDriver(driver_api.AgentDriverBase): if not bridge_device and not bridge_vlan: bridge_device, bridge_vlan = self._get_bridge_for_datapath( provider_datapath) + if not bridge_device: + return False # Connect to OVN if wire_utils.wire_provider_port( @@ -328,6 +330,8 @@ class OVNBGPDriver(driver_api.AgentDriverBase): proxy_cidrs, lladdr): # Expose the IP now that it is connected bgp_utils.announce_ips(port_ips) + return True + return False def _expose_tenant_port(self, port, ip_version, exposed_ips=None, ovn_ip_rules=None): @@ -388,7 +392,9 @@ class OVNBGPDriver(driver_api.AgentDriverBase): if not bridge_device and not bridge_vlan: bridge_device, bridge_vlan = self._get_bridge_for_datapath( provider_datapath) - wire_utils.unwire_provider_port( + if not bridge_device: + return False + return wire_utils.unwire_provider_port( self.ovn_routing_tables_routes, port_ips, bridge_device, bridge_vlan, self.ovn_routing_tables[bridge_device], proxy_cidrs, lladdr) @@ -439,14 +445,21 @@ class OVNBGPDriver(driver_api.AgentDriverBase): def _expose_ovn_lb_on_provider(self, ip, ovn_lb_vip_port, cr_lrp, exposed_ips=None, ovn_ip_rules=None): + LOG.debug("Adding BGP route for loadbalancer VIP %s", ip) + try: + bridge_device = self.ovn_local_cr_lrps[cr_lrp]['bridge_device'] + bridge_vlan = self.ovn_local_cr_lrps[cr_lrp]['bridge_vlan'] + except KeyError: + LOG.debug("Failure adding BGP route for loadbalancer VIP %s", ip) + return False + self.ovn_local_cr_lrps[cr_lrp]['ovn_lb_vips'].append(ovn_lb_vip_port) self.ovn_lb_vips.setdefault(ovn_lb_vip_port, []).append(ip) - bridge_device = self.ovn_local_cr_lrps[cr_lrp]['bridge_device'] - bridge_vlan = self.ovn_local_cr_lrps[cr_lrp]['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) + if not self._expose_provider_port( + [ip], None, bridge_device=bridge_device, + bridge_vlan=bridge_vlan): + LOG.debug("Failure adding BGP route for loadbalancer VIP %s", ip) + return False LOG.debug("Added BGP route for loadbalancer VIP %s", ip) if exposed_ips and ip in exposed_ips: exposed_ips.remove(ip) @@ -457,22 +470,32 @@ class OVNBGPDriver(driver_api.AgentDriverBase): else: ip_dst = "{}/32".format(ip) ovn_ip_rules.pop(ip_dst, None) + return True def _withdraw_ovn_lb_on_provider(self, ovn_lb_vip_port, cr_lrp): - bridge_device = self.ovn_local_cr_lrps[cr_lrp]['bridge_device'] - bridge_vlan = self.ovn_local_cr_lrps[cr_lrp]['bridge_vlan'] + try: + bridge_device = self.ovn_local_cr_lrps[cr_lrp]['bridge_device'] + bridge_vlan = self.ovn_local_cr_lrps[cr_lrp]['bridge_vlan'] + except KeyError: + LOG.debug("Failure deleting BGP routes for loadbalancer VIP port " + "%s", ovn_lb_vip_port) + return False for ip in self.ovn_lb_vips[ovn_lb_vip_port].copy(): LOG.debug("Deleting BGP route for loadbalancer VIP %s", ip) - self._withdraw_provider_port([ip], None, - bridge_device=bridge_device, - bridge_vlan=bridge_vlan) + if not self._withdraw_provider_port( + [ip], None, bridge_device=bridge_device, + bridge_vlan=bridge_vlan): + LOG.debug("Failure deleting BGP route for loadbalancer VIP " + "%s", ip) + return False if ip in self.ovn_lb_vips[ovn_lb_vip_port]: self.ovn_lb_vips[ovn_lb_vip_port].remove(ip) LOG.debug("Deleted BGP route for loadbalancer VIP %s", ip) if ovn_lb_vip_port in self.ovn_local_cr_lrps[cr_lrp]['ovn_lb_vips']: self.ovn_local_cr_lrps[cr_lrp]['ovn_lb_vips'].remove( ovn_lb_vip_port) + return True @lockutils.synchronized('bgp') def expose_ip(self, ips, row, associated_port=None): @@ -504,9 +527,10 @@ class OVNBGPDriver(driver_api.AgentDriverBase): LOG.debug("Port %s not being exposed as its associated " "datapath %s was removed", row.logical_port, row.datapath) - return + return [] # VM on provider Network if provider_network: + exposed_port = False LOG.debug("Adding BGP route for logical port with ip %s", ips) if row.type == constants.OVN_VIRTUAL_VIF_PORT_TYPE: # NOTE: For Amphora Load Balancer with IPv6 VIP on the @@ -518,11 +542,16 @@ class OVNBGPDriver(driver_api.AgentDriverBase): # prefix to add the ndp proxy n_cidr = row.external_ids.get( constants.OVN_CIDRS_EXT_ID_KEY) - self._expose_provider_port(ips, row.datapath, - bridge_device, bridge_vlan, - None, [n_cidr]) + exposed_port = self._expose_provider_port( + ips, row.datapath, bridge_device, bridge_vlan, None, + [n_cidr]) else: - self._expose_provider_port(ips, row.datapath) + exposed_port = self._expose_provider_port(ips, + row.datapath) + if not exposed_port: + LOG.debug("Failure adding BGP route for logical port with " + "ip %s", ips) + return [] LOG.debug("Added BGP route for logical port with ip %s", ips) return ips # VM with FIP @@ -533,19 +562,24 @@ class OVNBGPDriver(driver_api.AgentDriverBase): if fip_address: LOG.debug("Adding BGP route for FIP with ip %s", fip_address) - self._expose_provider_port([fip_address], fip_datapath) - LOG.debug("Added BGP route for FIP with ip %s", + 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_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)): LOG.debug("Adding BGP route for FIP with ip %s", ips) - self._expose_provider_port(ips, row.datapath) - LOG.debug("Added BGP route for FIP with ip %s", ips) - return ips + if self._expose_provider_port(ips, row.datapath): + LOG.debug("Added BGP route for FIP with ip %s", ips) + return ips + LOG.debug("Failure adding BGP route for FIP with ip %s", ips) + return [] # CR-LRP Port elif (row.type == constants.OVN_CHASSISREDIRECT_VIF_PORT_TYPE and @@ -572,12 +606,11 @@ class OVNBGPDriver(driver_api.AgentDriverBase): 'bridge_device': bridge_device } - self._expose_cr_lrp_port(ips, mac, bridge_device, bridge_vlan, - router_datapath=row.datapath, - provider_datapath=cr_lrp_datapath, - cr_lrp_port=row.logical_port) - - return ips + if self._expose_cr_lrp_port(ips, mac, bridge_device, bridge_vlan, + router_datapath=row.datapath, + provider_datapath=cr_lrp_datapath, + cr_lrp_port=row.logical_port): + return ips return [] @lockutils.synchronized('bgp') @@ -620,6 +653,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase): LOG.debug("Deleting BGP route for logical port with ip %s", ips) n_cidr = None + withdrawn_port = False if row.type == constants.OVN_VIRTUAL_VIF_PORT_TYPE: virtual_provider_ports = ( self.sb_idl.get_virtual_ports_on_datapath_by_chassis( @@ -636,12 +670,18 @@ class OVNBGPDriver(driver_api.AgentDriverBase): n_cidr = row.external_ids.get( constants.OVN_CIDRS_EXT_ID_KEY) if n_cidr: - self._withdraw_provider_port(ips, row.datapath, - bridge_device, bridge_vlan, - None, [n_cidr]) + withdrawn_port = self._withdraw_provider_port( + ips, row.datapath, bridge_device, bridge_vlan, None, + [n_cidr]) else: - self._withdraw_provider_port(ips, row.datapath) + withdrawn_port = self._withdraw_provider_port(ips, + row.datapath) + if not withdrawn_port: + LOG.debug("Failure deleting BGP route for logical port " + "with ip %s", ips) + return LOG.debug("Deleted BGP route for logical port with ip %s", ips) + return # VM with FIP else: # FIPs are only supported with IPv4 @@ -651,8 +691,13 @@ class OVNBGPDriver(driver_api.AgentDriverBase): return LOG.debug("Deleting BGP route for FIP with ip %s", fip_address) - self._withdraw_provider_port([fip_address], fip_datapath) + if not self._withdraw_provider_port([fip_address], + fip_datapath): + LOG.debug("Failure deleting BGP route for FIP with ip %s", + fip_address) + return LOG.debug("Deleted BGP route for FIP with ip %s", fip_address) + return # FIP disassociation to VM elif row.type == constants.OVN_PATCH_VIF_PORT_TYPE: @@ -661,8 +706,12 @@ 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) - self._withdraw_provider_port(ips, row.datapath) + if not self._withdraw_provider_port(ips, row.datapath): + LOG.debug("Failure deleting BGP route for FIP with ip %s", + ips) + return LOG.debug("Deleted BGP route for FIP with ip %s", ips) + return # CR-LRP Port elif (row.type == constants.OVN_CHASSISREDIRECT_VIF_PORT_TYPE and @@ -799,9 +848,11 @@ class OVNBGPDriver(driver_api.AgentDriverBase): router_datapath, provider_datapath, cr_lrp_port): LOG.debug("Adding BGP route for CR-LRP Port %s", ips) ips_without_mask = [ip.split("/")[0] for ip in ips] - self._expose_provider_port(ips_without_mask, provider_datapath, - bridge_device, bridge_vlan, - lladdr=mac, proxy_cidrs=ips) + if not self._expose_provider_port(ips_without_mask, provider_datapath, + bridge_device, bridge_vlan, + lladdr=mac, proxy_cidrs=ips): + LOG.debug("Failure adding BGP route for CR-LRP Port %s", ips) + return False LOG.debug("Added BGP route for CR-LRP Port %s", ips) # Check if there are networks attached to the router, @@ -820,6 +871,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase): self._expose_ovn_lb_on_provider(ovn_lb_ip, ovn_lb_port, cr_lrp_port) + return True def _withdraw_cr_lrp_port(self, ips, mac, bridge_device, bridge_vlan, provider_datapath, cr_lrp_port): @@ -839,11 +891,12 @@ class OVNBGPDriver(driver_api.AgentDriverBase): if (len(cr_lrps_on_same_provider) <= 1): proxy_cidrs.append(ip) - self._withdraw_provider_port(ips_without_mask, provider_datapath, - bridge_device=bridge_device, - bridge_vlan=bridge_vlan, - lladdr=mac, proxy_cidrs=proxy_cidrs) - + if not self._withdraw_provider_port( + ips_without_mask, provider_datapath, + bridge_device=bridge_device, bridge_vlan=bridge_vlan, + lladdr=mac, proxy_cidrs=proxy_cidrs): + LOG.debug("Failure deleting BGP route for CR-LRP Port %s", ips) + return False LOG.debug("Deleted BGP route for CR-LRP Port %s", ips) # Check if there are networks attached to the router, @@ -865,6 +918,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase): except KeyError: LOG.debug("Gateway port %s already cleanup from the agent.", cr_lrp_port) + return True def _expose_lrp_port(self, ip, lrp, associated_cr_lrp, subnet_datapath, exposed_ips=None, ovn_ip_rules=None): diff --git a/ovn_bgp_agent/drivers/openstack/utils/wire.py b/ovn_bgp_agent/drivers/openstack/utils/wire.py index 74832f6b..ea1d9bfe 100644 --- a/ovn_bgp_agent/drivers/openstack/utils/wire.py +++ b/ovn_bgp_agent/drivers/openstack/utils/wire.py @@ -26,6 +26,8 @@ LOG = logging.getLogger(__name__) def wire_provider_port(routing_tables_routes, port_ips, bridge_device, bridge_vlan, routing_table, proxy_cidrs, lladdr=None): + if not bridge_device: + return False for ip in port_ips: try: if lladdr: @@ -52,6 +54,8 @@ def wire_provider_port(routing_tables_routes, port_ips, bridge_device, def unwire_provider_port(routing_tables_routes, port_ips, bridge_device, bridge_vlan, routing_table, proxy_cidrs, lladdr=None): + if not bridge_device: + return False for ip in port_ips: if lladdr: if linux_net.get_ip_version(ip) == constants.IP_VERSION_6: @@ -83,6 +87,8 @@ def unwire_provider_port(routing_tables_routes, port_ips, bridge_device, def wire_lrp_port(routing_tables_routes, ip, bridge_device, bridge_vlan, routing_table, cr_lrp_ips): + if not bridge_device: + return False LOG.debug("Adding IP Rules for network %s", ip) try: linux_net.add_ip_rule(ip, routing_table) @@ -113,6 +119,8 @@ def wire_lrp_port(routing_tables_routes, ip, bridge_device, bridge_vlan, def unwire_lrp_port(routing_tables_routes, ip, bridge_device, bridge_vlan, routing_table, cr_lrp_ips): + if not bridge_device: + return False LOG.debug("Deleting IP Rules for network %s", ip) try: linux_net.del_ip_rule(ip, routing_table, bridge_device) 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 777eb7a0..46d70085 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 @@ -20,10 +20,12 @@ from oslo_config import cfg from ovn_bgp_agent import config from ovn_bgp_agent import constants from ovn_bgp_agent.drivers.openstack import ovn_bgp_driver +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 from ovn_bgp_agent import exceptions as agent_exc from ovn_bgp_agent.tests import base as test_base from ovn_bgp_agent.tests.unit import fakes @@ -287,6 +289,24 @@ class TestOVNBGPDriver(test_base.TestCase): 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_no_device(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 = (None, None) + ret = self.bgp_driver._expose_provider_port(port_ips, + provider_datapath) + + self.assertEqual(False, ret) + mock_add_ips_dev.assert_not_called() + mock_add_rule.assert_not_called() + 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') @@ -298,8 +318,10 @@ class TestOVNBGPDriver(test_base.TestCase): 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) + ret = self.bgp_driver._expose_provider_port(port_ips, + provider_datapath) + self.assertEqual(False, ret) mock_add_ips_dev.assert_not_called() mock_add_rule.assert_called_once_with( self.ipv4, 'fake-table') @@ -464,7 +486,7 @@ class TestOVNBGPDriver(test_base.TestCase): @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,): + mock_del_ips_dev): port_ips = [self.ipv4] provider_datapath = 'fake-provider-dp' mock_get_bridge = mock.patch.object( @@ -479,6 +501,26 @@ class TestOVNBGPDriver(test_base.TestCase): mock_del_route.assert_called_once_with( mock.ANY, self.ipv4, 'fake-table', self.bridge, vlan=10) + @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_no_device(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 = (None, None) + ret = self.bgp_driver._withdraw_provider_port(port_ips, + provider_datapath) + + self.assertEqual(False, ret) + mock_del_ips_dev.assert_called_once_with( + CONF.bgp_nic, [self.ipv4]) + mock_del_rule.assert_not_called() + mock_del_route.assert_not_called() + @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') @@ -847,21 +889,20 @@ class TestOVNBGPDriver(test_base.TestCase): self.bgp_driver._process_ovn_lb(ip, row, action) - if action == constants.EXPOSE: - if provider: - mock_expose_remote_ip.assert_not_called() - mock_withdraw_remote_ip.assert_not_called() - else: + if provider: + mock_expose_remote_ip.assert_not_called() + mock_withdraw_remote_ip.assert_not_called() + else: + if action == constants.EXPOSE: mock_expose_remote_ip.assert_called_once_with([ip], row) mock_withdraw_remote_ip.assert_not_called() - if action == constants.WITHDRAW: - if provider: - mock_expose_remote_ip.assert_not_called() - mock_withdraw_remote_ip.assert_not_called() - else: + elif action == constants.WITHDRAW: mock_expose_remote_ip.assert_not_called() mock_withdraw_remote_ip.assert_called_once_with([ip], row) + else: + mock_expose_remote_ip.assert_not_called() + mock_withdraw_remote_ip.assert_not_called() def test__process_ovn_lb_expose_provider(self): self._test_process_ovn_lb(action=constants.EXPOSE, provider=True) @@ -875,22 +916,65 @@ class TestOVNBGPDriver(test_base.TestCase): def test__process_ovn_lb_withdraw_no_provider(self): self._test_process_ovn_lb(action=constants.WITHDRAW) - def test__expose_ovn_lb_on_provider(self): + def test__process_ovn_lb_unknown_action(self): + self._test_process_ovn_lb(action="fake-action") + + def test__process_ovn_lb_datapath_exception(self): + ip = 'fake-vip-ip' + row = fakes.create_object({ + 'logical_port': 'fake-vip', + 'type': constants.OVN_VM_VIF_PORT_TYPE, + 'datapath': 'fake-provider-dp'}) + mock_expose_remote_ip = mock.patch.object( + self.bgp_driver, '_expose_remote_ip').start() + mock_withdraw_remote_ip = mock.patch.object( + self.bgp_driver, '_withdraw_remote_ip').start() + self.sb_idl.is_provider_network.side_effect = ( + agent_exc.DatapathNotFound(datapath=row.datapath)) + + self.bgp_driver._process_ovn_lb(ip, row, mock.ANY) + + mock_expose_remote_ip.assert_not_called() + mock_withdraw_remote_ip.assert_not_called() + + 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.bgp_driver.expose_ovn_lb_on_provider( self.ipv4, self.loadbalancer_vip_port, self.cr_lrp0) # Assert that the add methods were called mock_expose_provider_port.assert_called_once_with( [self.ipv4], None, bridge_device=self.bridge, bridge_vlan=None) + def test__expose_ovn_lb_on_provider_failure(self): + mock_expose_provider_port = mock.patch.object( + self.bgp_driver, '_expose_provider_port').start() + mock_expose_provider_port.return_value = False + ret = self.bgp_driver._expose_ovn_lb_on_provider( + self.ipv4, self.loadbalancer_vip_port, self.cr_lrp0) + + # Assert that the add methods were called + mock_expose_provider_port.assert_called_once_with( + [self.ipv4], None, bridge_device=self.bridge, bridge_vlan=None) + self.assertEqual(False, ret) + + def test__expose_ovn_lb_on_provider_keyerror(self): + mock_expose_provider_port = mock.patch.object( + self.bgp_driver, '_expose_provider_port').start() + ret = self.bgp_driver._expose_ovn_lb_on_provider( + self.ipv4, self.loadbalancer_vip_port, 'wrong-cr-logical-port') + + # Assert that the add methods were called + mock_expose_provider_port.assert_not_called() + self.assertEqual(False, ret) + @mock.patch.object(linux_net, 'del_ip_route') @mock.patch.object(linux_net, 'del_ip_rule') @mock.patch.object(linux_net, 'del_ips_from_dev') - def test__withdraw_ovn_lb_on_provider( + def test_withdraw_ovn_lb_on_provider( self, mock_del_ip_dev, mock_del_rule, mock_del_route): - self.bgp_driver._withdraw_ovn_lb_on_provider( + self.bgp_driver.withdraw_ovn_lb_on_provider( self.loadbalancer_vip_port, self.cr_lrp0) # Assert that the del methods were called @@ -908,6 +992,28 @@ class TestOVNBGPDriver(test_base.TestCase): self.bridge, vlan=None)] mock_del_route.assert_has_calls(expected_calls) + @mock.patch.object(linux_net, 'del_ip_route') + @mock.patch.object(linux_net, 'del_ip_rule') + @mock.patch.object(linux_net, 'del_ips_from_dev') + def test__withdraw_ovn_lb_on_provider_keyerror( + self, mock_del_ip_dev, mock_del_rule, mock_del_route): + ret = self.bgp_driver._withdraw_ovn_lb_on_provider( + self.loadbalancer_vip_port, 'wrong-cr-logical-port') + + # Assert that the del methods were called + self.assertEqual(False, ret) + mock_del_ip_dev.assert_not_called() + mock_del_rule.assert_not_called() + mock_del_route.assert_not_called() + + def test__withdraw_ovn_lb_on_provider_failure(self): + mock_withdraw_provider_port = mock.patch.object( + self.bgp_driver, '_withdraw_provider_port').start() + mock_withdraw_provider_port.return_value = False + ret = self.bgp_driver._withdraw_ovn_lb_on_provider( + self.loadbalancer_vip_port, self.cr_lrp0) + self.assertEqual(False, 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') @@ -918,7 +1024,7 @@ class TestOVNBGPDriver(test_base.TestCase): self.bgp_driver, '_get_bridge_for_datapath').start() mock_get_bridge.return_value = (self.bridge, 10) row = fakes.create_object({ - 'name': 'fake-row', + 'logical_port': 'fake-row', 'type': constants.OVN_VM_VIF_PORT_TYPE, 'datapath': 'fake-dp'}) @@ -939,12 +1045,55 @@ class TestOVNBGPDriver(test_base.TestCase): self.bridge, vlan=10)] mock_add_route.assert_has_calls(expected_calls) + @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_on_provider_network_datapath_not_found( + self, mock_add_ip_dev, mock_add_rule, mock_add_route): + self.sb_idl.is_provider_network.side_effect = ( + agent_exc.DatapathNotFound(datapath="fake-dp")) + row = fakes.create_object({ + 'logical_port': 'fake-row', + 'type': constants.OVN_VM_VIF_PORT_TYPE, + '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(wire_utils, 'wire_provider_port') + @mock.patch.object(bgp_utils, 'announce_ips') + def test__expose_ip_vm_on_provider_network_expose_failure( + self, mock_bgp_announce, mock_wire_port): + self.sb_idl.is_provider_network.return_value = True + mock_get_bridge = mock.patch.object( + self.bgp_driver, '_get_bridge_for_datapath').start() + mock_get_bridge.return_value = (self.bridge, 10) + mock_wire_port.return_value = False + row = fakes.create_object({ + 'logical_port': 'fake-row', + 'type': constants.OVN_VM_VIF_PORT_TYPE, + '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_wire_port.assert_called_once() + mock_bgp_announce.assert_not_called() + @mock.patch.object(linux_net, 'add_ndp_proxy') @mock.patch.object(linux_net, 'get_ip_version') @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_virtual_port_on_provider_network( + def test__expose_ip_virtual_port_on_provider_network( self, mock_add_ip_dev, mock_add_rule, mock_add_route, mock_ip_version, mock_add_ndp_proxy): self.sb_idl.is_provider_network.return_value = True @@ -953,15 +1102,16 @@ class TestOVNBGPDriver(test_base.TestCase): mock_get_bridge.return_value = (self.bridge, 10) mock_ip_version.return_value = constants.IP_VERSION_6 row = fakes.create_object({ - 'name': 'fake-row', + 'logical_port': 'fake-row', 'type': constants.OVN_VIRTUAL_VIF_PORT_TYPE, 'datapath': 'fake-dp', 'external_ids': {'neutron:cidrs': '{}/128'.format(self.ipv6)}}) ips = [self.ipv4, self.ipv6] - self.bgp_driver.expose_ip(ips, row) + ret = self.bgp_driver._expose_ip(ips, row) # Assert that the add methods were called + self.assertEqual(ips, ret) mock_add_ip_dev.assert_called_once_with( CONF.bgp_nic, ips) @@ -977,10 +1127,39 @@ class TestOVNBGPDriver(test_base.TestCase): mock_add_ndp_proxy.assert_called_once_with( '{}/128'.format(self.ipv6), self.bridge, 10) + @mock.patch.object(linux_net, 'add_ndp_proxy') + @mock.patch.object(linux_net, 'get_ip_version') @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( + def test__expose_ip_virtual_port_on_provider_network_expose_failure( + self, mock_add_ip_dev, mock_add_rule, mock_add_route, + mock_ip_version, mock_add_ndp_proxy): + self.sb_idl.is_provider_network.return_value = True + mock_get_bridge = mock.patch.object( + self.bgp_driver, '_get_bridge_for_datapath').start() + mock_get_bridge.return_value = (None, None) + mock_ip_version.return_value = constants.IP_VERSION_6 + row = fakes.create_object({ + 'logical_port': 'fake-row', + 'type': constants.OVN_VIRTUAL_VIF_PORT_TYPE, + 'datapath': 'fake-dp', + 'external_ids': {'neutron:cidrs': '{}/128'.format(self.ipv6)}}) + + 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_add_ndp_proxy.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') + 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.get_fip_associated.return_value = ( @@ -989,15 +1168,15 @@ class TestOVNBGPDriver(test_base.TestCase): self.bgp_driver, '_get_bridge_for_datapath').start() mock_get_bridge.return_value = (self.bridge, 10) row = fakes.create_object({ - 'name': 'fake-row', 'type': constants.OVN_VM_VIF_PORT_TYPE, 'logical_port': 'fake-logical-port', 'datapath': 'fake-dp'}) ips = [self.ipv4, self.ipv6] - self.bgp_driver.expose_ip(ips, row) + ret = self.bgp_driver._expose_ip(ips, row) # Assert that the add methods were called + self.assertEqual([self.fip], ret) mock_add_ip_dev.assert_called_once_with( CONF.bgp_nic, [self.fip]) mock_add_rule.assert_called_once_with( @@ -1008,20 +1187,20 @@ class TestOVNBGPDriver(test_base.TestCase): @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_fip_address( + def test__expose_ip_vm_with_fip_no_fip_address( self, mock_add_ip_dev, mock_add_rule, mock_add_route): self.sb_idl.is_provider_network.return_value = False self.sb_idl.get_fip_associated.return_value = (None, None) row = fakes.create_object({ - 'name': 'fake-row', 'type': constants.OVN_VM_VIF_PORT_TYPE, 'logical_port': 'fake-logical-port', 'datapath': 'fake-dp'}) ips = [self.ipv4, self.ipv6] - self.bgp_driver.expose_ip(ips, row) + ret = self.bgp_driver._expose_ip(ips, row) # Assert that the add methods were not called + self.assertEqual([], ret) mock_add_ip_dev.assert_not_called() mock_add_rule.assert_not_called() mock_add_route.assert_not_called() @@ -1029,7 +1208,7 @@ class TestOVNBGPDriver(test_base.TestCase): @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_fip_association_to_vm( + def test__expose_ip_fip_association_to_vm( self, mock_add_ip_dev, mock_add_rule, mock_add_route): self.sb_idl.is_provider_network.return_value = False self.sb_idl.is_port_on_chassis.return_value = True @@ -1037,15 +1216,15 @@ class TestOVNBGPDriver(test_base.TestCase): self.bgp_driver, '_get_bridge_for_datapath').start() mock_get_bridge.return_value = (self.bridge, 10) row = fakes.create_object({ - 'name': 'fake-row', 'type': constants.OVN_PATCH_VIF_PORT_TYPE, 'logical_port': 'fake-logical-port', 'datapath': 'fake-dp'}) ips = [self.ipv4, self.ipv6] - self.bgp_driver.expose_ip(ips, row, associated_port=True) + ret = self.bgp_driver._expose_ip(ips, row, associated_port=True) # Assert that the add methods were called + self.assertEqual(ips, ret) mock_add_ip_dev.assert_called_once_with( CONF.bgp_nic, ips) @@ -1064,7 +1243,7 @@ class TestOVNBGPDriver(test_base.TestCase): @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_chassisredirect_port( + def test__expose_ip_chassisredirect_port( self, mock_add_ip_dev, mock_add_rule, mock_add_route, mock_ip_version, mock_ndp_proxy): self.sb_idl.get_provider_datapath_from_cr_lrp.return_value = ( @@ -1094,7 +1273,6 @@ class TestOVNBGPDriver(test_base.TestCase): mock_ip_version.side_effect = (constants.IP_VERSION_4, constants.IP_VERSION_6) row = fakes.create_object({ - 'name': 'fake-row', 'type': constants.OVN_CHASSISREDIRECT_VIF_PORT_TYPE, 'logical_port': self.cr_lrp0, 'mac': ['{} {} {}'.format(self.mac, self.ipv4, self.ipv6)], @@ -1108,9 +1286,10 @@ class TestOVNBGPDriver(test_base.TestCase): self.bgp_driver, '_expose_ovn_lb_on_provider').start() ips = [self.ipv4, self.ipv6] - self.bgp_driver.expose_ip(ips, row) + ret = self.bgp_driver._expose_ip(ips, row) # Assert that the add methods were called + self.assertEqual(ips, ret) mock_add_ip_dev.assert_called_once_with( CONF.bgp_nic, ips) @@ -1137,6 +1316,31 @@ class TestOVNBGPDriver(test_base.TestCase): mock_expose_ovn_lb.assert_called_once_with( ovn_lb_vip, 'fake-vip-port', self.cr_lrp0) + @mock.patch.object(linux_net, 'add_ndp_proxy') + @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_chassisredirect_port_no_datapath( + self, mock_add_ip_dev, mock_add_rule, mock_add_route, + mock_ndp_proxy): + self.sb_idl.get_provider_datapath_from_cr_lrp.return_value = None + + row = fakes.create_object({ + 'type': constants.OVN_CHASSISREDIRECT_VIF_PORT_TYPE, + 'logical_port': self.cr_lrp0, + 'mac': ['{} {} {}'.format(self.mac, self.ipv4, self.ipv6)], + 'datapath': 'fake-router-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_ndp_proxy.assert_not_called() + @mock.patch.object(linux_net, 'del_ip_route') @mock.patch.object(linux_net, 'del_ip_rule') @mock.patch.object(linux_net, 'del_ips_from_dev') @@ -1147,7 +1351,7 @@ class TestOVNBGPDriver(test_base.TestCase): self.bgp_driver, '_get_bridge_for_datapath').start() mock_get_bridge.return_value = (self.bridge, 10) row = fakes.create_object({ - 'name': 'fake-row', + 'logical_port': 'fake-row', 'type': constants.OVN_VM_VIF_PORT_TYPE, 'datapath': 'fake-dp'}) @@ -1181,7 +1385,7 @@ class TestOVNBGPDriver(test_base.TestCase): self.bgp_driver, '_get_bridge_for_datapath').start() mock_get_bridge.return_value = (self.bridge, 10) row = fakes.create_object({ - 'name': 'fake-row', + 'logical_port': 'fake-row', 'type': constants.OVN_VIRTUAL_VIF_PORT_TYPE, 'datapath': 'fake-dp', 'external_ids': {'neutron:cidrs': '{}/128'.format(self.ipv6)}}) @@ -1220,7 +1424,6 @@ class TestOVNBGPDriver(test_base.TestCase): self.bgp_driver, '_get_bridge_for_datapath').start() mock_get_bridge.return_value = (self.bridge, 10) row = fakes.create_object({ - 'name': 'fake-row', 'type': constants.OVN_VM_VIF_PORT_TYPE, 'logical_port': 'fake-logical-port', 'datapath': 'fake-dp'}) @@ -1244,7 +1447,6 @@ class TestOVNBGPDriver(test_base.TestCase): self.sb_idl.is_provider_network.return_value = False self.sb_idl.get_fip_associated.return_value = (None, None) row = fakes.create_object({ - 'name': 'fake-row', 'type': constants.OVN_VM_VIF_PORT_TYPE, 'logical_port': 'fake-logical-port', 'datapath': 'fake-dp'}) @@ -1268,7 +1470,6 @@ class TestOVNBGPDriver(test_base.TestCase): self.bgp_driver, '_get_bridge_for_datapath').start() mock_get_bridge.return_value = (self.bridge, 10) row = fakes.create_object({ - 'name': 'fake-row', 'type': constants.OVN_PATCH_VIF_PORT_TYPE, 'logical_port': 'fake-logical-port', 'datapath': 'fake-dp'}) @@ -1307,7 +1508,6 @@ class TestOVNBGPDriver(test_base.TestCase): constants.IP_VERSION_6, constants.IP_VERSION_6) row = fakes.create_object({ - 'name': 'fake-row', 'type': constants.OVN_CHASSISREDIRECT_VIF_PORT_TYPE, 'logical_port': self.cr_lrp0, 'mac': ['{} {} {}'.format(self.mac, self.ipv4, self.ipv6)], @@ -1561,6 +1761,29 @@ class TestOVNBGPDriver(test_base.TestCase): mock_expose_ovn_lb.assert_called_once_with( 'fake-vip-ip', 'fake-vip-port', self.cr_lrp0) + def test__expose_cr_lrp_port_failure(self): + mock_expose_provider_port = mock.patch.object( + self.bgp_driver, '_expose_provider_port').start() + mock_expose_provider_port.return_value = False + mock_process_lrp_port = mock.patch.object( + self.bgp_driver, '_process_lrp_port').start() + mock_expose_ovn_lb = mock.patch.object( + self.bgp_driver, '_expose_ovn_lb_on_provider').start() + ips = [self.ipv4, self.ipv6] + + ret = self.bgp_driver._expose_cr_lrp_port( + ips, self.mac, self.bridge, None, router_datapath='fake-router-dp', + provider_datapath='fake-provider-dp', cr_lrp_port=self.cr_lrp0) + + self.assertEqual(False, ret) + + ips_without_mask = [ip.split("/")[0] for ip in ips] + mock_expose_provider_port.assert_called_once_with( + ips_without_mask, 'fake-provider-dp', self.bridge, None, + lladdr=self.mac, proxy_cidrs=ips) + mock_process_lrp_port.assert_not_called() + mock_expose_ovn_lb.assert_not_called() + @mock.patch.object(linux_net, 'get_ip_version') def test__withdraw_cr_lrp_port(self, mock_ip_version): mock_withdraw_provider_port = mock.patch.object( @@ -1597,6 +1820,42 @@ class TestOVNBGPDriver(test_base.TestCase): mock_withdraw_ovn_lb_on_provider.assert_called_once_with( ovn_lb_vip_port, 'gateway_port') + @mock.patch.object(linux_net, 'get_ip_version') + def test__withdraw_cr_lrp_port_withdraw_failure(self, mock_ip_version): + mock_withdraw_provider_port = mock.patch.object( + self.bgp_driver, '_withdraw_provider_port').start() + mock_withdraw_provider_port.return_value = False + mock_withdraw_lrp_port = mock.patch.object( + self.bgp_driver, '_withdraw_lrp_port').start() + mock_withdraw_ovn_lb_on_provider = mock.patch.object( + self.bgp_driver, '_withdraw_ovn_lb_on_provider').start() + + ips = [self.ipv4, self.ipv6] + mock_ip_version.side_effect = [constants.IP_VERSION_4, + constants.IP_VERSION_6] + ovn_lb_vip_port = mock.Mock() + gateway = { + 'ips': ips, + 'provider_datapath': 'fake-provider-dp', + 'subnets_cidr': ['192.168.1.1/24'], + 'bridge_device': self.bridge, + 'bridge_vlan': 10, + 'mac': self.mac, + 'ovn_lb_vips': [ovn_lb_vip_port]} + self.bgp_driver.ovn_local_cr_lrps = {'gateway_port': gateway} + + ret = self.bgp_driver._withdraw_cr_lrp_port( + ips, self.mac, self.bridge, 10, + provider_datapath='fake-provider-dp', cr_lrp_port='gateway_port') + + self.assertEqual(False, ret) + ips_without_mask = [ip.split("/")[0] for ip in ips] + mock_withdraw_provider_port.assert_called_once_with( + ips_without_mask, 'fake-provider-dp', bridge_device=self.bridge, + bridge_vlan=10, lladdr=self.mac, proxy_cidrs=[self.ipv6]) + mock_withdraw_lrp_port.assert_not_called() + mock_withdraw_ovn_lb_on_provider.assert_not_called() + @mock.patch.object(linux_net, 'add_ip_route') @mock.patch.object(linux_net, 'add_ip_rule') @mock.patch.object(linux_net, 'get_ip_version')