From 0c6a71314b97e15d0aeb34c6e147a835e2c5c9e2 Mon Sep 17 00:00:00 2001 From: Luis Tomas Bolivar Date: Thu, 19 Oct 2023 13:40:21 +0200 Subject: [PATCH] Add support at NB BGP Driver for exposing CR-LRPs Change-Id: I5c78f3aae9e02f862d1515aa40f17ff3b7e68bc9 --- ovn_bgp_agent/constants.py | 2 + .../drivers/openstack/nb_ovn_bgp_driver.py | 80 +++++-- ovn_bgp_agent/drivers/openstack/utils/ovn.py | 17 +- .../openstack/watchers/base_watcher.py | 9 + .../openstack/watchers/nb_bgp_watcher.py | 98 +++++++- .../openstack/test_nb_ovn_bgp_driver.py | 209 +++++++++++++----- .../unit/drivers/openstack/utils/test_ovn.py | 25 ++- .../openstack/watchers/test_base_watcher.py | 23 ++ .../openstack/watchers/test_nb_bgp_watcher.py | 173 ++++++++++++++- 9 files changed, 536 insertions(+), 100 deletions(-) diff --git a/ovn_bgp_agent/constants.py b/ovn_bgp_agent/constants.py index 9ed38332..b67f080d 100644 --- a/ovn_bgp_agent/constants.py +++ b/ovn_bgp_agent/constants.py @@ -20,6 +20,7 @@ OVN_PATCH_VIF_PORT_TYPE = "patch" OVN_CHASSISREDIRECT_VIF_PORT_TYPE = "chassisredirect" OVN_LOCALNET_VIF_PORT_TYPE = "localnet" OVN_DNAT_AND_SNAT = "dnat_and_snat" +OVN_CR_LRP_PORT_TYPE = 'crlrp' OVN_CIDRS_EXT_ID_KEY = 'neutron:cidrs' OVN_PORT_NAME_EXT_ID_KEY = 'neutron:port_name' @@ -67,6 +68,7 @@ EXPOSE = "expose" WITHDRAW = "withdraw" OVN_REQUESTED_CHASSIS = "requested-chassis" +OVN_STATUS_CHASSIS = "hosting-chassis" OVN_HOST_ID_EXT_ID_KEY = "neutron:host_id" OVN_CHASSIS_AT_OPTIONS = "options" OVN_CHASSIS_AT_EXT_IDS = "external-ids" 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 5a283ca6..ef16c767 100644 --- a/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py +++ b/ovn_bgp_agent/drivers/openstack/nb_ovn_bgp_driver.py @@ -34,7 +34,8 @@ LOG = logging.getLogger(__name__) # LOG.setLevel(logging.DEBUG) # logging.basicConfig(level=logging.DEBUG) -OVN_TABLES = ['Logical_Switch_Port', 'NAT', 'Logical_Switch'] +OVN_TABLES = ['Logical_Switch_Port', 'NAT', 'Logical_Switch', + 'Logical_Router_Port'] LOCAL_CLUSTER_OVN_TABLES = ['Logical_Switch', 'Logical_Switch_Port', 'Logical_Router', 'Logical_Router_Port', 'Logical_Router_Policy', @@ -97,6 +98,8 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): self.ovs_idl = ovs.OvsIdl() self.ovs_idl.start(CONF.ovsdb_connection) self.chassis = self.ovs_idl.get_own_chassis_name() + self.chassis_id = self.ovs_idl.get_own_chassis_id() + # NOTE(ltomasbo): remote should point to NB DB port instead of SB DB, # so changing 6642 by 6641 self.ovn_remote = self.ovs_idl.get_ovn_remote(nb=True) @@ -144,7 +147,8 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): "LogicalSwitchPortFIPDeleteEvent", "LocalnetCreateDeleteEvent"]) if self._expose_tenant_networks: - events.update([]) + events.update(["ChassisRedirectCreateEvent", + "ChassisRedirectDeleteEvent"]) return events @lockutils.synchronized('nbbgp') @@ -167,13 +171,17 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): routing_tables=self.ovn_routing_tables)) LOG.debug("Syncing current routes.") + # add missing reoutes/ips for OVN router gateway ports + ports = self.nb_idl.get_active_cr_lrp_on_chassis(self.chassis_id) + for port in ports: + self._ensure_crlrp_exposed(port) # add missing routes/ips for IPs on provider network - ports = self.nb_idl.get_active_ports_on_chassis(self.chassis) + ports = self.nb_idl.get_active_lsp_on_chassis(self.chassis) for port in ports: if port.type not in [constants.OVN_VM_VIF_PORT_TYPE, constants.OVN_VIRTUAL_VIF_PORT_TYPE]: continue - self._ensure_port_exposed(port) + self._ensure_lsp_exposed(port) # remove extra wiring leftovers wire_utils.cleanup_wiring(self.nb_idl, @@ -183,7 +191,7 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): self.ovn_routing_tables, self.ovn_routing_tables_routes) - def _ensure_port_exposed(self, port): + def _ensure_lsp_exposed(self, port): port_fip = port.external_ids.get(constants.OVN_FIP_EXT_ID_KEY) if port_fip: external_ip, external_mac, ls_name = ( @@ -219,8 +227,31 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): ips = port.addresses[0].strip().split(' ')[1:] mac = port.addresses[0].strip().split(' ')[0] self._expose_ip(ips, mac, logical_switch, bridge_device, bridge_vlan, - port.type, port.external_ids.get( - constants.OVN_CIDRS_EXT_ID_KEY)) + port.type, [port.external_ids.get( + constants.OVN_CIDRS_EXT_ID_KEY)]) + + def _ensure_crlrp_exposed(self, port): + if not port.networks: + return + + logical_switch = port.external_ids.get( + constants.OVN_LS_NAME_EXT_ID_KEY) + if not logical_switch: + return + localnet, bridge_device, bridge_vlan = self._get_ls_localnet_info( + logical_switch) + + if not bridge_device: + return + + self.ovn_provider_ls[logical_switch] = { + 'bridge_device': bridge_device, + 'bridge_vlan': bridge_vlan, + 'localnet': localnet} + ips = [net.split("/")[0] for net in port.networks] + self._expose_ip(ips, port.mac, logical_switch, bridge_device, + bridge_vlan, constants.OVN_CR_LRP_PORT_TYPE, + port.networks) def _expose_provider_port(self, port_ips, mac, logical_switch, bridge_device, bridge_vlan, localnet, @@ -272,7 +303,7 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): return bridge_device, bridge_vlan @lockutils.synchronized('nbbgp') - def expose_ip(self, ips, row): + def expose_ip(self, ips, ips_info): '''Advertice BGP route by adding IP to device. This methods ensures BGP advertises the IP of the VM in the provider @@ -284,7 +315,7 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): VRF), and adds the IP of: - VM IP on the provider network ''' - logical_switch = row.external_ids.get(constants.OVN_LS_NAME_EXT_ID_KEY) + logical_switch = ips_info.get('logical_switch') if not logical_switch: return False localnet, bridge_device, bridge_vlan = self._get_ls_localnet_info( @@ -301,24 +332,24 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): 'bridge_device': bridge_device, 'bridge_vlan': bridge_vlan, 'localnet': localnet} - mac = row.addresses[0].strip().split(' ')[0] + mac = ips_info.get('mac') return self._expose_ip(ips, mac, logical_switch, bridge_device, - bridge_vlan, port_type=row.type, - cidr=row.external_ids.get( - constants.OVN_CIDRS_EXT_ID_KEY)) + bridge_vlan, port_type=ips_info['type'], + cidrs=ips_info['cidrs']) def _expose_ip(self, ips, mac, logical_switch, bridge_device, bridge_vlan, - port_type, cidr): + port_type, cidrs): LOG.debug("Adding BGP route for logical port with ip %s", ips) localnet = self.ovn_provider_ls[logical_switch]['localnet'] - if cidr and port_type == constants.OVN_VIRTUAL_VIF_PORT_TYPE: + if cidrs and port_type in [constants.OVN_VIRTUAL_VIF_PORT_TYPE, + constants.OVN_CR_LRP_PORT_TYPE]: # NOTE: For Amphora Load Balancer with IPv6 VIP on the provider # network, we need a NDP Proxy so that the traffic from the # amphora can properly be redirected back if not self._expose_provider_port(ips, mac, logical_switch, bridge_device, bridge_vlan, - localnet, [cidr]): + localnet, cidrs): return [] else: @@ -330,7 +361,7 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): return ips @lockutils.synchronized('nbbgp') - def withdraw_ip(self, ips, row): + def withdraw_ip(self, ips, ips_info): '''Withdraw BGP route by removing IP from device. This methods ensures BGP withdraw an advertised IP of a VM, either @@ -342,7 +373,7 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): VRF), and removes the IP of: - VM IP on the provider network ''' - logical_switch = row.external_ids.get(constants.OVN_LS_NAME_EXT_ID_KEY) + logical_switch = ips_info.get('logical_switch') if not logical_switch: return _, bridge_device, bridge_vlan = self._get_ls_localnet_info( @@ -352,12 +383,13 @@ class NBOVNBGPDriver(driver_api.AgentDriverBase): return proxy_cidr = None - if row.type == constants.OVN_VIRTUAL_VIF_PORT_TYPE: - n_cidr = row.external_ids.get(constants.OVN_CIDRS_EXT_ID_KEY) - if n_cidr and (linux_net.get_ip_version(n_cidr) == - constants.IP_VERSION_6): - if not self.nb_idl.ls_has_virtual_ports(logical_switch): - proxy_cidr = n_cidr + + if ips_info['type'] in [constants.OVN_VIRTUAL_VIF_PORT_TYPE, + constants.OVN_CR_LRP_PORT_TYPE]: + for n_cidr in ips_info['cidrs']: + if linux_net.get_ip_version(n_cidr) == constants.IP_VERSION_6: + if not self.nb_idl.ls_has_virtual_ports(logical_switch): + proxy_cidr = n_cidr LOG.debug("Deleting BGP route for logical port with ip %s", ips) if proxy_cidr: self._withdraw_provider_port(ips, logical_switch, bridge_device, diff --git a/ovn_bgp_agent/drivers/openstack/utils/ovn.py b/ovn_bgp_agent/drivers/openstack/utils/ovn.py index bede793c..ddc6e89c 100644 --- a/ovn_bgp_agent/drivers/openstack/utils/ovn.py +++ b/ovn_bgp_agent/drivers/openstack/utils/ovn.py @@ -409,7 +409,7 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend): nat_info = cmd.execute(check_error=True) return nat_info[0] if nat_info else [] - def get_active_ports_on_chassis(self, chassis): + def get_active_lsp_on_chassis(self, chassis): ports = [] cmd = self.db_find_rows('Logical_Switch_Port', ('up', '=', True)) for row in cmd.execute(check_error=True): @@ -423,6 +423,21 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend): ports.append(row) return ports + def get_active_cr_lrp_on_chassis(self, chassis): + ports = [] + rows = self.db_list_rows('Logical_Router_Port').execute( + check_error=True) + + for row in rows: + if not hasattr(row, 'status'): + LOG.warning("OVN version does not include support for status " + "information. Therefore router ports and tenant " + "IPs cannot be exposed.") + break + if row.status.get(constants.OVN_STATUS_CHASSIS) == chassis: + ports.append(row) + return ports + # FIXME(ltomasbo): This can be removed once ovsdbapp version is >=2.3.0 def ls_get_localnet_ports(self, logical_switch, if_exists=True): return LSGetLocalnetPortsCommand(self, logical_switch, diff --git a/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py b/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py index bd828546..8594cc92 100644 --- a/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py +++ b/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py @@ -72,3 +72,12 @@ class LSPChassisEvent(Event): return (row.options[constants.OVN_REQUESTED_CHASSIS], constants.OVN_CHASSIS_AT_OPTIONS) return None, None + + +class LRPChassisEvent(Event): + def __init__(self, bgp_agent, events): + self.agent = bgp_agent + table = 'Logical_Router_Port' + super(LRPChassisEvent, self).__init__( + events, table, None) + self.event_name = self.__class__.__name__ diff --git a/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py b/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py index 01055a24..fec52ac9 100644 --- a/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py +++ b/ovn_bgp_agent/drivers/openstack/watchers/nb_bgp_watcher.py @@ -66,7 +66,16 @@ class LogicalSwitchPortProviderCreateEvent(base_watcher.LSPChassisEvent): return with _SYNC_STATE_LOCK.read_lock(): ips = row.addresses[0].split(' ')[1:] - self.agent.expose_ip(ips, row) + mac = row.addresses[0].strip().split(' ')[0] + ips_info = { + 'mac': mac, + 'cidrs': [ + row.external_ids.get(constants.OVN_CIDRS_EXT_ID_KEY)], + 'type': row.type, + 'logical_switch': row.external_ids.get( + constants.OVN_LS_NAME_EXT_ID_KEY) + } + self.agent.expose_ip(ips, ips_info) class LogicalSwitchPortProviderDeleteEvent(base_watcher.LSPChassisEvent): @@ -124,7 +133,16 @@ class LogicalSwitchPortProviderDeleteEvent(base_watcher.LSPChassisEvent): return with _SYNC_STATE_LOCK.read_lock(): ips = row.addresses[0].split(' ')[1:] - self.agent.withdraw_ip(ips, row) + mac = row.addresses[0].strip().split(' ')[0] + ips_info = { + 'mac': mac, + 'cidrs': [ + row.external_ids.get(constants.OVN_CIDRS_EXT_ID_KEY)], + 'type': row.type, + 'logical_switch': row.external_ids.get( + constants.OVN_LS_NAME_EXT_ID_KEY) + } + self.agent.withdraw_ip(ips, ips_info) class LogicalSwitchPortFIPCreateEvent(base_watcher.LSPChassisEvent): @@ -287,3 +305,79 @@ class LocalnetCreateDeleteEvent(base_watcher.LSPChassisEvent): def _run(self, event, row, old): with _SYNC_STATE_LOCK.read_lock(): self.agent.sync() + + +class ChassisRedirectCreateEvent(base_watcher.LRPChassisEvent): + def __init__(self, bgp_agent): + events = (self.ROW_UPDATE,) + super(ChassisRedirectCreateEvent, self).__init__( + bgp_agent, events) + + def match_fn(self, event, row, old): + try: + # check if hosting-chassis is being added + hosting_chassis = row.status.get(constants.OVN_STATUS_CHASSIS) + if hosting_chassis != self.agent.chassis_id: + # No chassis set or different one + return False + + if hasattr(old, 'status'): + # status has changed + old_hosting_chassis = old.status.get( + constants.OVN_STATUS_CHASSIS) + if old_hosting_chassis != hosting_chassis: + return True + return False + except (IndexError, AttributeError): + return False + + def _run(self, event, row, old): + with _SYNC_STATE_LOCK.read_lock(): + if row.networks: + ips_info = { + 'mac': row.mac, + 'cidrs': row.networks, + 'type': constants.OVN_CR_LRP_PORT_TYPE, + 'logical_switch': row.external_ids.get( + constants.OVN_LS_NAME_EXT_ID_KEY) + } + ips = [net.split("/")[0] for net in row.networks] + self.agent.expose_ip(ips, ips_info) + + +class ChassisRedirectDeleteEvent(base_watcher.LRPChassisEvent): + def __init__(self, bgp_agent): + events = (self.ROW_UPDATE, self.ROW_DELETE,) + super(ChassisRedirectDeleteEvent, self).__init__( + bgp_agent, events) + + def match_fn(self, event, row, old): + try: + if event == self.ROW_DELETE: + return (row.status.get(constants.OVN_STATUS_CHASSIS) == + self.agent.chassis_id) + # ROW UPDATE EVENT + if hasattr(old, 'status'): + # status has changed + hosting_chassis = row.status.get(constants.OVN_STATUS_CHASSIS) + old_hosting_chassis = old.status.get( + constants.OVN_STATUS_CHASSIS) + if (hosting_chassis != old_hosting_chassis and + old_hosting_chassis == self.agent.chassis_id): + return True + return False + except (IndexError, AttributeError): + return False + + def _run(self, event, row, old): + with _SYNC_STATE_LOCK.read_lock(): + if row.networks: + ips_info = { + 'mac': row.mac, + 'cidrs': row.networks, + 'type': constants.OVN_CR_LRP_PORT_TYPE, + 'logical_switch': row.external_ids.get( + constants.OVN_LS_NAME_EXT_ID_KEY) + } + ips = [net.split("/")[0] for net in row.networks] + self.agent.withdraw_ip(ips, ips_info) 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 3ea01d0f..e243d7c1 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 @@ -41,6 +41,7 @@ class TestNBOVNBGPDriver(test_base.TestCase): self.nb_bgp_driver.nb_idl = mock.Mock() self.nb_idl = self.nb_bgp_driver.nb_idl self.nb_bgp_driver.chassis = 'fake-chassis' + self.nb_bgp_driver.chassis_id = 'fake-chassis-id' self.nb_bgp_driver.ovn_bridge_mappings = {'fake-network': self.bridge} self.mock_nbdb = mock.patch.object(ovn, 'OvnNbIdl').start() @@ -71,6 +72,7 @@ class TestNBOVNBGPDriver(test_base.TestCase): CONF.set_override('clear_vrf_routes_on_startup', True) self.addCleanup(CONF.clear_override, 'clear_vrf_routes_on_startup') self.mock_ovs_idl.get_own_chassis_name.return_value = 'chassis-name' + self.mock_ovs_idl.get_own_chassis_id.return_value = 'chassis-id' self.mock_ovs_idl.get_ovn_remote.return_value = ( self.conf_ovsdb_connection) @@ -80,6 +82,8 @@ class TestNBOVNBGPDriver(test_base.TestCase): self.mock_ovs_idl().start.assert_called_once_with( CONF.ovsdb_connection) self.mock_ovs_idl().get_own_chassis_name.assert_called_once() + self.mock_ovs_idl().get_own_chassis_id.assert_called_once() + self.mock_ovs_idl().get_ovn_remote.assert_called_once() mock_ensure_vrf.assert_called_once_with( @@ -138,16 +142,21 @@ class TestNBOVNBGPDriver(test_base.TestCase): ips = [self.ipv4, self.ipv6] mock_exposed_ips.return_value = ips + crlrp_port = fakes.create_object({ + 'name': 'crlrp_port'}) port0 = fakes.create_object({ 'name': 'port-0', 'type': constants.OVN_VM_VIF_PORT_TYPE}) port1 = fakes.create_object({ 'name': 'port-1', 'type': constants.OVN_CHASSISREDIRECT_VIF_PORT_TYPE}) - self.nb_idl.get_active_ports_on_chassis.return_value = [ + self.nb_idl.get_active_cr_lrp_on_chassis.return_value = [crlrp_port] + self.nb_idl.get_active_lsp_on_chassis.return_value = [ port0, port1] - mock_ensure_port_exposed = mock.patch.object( - self.nb_bgp_driver, '_ensure_port_exposed').start() + mock_ensure_crlrp_exposed = mock.patch.object( + self.nb_bgp_driver, '_ensure_crlrp_exposed').start() + mock_ensure_lsp_exposed = mock.patch.object( + self.nb_bgp_driver, '_ensure_lsp_exposed').start() mock_routing_bridge.return_value = ['fake-route'] mock_nic_address.return_value = self.mac mock_get_patch_ports.return_value = [1, 2] @@ -177,7 +186,8 @@ class TestNBOVNBGPDriver(test_base.TestCase): mock.call(mock.ANY, 'bridge1', constants.OVS_RULE_COOKIE)] mock_remove_flows.assert_has_calls(expected_calls) mock_get_ip_rules.assert_called_once() - mock_ensure_port_exposed.assert_called_once_with(port0) + mock_ensure_crlrp_exposed.assert_called_once_with(crlrp_port) + mock_ensure_lsp_exposed.assert_called_once_with(port0) mock_del_exposed_ips.assert_called_once_with( ips, CONF.bgp_nic) mock_del_ip_rules.assert_called_once_with(fake_ip_rules) @@ -186,7 +196,7 @@ class TestNBOVNBGPDriver(test_base.TestCase): bridge = set(self.nb_bgp_driver.ovn_bridge_mappings.values()).pop() mock_delete_vlan_dev.assert_called_once_with(bridge, 12) - def test__ensure_port_exposed_fip(self): + def test__ensure_lsp_exposed_fip(self): port0 = fakes.create_object({ 'name': 'port-0', 'external_ids': {constants.OVN_FIP_EXT_ID_KEY: "fip"}}) @@ -201,14 +211,14 @@ class TestNBOVNBGPDriver(test_base.TestCase): mock_expose_ip = mock.patch.object( self.nb_bgp_driver, '_expose_ip').start() - self.nb_bgp_driver._ensure_port_exposed(port0) + self.nb_bgp_driver._ensure_lsp_exposed(port0) mock_get_port_external_ip_and_ls.assert_called_once_with(port0.name) mock_expose_fip.assert_called_once_with("192.168.0.10", "fake-mac", "test-ls", port0) mock_expose_ip.assert_not_called() - def test__ensure_port_exposed_tenant_ls(self): + def test__ensure_lsp_exposed_tenant_ls(self): port0 = fakes.create_object({ 'name': 'port-0', 'external_ids': {constants.OVN_LS_NAME_EXT_ID_KEY: "test-ls"}}) @@ -221,14 +231,14 @@ class TestNBOVNBGPDriver(test_base.TestCase): mock_expose_ip = mock.patch.object( self.nb_bgp_driver, '_expose_ip').start() - self.nb_bgp_driver._ensure_port_exposed(port0) + self.nb_bgp_driver._ensure_lsp_exposed(port0) mock_get_port_external_ip_and_ls.assert_not_called() mock_expose_fip.assert_not_called() mock_expose_ip.assert_not_called() @mock.patch.object(linux_net, 'get_ip_version') - def test__ensure_port_exposed_no_fip_no_tenant_ls(self, mock_ip_version): + def test__ensure_lsp_exposed_no_fip_no_tenant_ls(self, mock_ip_version): port0 = fakes.create_object({ 'name': 'port-0', 'addresses': ["fake_mac 192.168.0.10"], @@ -250,14 +260,79 @@ class TestNBOVNBGPDriver(test_base.TestCase): mock_get_ls_localnet_info.return_value = ('fake-localnet', 'br-ex', 10) mock_ip_version.return_value = constants.IP_VERSION_4 - self.nb_bgp_driver._ensure_port_exposed(port0) + self.nb_bgp_driver._ensure_lsp_exposed(port0) mock_get_port_external_ip_and_ls.assert_not_called() mock_get_ls_localnet_info.assert_called_once_with('test-ls') mock_expose_fip.assert_not_called() mock_expose_ip.assert_called_once_with( ['192.168.0.10'], 'fake_mac', 'test-ls', 'br-ex', 10, - constants.OVN_VM_VIF_PORT_TYPE, None) + constants.OVN_VM_VIF_PORT_TYPE, [None]) + + def test__ensure_crlrp_exposed(self): + port = fakes.create_object({ + 'name': 'lrp-port', + 'networks': ['172.24.16.2/24'], + 'mac': "fake_mac", + 'status': {'hosting-chassis': self.nb_bgp_driver.chassis_id}, + 'external_ids': {constants.OVN_LS_NAME_EXT_ID_KEY: "test-ls"}}) + mock_get_ls_localnet_info = mock.patch.object( + self.nb_bgp_driver, '_get_ls_localnet_info').start() + mock_get_ls_localnet_info.return_value = ('fake-localnet', 'br-ex', 10) + mock_expose_ip = mock.patch.object( + self.nb_bgp_driver, '_expose_ip').start() + + self.nb_bgp_driver._ensure_crlrp_exposed(port) + + mock_expose_ip.assert_called_once_with( + ['172.24.16.2'], 'fake_mac', 'test-ls', 'br-ex', 10, + constants.OVN_CR_LRP_PORT_TYPE, ['172.24.16.2/24']) + + def test__ensure_crlrp_exposed_no_networks(self): + port = fakes.create_object({ + 'name': 'lrp-port', + 'networks': [], + 'mac': "fake_mac", + 'status': {'hosting-chassis': self.nb_bgp_driver.chassis_id}, + 'external_ids': {constants.OVN_LS_NAME_EXT_ID_KEY: "test-ls"}}) + mock_expose_ip = mock.patch.object( + self.nb_bgp_driver, '_expose_ip').start() + + self.nb_bgp_driver._ensure_crlrp_exposed(port) + + mock_expose_ip.assert_not_called() + + def test__ensure_crlrp_exposed_no_logical_switch(self): + port = fakes.create_object({ + 'name': 'lrp-port', + 'networks': ['172.24.16.2/24'], + 'mac': "fake_mac", + 'status': {'hosting-chassis': self.nb_bgp_driver.chassis_id}, + 'external_ids': {}}) + + mock_expose_ip = mock.patch.object( + self.nb_bgp_driver, '_expose_ip').start() + + self.nb_bgp_driver._ensure_crlrp_exposed(port) + + mock_expose_ip.assert_not_called() + + def test__ensure_crlrp_exposed_no_bridge(self): + port = fakes.create_object({ + 'name': 'lrp-port', + 'networks': ['172.24.16.2/24'], + 'mac': "fake_mac", + 'status': {'hosting-chassis': self.nb_bgp_driver.chassis_id}, + 'external_ids': {constants.OVN_LS_NAME_EXT_ID_KEY: "test-ls"}}) + mock_get_ls_localnet_info = mock.patch.object( + self.nb_bgp_driver, '_get_ls_localnet_info').start() + mock_get_ls_localnet_info.return_value = (None, None, None) + mock_expose_ip = mock.patch.object( + self.nb_bgp_driver, '_expose_ip').start() + + self.nb_bgp_driver._ensure_crlrp_exposed(port) + + mock_expose_ip.assert_not_called() @mock.patch.object(wire_utils, 'wire_provider_port') @mock.patch.object(bgp_utils, 'announce_ips') @@ -337,7 +412,7 @@ class TestNBOVNBGPDriver(test_base.TestCase): self.assertEqual(bridge_device, None) self.assertEqual(bridge_vlan, None) - def _test_expose_ip(self, ips, row): + def _test_expose_ip(self, ips, ips_info): mock_expose_provider_port = mock.patch.object( self.nb_bgp_driver, '_expose_provider_port').start() mock_get_ls_localnet_info = mock.patch.object( @@ -345,58 +420,63 @@ class TestNBOVNBGPDriver(test_base.TestCase): mock_get_ls_localnet_info.return_value = ('fake-localnet', 'br-ex', 10) self.nb_bgp_driver.ovn_bridge_mappings = {'fake-localnet': 'br-ex'} - cidr = row.external_ids.get(constants.OVN_CIDRS_EXT_ID_KEY) - logical_switch = row.external_ids.get(constants.OVN_LS_NAME_EXT_ID_KEY) + self.nb_bgp_driver.expose_ip(ips, ips_info) - self.nb_bgp_driver.expose_ip(ips, row) - - if not logical_switch: + if not ips_info['logical_switch']: mock_expose_provider_port.assert_not_called() mock_get_ls_localnet_info.assert_not_called() return - mock_get_ls_localnet_info.assert_called_once_with(logical_switch) - self.assertEqual(self.nb_bgp_driver.ovn_provider_ls[logical_switch], - {'bridge_device': 'br-ex', 'bridge_vlan': 10, - 'localnet': 'fake-localnet'}) - if row.type == constants.OVN_VIRTUAL_VIF_PORT_TYPE and cidr: + mock_get_ls_localnet_info.assert_called_once_with( + ips_info['logical_switch']) + self.assertEqual( + self.nb_bgp_driver.ovn_provider_ls[ips_info['logical_switch']], + {'bridge_device': 'br-ex', 'bridge_vlan': 10, + 'localnet': 'fake-localnet'}) + if (ips_info['type'] == constants.OVN_VIRTUAL_VIF_PORT_TYPE and + ips_info['cidrs']): mock_expose_provider_port.assert_called_once_with( ips, 'fake-mac', 'test-ls', 'br-ex', 10, 'fake-localnet', - [cidr]) + ips_info['cidrs']) else: mock_expose_provider_port.assert_called_once_with( ips, 'fake-mac', 'test-ls', 'br-ex', 10, 'fake-localnet') def test_expose_ip(self): ips = [self.ipv4, self.ipv6] - row = fakes.create_object({ - 'addresses': ['fake-mac {} {}'.format(self.ipv4, self.ipv6)], + ips_info = { + 'mac': 'fake-mac', + 'cidrs': [], 'type': constants.OVN_VM_VIF_PORT_TYPE, - 'external_ids': {constants.OVN_LS_NAME_EXT_ID_KEY: 'test-ls'}}) + 'logical_switch': 'test-ls' + } - self._test_expose_ip(ips, row) + self._test_expose_ip(ips, ips_info) def test_expose_ip_virtual(self): ips = [self.ipv4, self.ipv6] - row = fakes.create_object({ - 'addresses': ['fake-mac {} {}'.format(self.ipv4, self.ipv6)], + ips_info = { + 'mac': 'fake-mac', + 'cidrs': ['test-cidr'], 'type': constants.OVN_VIRTUAL_VIF_PORT_TYPE, - 'external_ids': {constants.OVN_LS_NAME_EXT_ID_KEY: 'test-ls', - constants.OVN_CIDRS_EXT_ID_KEY: 'test-cidr'}}) + 'logical_switch': 'test-ls' + } - self._test_expose_ip(ips, row) + self._test_expose_ip(ips, ips_info) def test_expose_ip_no_switch(self): ips = [self.ipv4, self.ipv6] - row = fakes.create_object({ - 'addresses': ['fake-mac {} {}'.format(self.ipv4, self.ipv6)], + ips_info = { + 'mac': 'fake-mac', + 'cidrs': [], 'type': constants.OVN_VM_VIF_PORT_TYPE, - 'external_ids': {}}) + 'logical_switch': None + } - self._test_expose_ip(ips, row) + self._test_expose_ip(ips, ips_info) @mock.patch.object(linux_net, 'get_ip_version') - def _test_withdraw_ip(self, ips, row, provider, mock_ip_version): + def _test_withdraw_ip(self, ips, ips_info, provider, mock_ip_version): mock_withdraw_provider_port = mock.patch.object( self.nb_bgp_driver, '_withdraw_provider_port').start() mock_get_ls_localnet_info = mock.patch.object( @@ -409,60 +489,71 @@ class TestNBOVNBGPDriver(test_base.TestCase): else: mock_get_ls_localnet_info.return_value = (None, None, None) - cidr = row.external_ids.get(constants.OVN_CIDRS_EXT_ID_KEY) - logical_switch = row.external_ids.get(constants.OVN_LS_NAME_EXT_ID_KEY) + self.nb_bgp_driver.withdraw_ip(ips, ips_info) - self.nb_bgp_driver.withdraw_ip(ips, row) - - if not logical_switch: + if not ips_info['logical_switch']: mock_get_ls_localnet_info.assert_not_called() mock_withdraw_provider_port.assert_not_called() return if not provider: - mock_get_ls_localnet_info.assert_called_once_with(logical_switch) + mock_get_ls_localnet_info.assert_called_once_with( + ips_info['logical_switch']) mock_withdraw_provider_port.assert_not_called() return - mock_get_ls_localnet_info.assert_called_once_with(logical_switch) - if row.type == constants.OVN_VIRTUAL_VIF_PORT_TYPE and cidr: + mock_get_ls_localnet_info.assert_called_once_with( + ips_info['logical_switch']) + if (ips_info['type'] == constants.OVN_VIRTUAL_VIF_PORT_TYPE and + ips_info['cidrs']): mock_withdraw_provider_port.assert_called_once_with( - ips, 'test-ls', 'br-ex', 10, [cidr]) + ips, 'test-ls', 'br-ex', 10, ips_info['cidrs']) else: mock_withdraw_provider_port.assert_called_once_with( ips, 'test-ls', 'br-ex', 10) def test_withdraw_ip(self): ips = [self.ipv4, self.ipv6] - row = fakes.create_object({ + ips_info = { + 'mac': 'fake-mac', + 'cidrs': [], 'type': constants.OVN_VM_VIF_PORT_TYPE, - 'external_ids': {constants.OVN_LS_NAME_EXT_ID_KEY: 'test-ls'}}) + 'logical_switch': 'test-ls' + } - self._test_withdraw_ip(ips, row, True) + self._test_withdraw_ip(ips, ips_info, True) def test_withdraw_ip_no_provider(self): ips = [self.ipv4, self.ipv6] - row = fakes.create_object({ + ips_info = { + 'mac': 'fake-mac', + 'cidrs': [], 'type': constants.OVN_VM_VIF_PORT_TYPE, - 'external_ids': {constants.OVN_LS_NAME_EXT_ID_KEY: 'test-ls'}}) + 'logical_switch': 'test-ls' + } - self._test_withdraw_ip(ips, row, False) + self._test_withdraw_ip(ips, ips_info, False) def test_withdraw_ip_virtual(self): ips = [self.ipv4, self.ipv6] - row = fakes.create_object({ + ips_info = { + 'mac': 'fake-mac', + 'cidrs': ['test-cidr'], 'type': constants.OVN_VIRTUAL_VIF_PORT_TYPE, - 'external_ids': {constants.OVN_LS_NAME_EXT_ID_KEY: 'test-ls', - constants.OVN_CIDRS_EXT_ID_KEY: 'test-cidr'}}) + 'logical_switch': 'test-ls' + } - self._test_withdraw_ip(ips, row, True) + self._test_withdraw_ip(ips, ips_info, True) def test_withdraw_ip_no_switch(self): ips = [self.ipv4, self.ipv6] - row = fakes.create_object({ + ips_info = { + 'mac': 'fake-mac', + 'cidrs': [], 'type': constants.OVN_VM_VIF_PORT_TYPE, - 'external_ids': {}}) + 'logical_switch': None + } - self._test_withdraw_ip(ips, row, True) + self._test_withdraw_ip(ips, ips_info, True) def test__get_ls_localnet_info(self): logical_switch = 'lswitch1' diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovn.py b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovn.py index fa982c6e..7458cf6f 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovn.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/utils/test_ovn.py @@ -37,6 +37,7 @@ class TestOvsdbNbOvnIdl(test_base.TestCase): # Monkey-patch parent class methods self.nb_idl.db_find_rows = mock.Mock() + self.nb_idl.db_list_rows = mock.Mock() self.nb_idl.lookup = mock.Mock() def test_get_network_vlan_tags(self): @@ -99,7 +100,7 @@ class TestOvsdbNbOvnIdl(test_base.TestCase): 'NAT', ('logical_port', '=', logical_port)) - def test_get_active_ports_on_chassis_options(self): + def test_get_active_lsp_on_chassis_options(self): chassis = 'local_chassis' row1 = fakes.create_object({ 'options': {'requested-chassis': chassis}, @@ -109,14 +110,14 @@ class TestOvsdbNbOvnIdl(test_base.TestCase): 'external_ids': {}}) self.nb_idl.db_find_rows.return_value.execute.return_value = [ row1, row2] - ret = self.nb_idl.get_active_ports_on_chassis(chassis) + ret = self.nb_idl.get_active_lsp_on_chassis(chassis) self.assertEqual([row1], ret) self.nb_idl.db_find_rows.assert_called_once_with( 'Logical_Switch_Port', ('up', '=', True)) - def test_get_active_ports_on_chassis_external_ids(self): + def test_get_active_lsp_on_chassis_external_ids(self): chassis = 'local_chassis' row1 = fakes.create_object({ 'options': {}, @@ -126,13 +127,29 @@ class TestOvsdbNbOvnIdl(test_base.TestCase): 'external_ids': {'neutron:host_id': 'other_chassis'}}) self.nb_idl.db_find_rows.return_value.execute.return_value = [ row1, row2] - ret = self.nb_idl.get_active_ports_on_chassis(chassis) + ret = self.nb_idl.get_active_lsp_on_chassis(chassis) self.assertEqual([row1], ret) self.nb_idl.db_find_rows.assert_called_once_with( 'Logical_Switch_Port', ('up', '=', True)) + def test_get_active_cr_lrp_on_chassis(self): + chassis = 'local_chassis' + row1 = fakes.create_object({ + 'status': {'hosting-chassis': 'local_chassis'}}) + row2 = fakes.create_object({ + 'status': {'hosting-chassis': 'other_chassis'}}) + row3 = fakes.create_object({}) + self.nb_idl.db_list_rows.return_value.execute.return_value = [ + row1, row2, row3] + + ret = self.nb_idl.get_active_cr_lrp_on_chassis(chassis) + + self.assertEqual([row1], ret) + self.nb_idl.db_list_rows.assert_called_once_with( + 'Logical_Router_Port') + class TestOvsdbSbOvnIdl(test_base.TestCase): diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_base_watcher.py b/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_base_watcher.py index f3dcc1b7..4e0a52de 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_base_watcher.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_base_watcher.py @@ -40,3 +40,26 @@ class TestPortBindingChassisEvent(test_base.TestCase): 'aa:bb:cc:dd:ee:ff')) self.assertTrue(self.pb_event._check_ip_associated( 'aa:bb:cc:dd:ee:ff 10.10.1.16 10.10.1.17 10.10.1.18')) + + +class FakeLSPChassisEvent(base_watcher.LSPChassisEvent): + def run(self): + pass + + +class TestLSPChassisEvent(test_base.TestCase): + + def setUp(self): + super(TestLSPChassisEvent, self).setUp() + self.lsp_event = FakeLSPChassisEvent( + mock.Mock(), [mock.Mock()]) + + def test__check_ip_associated(self): + self.assertTrue(self.lsp_event._check_ip_associated( + 'aa:bb:cc:dd:ee:ff 10.10.1.16')) + self.assertTrue(self.lsp_event._check_ip_associated( + 'aa:bb:cc:dd:ee:ff 10.10.1.16 10.10.1.17')) + self.assertFalse(self.lsp_event._check_ip_associated( + 'aa:bb:cc:dd:ee:ff')) + self.assertTrue(self.lsp_event._check_ip_associated( + 'aa:bb:cc:dd:ee:ff 10.10.1.16 10.10.1.17 10.10.1.18')) diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_nb_bgp_watcher.py b/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_nb_bgp_watcher.py index 86cab390..b1c61965 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_nb_bgp_watcher.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_nb_bgp_watcher.py @@ -86,12 +86,21 @@ class TestLogicalSwitchPortProviderCreateEvent(test_base.TestCase): self.assertFalse(self.event.match_fn(mock.Mock(), row, old)) def test_run(self): - row = utils.create_row(type=constants.OVN_VM_VIF_PORT_TYPE, - addresses=['mac 192.168.0.1'], - options={'requested-chassis': self.chassis}, - up=[True]) + row = utils.create_row( + type=constants.OVN_VM_VIF_PORT_TYPE, + external_ids={'neutron:host_id': self.chassis, + constants.OVN_LS_NAME_EXT_ID_KEY: 'test-ls'}, + addresses=['mac 192.168.0.1'], + options={'requested-chassis': self.chassis}, + up=[True]) + ips_info = { + 'mac': 'mac', + 'cidrs': [None], + 'type': constants.OVN_VM_VIF_PORT_TYPE, + 'logical_switch': 'test-ls' + } self.event.run(mock.Mock(), row, mock.Mock()) - self.agent.expose_ip.assert_called_once_with(['192.168.0.1'], row) + self.agent.expose_ip.assert_called_once_with(['192.168.0.1'], ips_info) def test_run_wrong_type(self): row = utils.create_row( @@ -170,12 +179,23 @@ class TestLogicalSwitchPortProviderDeleteEvent(test_base.TestCase): self.assertFalse(self.event.match_fn(mock.Mock(), row, old)) def test_run(self): - row = utils.create_row(type=constants.OVN_VM_VIF_PORT_TYPE, - addresses=['mac 192.168.0.1'], - options={'requested-chassis': self.chassis}, - up=[True]) + row = utils.create_row( + type=constants.OVN_VM_VIF_PORT_TYPE, + external_ids={'neutron:host_id': self.chassis, + constants.OVN_LS_NAME_EXT_ID_KEY: 'test-ls'}, + addresses=['mac 192.168.0.1'], + options={'requested-chassis': self.chassis}, + up=[True]) + ips_info = { + 'mac': 'mac', + 'cidrs': [None], + 'type': constants.OVN_VM_VIF_PORT_TYPE, + 'logical_switch': 'test-ls' + } + self.event.run(mock.Mock(), row, mock.Mock()) - self.agent.withdraw_ip.assert_called_once_with(['192.168.0.1'], row) + self.agent.withdraw_ip.assert_called_once_with(['192.168.0.1'], + ips_info) def test_run_wrong_type(self): row = utils.create_row( @@ -482,3 +502,136 @@ class TestLocalnetCreateDeleteEvent(test_base.TestCase): row = utils.create_row(type=constants.OVN_LOCALNET_VIF_PORT_TYPE) self.event.run(None, row, None) self.agent.sync.assert_called_once() + + +class TestChassisRedirectCreateEvent(test_base.TestCase): + def setUp(self): + super(TestChassisRedirectCreateEvent, self).setUp() + self.chassis = 'fake-chassis' + self.chassis_id = 'fake-chassis-id' + self.agent = mock.Mock(chassis=self.chassis, + chassis_id=self.chassis_id) + self.event = nb_bgp_watcher.ChassisRedirectCreateEvent( + self.agent) + + def test_match_fn(self): + row = utils.create_row(mac='fake-mac', + networks=['192.168.0.2/24'], + status={'hosting-chassis': self.chassis_id}) + old = utils.create_row(mac='fake-mac', + networks=['192.168.0.2/24'], + status={}) + self.assertTrue(self.event.match_fn(mock.Mock(), row, old)) + + def test_match_fn_exception(self): + row = utils.create_row(mac='fake-mac', + networks=['192.168.0.2/24']) + self.assertFalse(self.event.match_fn(mock.Mock(), row, mock.Mock())) + + def test_match_fn_no_status_change(self): + row = utils.create_row(mac='fake-mac', + networks=['192.168.0.2/24'], + status={'hosting-chassis': self.chassis_id}) + old = utils.create_row() + self.assertFalse(self.event.match_fn(mock.Mock(), row, old)) + + def test_match_fn_different_chassis(self): + row = utils.create_row(mac='fake-mac', + networks=['192.168.0.2/24'], + status={'hosting-chassis': 'other_chassis'}) + self.assertFalse(self.event.match_fn(mock.Mock(), row, mock.Mock())) + + def test_run(self): + row = utils.create_row( + mac='fake-mac', + networks=['192.168.0.2/24'], + status={'hosting-chassis': self.chassis_id}, + external_ids={constants.OVN_LS_NAME_EXT_ID_KEY: 'test-ls'}) + ips_info = {'mac': 'fake-mac', + 'cidrs': ['192.168.0.2/24'], + 'type': constants.OVN_CR_LRP_PORT_TYPE, + 'logical_switch': 'test-ls'} + self.event.run(None, row, None) + self.agent.expose_ip.assert_called_once_with(['192.168.0.2'], ips_info) + + def test_run_no_networks(self): + row = utils.create_row( + mac='fake-mac', + networks=[], + status={'hosting-chassis': self.chassis_id}, + external_ids={constants.OVN_LS_NAME_EXT_ID_KEY: 'test-ls'}) + self.event.run(None, row, None) + self.agent.expose_ip.assert_not_called() + + +class TestChassisRedirectDeleteEvent(test_base.TestCase): + def setUp(self): + super(TestChassisRedirectDeleteEvent, self).setUp() + self.chassis = 'fake-chassis' + self.chassis_id = 'fake-chassis-id' + self.agent = mock.Mock(chassis=self.chassis, + chassis_id=self.chassis_id) + self.event = nb_bgp_watcher.ChassisRedirectDeleteEvent( + self.agent) + + def test_match_fn(self): + row = utils.create_row(mac='fake-mac', + networks=['192.168.0.2/24'], + status={}) + old = utils.create_row(mac='fake-mac', + networks=['192.168.0.2/24'], + status={'hosting-chassis': self.chassis_id}) + self.assertTrue(self.event.match_fn(mock.Mock(), row, old)) + + def test_match_fn_delete(self): + event = self.event.ROW_DELETE + row = utils.create_row(mac='fake-mac', + networks=['192.168.0.2/24'], + status={'hosting-chassis': self.chassis_id}) + self.assertTrue(self.event.match_fn(event, row, mock.Mock())) + + def test_match_fn_exception(self): + row = utils.create_row(mac='fake-mac', + networks=['192.168.0.2/24']) + self.assertFalse(self.event.match_fn(mock.Mock(), row, mock.Mock())) + + def test_match_fn_no_status_change(self): + row = utils.create_row(mac='fake-mac', + networks=['192.168.0.2/24'], + status={'hosting-chassis': self.chassis_id}) + old = utils.create_row(mac='fake-mac', + networks=['192.168.0.2/24'], + status={'hosting-chassis': self.chassis_id}) + self.assertFalse(self.event.match_fn(mock.Mock(), row, old)) + + def test_match_fn_different_chassis(self): + row = utils.create_row(mac='fake-mac', + networks=['192.168.0.2/24'], + status={'hosting-chassis': self.chassis_id}) + old = utils.create_row(mac='fake-mac', + networks=['192.168.0.2/24'], + status={'hosting-chassis': 'different_chassis'}) + self.assertFalse(self.event.match_fn(mock.Mock(), row, old)) + + def test_run(self): + row = utils.create_row( + mac='fake-mac', + networks=['192.168.0.2/24'], + status={'hosting-chassis': self.chassis_id}, + external_ids={constants.OVN_LS_NAME_EXT_ID_KEY: 'test-ls'}) + ips_info = {'mac': 'fake-mac', + 'cidrs': ['192.168.0.2/24'], + 'type': constants.OVN_CR_LRP_PORT_TYPE, + 'logical_switch': 'test-ls'} + self.event.run(None, row, None) + self.agent.withdraw_ip.assert_called_once_with(['192.168.0.2'], + ips_info) + + def test_run_no_networks(self): + row = utils.create_row( + mac='fake-mac', + networks=[], + status={'hosting-chassis': self.chassis_id}, + external_ids={constants.OVN_LS_NAME_EXT_ID_KEY: 'test-ls'}) + self.event.run(None, row, None) + self.agent.withdraw_ip.assert_not_called()