diff --git a/ovn_bgp_agent/constants.py b/ovn_bgp_agent/constants.py index 0cbb1401..019b30e4 100644 --- a/ovn_bgp_agent/constants.py +++ b/ovn_bgp_agent/constants.py @@ -53,3 +53,6 @@ LINK_DOWN = "down" SUBNET_POOL_ADDR_SCOPE4 = "neutron:subnet_pool_addr_scope4" SUBNET_POOL_ADDR_SCOPE6 = "neutron:subnet_pool_addr_scope6" + +EXPOSE = "expose" +WITHDRAW = "withdraw" diff --git a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py index 425f0724..e5e64e38 100644 --- a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py +++ b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py @@ -37,7 +37,7 @@ LOG = logging.getLogger(__name__) # LOG.setLevel(logging.DEBUG) # logging.basicConfig(level=logging.DEBUG) -OVN_TABLES = ["Port_Binding", "Chassis", "Datapath_Binding", "Load_Balancer"] +OVN_TABLES = ["Port_Binding", "Chassis", "Datapath_Binding"] class OVNBGPDriver(driver_api.AgentDriverBase): @@ -134,14 +134,13 @@ class OVNBGPDriver(driver_api.AgentDriverBase): "PortBindingChassisDeletedEvent", "FIPSetEvent", "FIPUnsetEvent", - "OVNLBMemberUpdateEvent", + "OVNLBVIPPortEvent", "ChassisCreateEvent"]) if self._expose_tenant_networks: events.update(["SubnetRouterAttachedEvent", "SubnetRouterDetachedEvent", "TenantPortCreatedEvent", - "TenantPortDeletedEvent", - "OVNLBTenantPortEvent"]) + "TenantPortDeletedEvent"]) return events @lockutils.synchronized('bgp') @@ -229,11 +228,12 @@ class OVNBGPDriver(driver_api.AgentDriverBase): # add missing routes/ips related to ovn-octavia loadbalancers # on the provider networks - ovn_lbs = self.sb_idl.get_ovn_lb_on_provider_datapath( + ovn_lb_vips = self.sb_idl.get_ovn_lb_vips_on_provider_datapath( cr_lrp_info['provider_datapath']) - for ovn_lb in ovn_lbs: - self._process_ovn_lb(ovn_lb, cr_lrp_port, exposed_ips, - ovn_ip_rules) + for ovn_lb_port, ovn_lb_ip in ovn_lb_vips.items(): + self._expose_ovn_lb_on_provider(ovn_lb_ip, + ovn_lb_port, + cr_lrp_port) # remove extra routes/ips # remove all the leftovers on the list of current ips on dev OVN @@ -407,12 +407,39 @@ class OVNBGPDriver(driver_api.AgentDriverBase): return None, None @lockutils.synchronized('bgp') - def expose_ovn_lb_on_provider(self, ovn_lb, ip, cr_lrp): - self._expose_ovn_lb_on_provider(ovn_lb, ip, cr_lrp) + def expose_ovn_lb(self, ip, row): + self._process_ovn_lb(ip, row, constants.EXPOSE) - 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) + @lockutils.synchronized('bgp') + def withdraw_ovn_lb(self, ip, row): + self._process_ovn_lb(ip, row, constants.WITHDRAW) + + def _process_ovn_lb(self, ip, row, action): + if not self.sb_idl.is_provider_network(row.datapath): + if not self._expose_tenant_networks: + return + if action == constants.EXPOSE: + return self._expose_remote_ip([ip], row) + if action == constants.WITHDRAW: + return self._withdraw_remote_ip([ip], row) + # if unknown action return + return + local_lb_cr_lrp = None + for cr_lrp_port, cr_lrp_info in self.ovn_local_cr_lrps.items(): + if cr_lrp_info['provider_datapath'] == row.datapath: + local_lb_cr_lrp = cr_lrp_port + break + if not local_lb_cr_lrp: + return + vip_port = row.logical_port + if action == constants.EXPOSE: + self._expose_ovn_lb_on_provider(ip, vip_port, local_lb_cr_lrp) + if action == constants.WITHDRAW: + self._withdraw_ovn_lb_on_provider(vip_port, local_lb_cr_lrp) + + def _expose_ovn_lb_on_provider(self, ip, ovn_lb_vip_port, cr_lrp): + 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'] @@ -421,21 +448,21 @@ class OVNBGPDriver(driver_api.AgentDriverBase): 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): + 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'] - for ip in self.ovn_lb_vips[ovn_lb].copy(): + 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 ip in self.ovn_lb_vips[ovn_lb]: - self.ovn_lb_vips[ovn_lb].remove(ip) + 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 in self.ovn_local_cr_lrps[cr_lrp]['ovn_lbs']: - self.ovn_local_cr_lrps[cr_lrp]['ovn_lbs'].remove(ovn_lb) + 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) @lockutils.synchronized('bgp') def expose_ip(self, ips, row, associated_port=None): @@ -521,7 +548,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase): 'mac': mac, 'subnets_datapath': {}, 'subnets_cidr': [], - 'ovn_lbs': [], + 'ovn_lb_vips': [], 'bridge_vlan': bridge_vlan, 'bridge_device': bridge_device } @@ -620,6 +647,9 @@ class OVNBGPDriver(driver_api.AgentDriverBase): @lockutils.synchronized('bgp') def expose_remote_ip(self, ips, row): + self._expose_remote_ip(ips, row) + + def _expose_remote_ip(self, ips, row): if (self.sb_idl.is_provider_network(row.datapath) or not self._expose_tenant_networks): return @@ -650,6 +680,9 @@ class OVNBGPDriver(driver_api.AgentDriverBase): @lockutils.synchronized('bgp') def withdraw_remote_ip(self, ips, row, chassis=None): + self._withdraw_remote_ip(ips, row, chassis) + + def _withdraw_remote_ip(self, ips, row, chassis=None): if (self.sb_idl.is_provider_network(row.datapath) or not self._expose_tenant_networks): return @@ -690,7 +723,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase): 'mac': mac, 'subnets_datapath': {}, 'subnets_cidr': [], - 'ovn_lbs': [], + 'ovn_lb_vips': [], 'bridge_vlan': bridge_vlan, 'bridge_device': bridge_device } @@ -725,32 +758,6 @@ class OVNBGPDriver(driver_api.AgentDriverBase): exposed_ips=exposed_ips, ovn_ip_rules=ovn_ip_rules) - def _process_ovn_lb(self, ovn_lb, cr_lrp_port, exposed_ips=[], - ovn_ip_rules={}): - if hasattr(ovn_lb, 'datapath_group'): - ovn_lb_datapaths = ovn_lb.datapath_group[0].datapaths - else: - ovn_lb_datapaths = ovn_lb.datapaths - for ovn_dp in ovn_lb_datapaths: - if ovn_dp in self.ovn_local_cr_lrps[ - cr_lrp_port]['subnets_datapath'].values(): - break - else: - return - - for vip in ovn_lb.vips.keys(): - ip = driver_utils.parse_vip_from_lb_table(vip) - self._expose_ovn_lb_on_provider(ovn_lb.name, ip, cr_lrp_port) - if exposed_ips and ip in exposed_ips: - exposed_ips.remove(ip) - if ovn_ip_rules: - ip_version = linux_net.get_ip_version(ip) - if ip_version == constants.IP_VERSION_6: - ip_dst = "{}/128".format(ip) - else: - ip_dst = "{}/32".format(ip) - ovn_ip_rules.pop(ip_dst, None) - def _expose_cr_lrp_port(self, ips, mac, bridge_device, bridge_vlan, router_datapath, provider_datapath, cr_lrp_port): LOG.debug("Adding BGP route for CR-LRP Port %s", ips) @@ -770,10 +777,12 @@ class OVNBGPDriver(driver_api.AgentDriverBase): for lrp in lrp_ports: self._process_lrp_port(lrp, cr_lrp_port) - ovn_lbs = self.sb_idl.get_ovn_lb_on_provider_datapath( + ovn_lb_vips = self.sb_idl.get_ovn_lb_vips_on_provider_datapath( provider_datapath) - for ovn_lb in ovn_lbs: - self._process_ovn_lb(ovn_lb, cr_lrp_port) + for ovn_lb_port, ovn_lb_ip in ovn_lb_vips.items(): + self._expose_ovn_lb_on_provider(ovn_lb_ip, + ovn_lb_port, + cr_lrp_port) def _withdraw_cr_lrp_port(self, ips, mac, bridge_device, bridge_vlan, provider_datapath, cr_lrp_port): @@ -805,10 +814,12 @@ class OVNBGPDriver(driver_api.AgentDriverBase): # check if there are loadbalancers associated to the router, # and if so delete the needed routes/rules - ovn_lbs = self.ovn_local_cr_lrps[cr_lrp_port]['ovn_lbs'].copy() - for ovn_lb in ovn_lbs: - self.withdraw_ovn_lb_on_provider(ovn_lb, cr_lrp_port) - self.ovn_local_cr_lrps[cr_lrp_port]['ovn_lbs'].remove(ovn_lb) + ovn_lb_vips = self.ovn_local_cr_lrps[cr_lrp_port][ + 'ovn_lb_vips'].copy() + for ovn_lb_vip in ovn_lb_vips: + self._withdraw_ovn_lb_on_provider(ovn_lb_vip, cr_lrp_port) + self.ovn_local_cr_lrps[cr_lrp_port]['ovn_lb_vips'].remove( + ovn_lb_vip) try: del self.ovn_local_cr_lrps[cr_lrp_port] except KeyError: diff --git a/ovn_bgp_agent/drivers/openstack/utils/driver_utils.py b/ovn_bgp_agent/drivers/openstack/utils/driver_utils.py index 964e19be..226477a5 100644 --- a/ovn_bgp_agent/drivers/openstack/utils/driver_utils.py +++ b/ovn_bgp_agent/drivers/openstack/utils/driver_utils.py @@ -21,19 +21,6 @@ from ovn_bgp_agent.utils import linux_net LOG = logging.getLogger(__name__) -def parse_vip_from_lb_table(vip): - # The VIP format comes from the Load_Balancer table as is like: - # (ipv4): 172.24.100.66:80 - # (ipv6): [2001:db8::f816:3eff:fe55:ef1e]:80 - vip_split = vip.split("]:") - if len(vip_split) == 1: # ipv4 - return vip.split(":")[0] - if len(vip_split) == 2: # ipv6 - return vip_split[0].split("[")[1] - - LOG.error("Malformated VIP at Load Balancer SB table: %s", vip) - - def is_ipv6_gua(ip): if linux_net.get_ip_version(ip) != constants.IP_VERSION_6: return False diff --git a/ovn_bgp_agent/drivers/openstack/utils/ovn.py b/ovn_bgp_agent/drivers/openstack/utils/ovn.py index 5c538712..b56f1351 100644 --- a/ovn_bgp_agent/drivers/openstack/utils/ovn.py +++ b/ovn_bgp_agent/drivers/openstack/utils/ovn.py @@ -303,27 +303,23 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend): datapath, port_type=constants.OVN_VIRTUAL_VIF_PORT_TYPE) return [r for r in rows if r.chassis and r.chassis[0].name == chassis] - def get_ovn_lb_on_provider_datapath(self, datapath): - # TODO(ltomasbo): Once ovsdbapp supports {>=} operator we can query - # it directly with: - # ovn_lbs = self.db_find_rows( - # 'Load_Balancer', - # ('datapaths', '{>=}', [datapath])).execute(check_error=True) - # return [ovn_lb for ovn_lb in ovn_lbs if len(ovn_lb.datapaths) > 1] - ovn_lbs = self.db_list_rows('Load_Balancer').execute( - check_error=True) + def get_ovn_lb_vips_on_provider_datapath(self, datapath): + # return {vip_port: vip_ip, vip_port2: vip_ip2, ...} + # ovn-sbctl find port_binding type=\"\" chassis=[] mac=[] up=false + cmd = self.db_find_rows('Port_Binding', + ('datapath', '=', datapath), + ('type', '=', constants.OVN_VM_VIF_PORT_TYPE), + ('chassis', '=', []), + ('mac', '=', []), + ('up', '=', False)) + lbs = {} + for row in cmd.execute(check_error=True): + # This is depending on the external-id information added by + # neutron, regarding the neutron:cidrs + ext_n_cidr = row.external_ids.get(constants.OVN_CIDRS_EXT_ID_KEY) + if not ext_n_cidr: + continue + ovn_lb_ip = ext_n_cidr.split(" ")[0].split("/")[0] + lbs[row.logical_port] = ovn_lb_ip - lbs = [] - for ovn_lb in ovn_lbs: - if hasattr(ovn_lb, 'datapath_group'): - if ovn_lb.datapath_group: - dp_group_datapaths = ovn_lb.datapath_group[0].datapaths - if (len(dp_group_datapaths) > 1 and - datapath in dp_group_datapaths): - lbs.append(ovn_lb) - else: - # TODO(ltomasbo): Once usage of datapath_group is common, we - # should remove the checks for datapaths - if len(ovn_lb.datapaths) > 1 and datapath in ovn_lb.datapaths: - lbs.append(ovn_lb) return lbs diff --git a/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py b/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py index 11ffec81..724fdacf 100644 --- a/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py +++ b/ovn_bgp_agent/drivers/openstack/watchers/base_watcher.py @@ -25,12 +25,3 @@ class PortBindingChassisEvent(row_event.RowEvent): def _check_ip_associated(self, mac): return len(mac.split(' ')) > 1 - - -class OVNLBMemberEvent(row_event.RowEvent): - def __init__(self, bgp_agent, events): - self.agent = bgp_agent - table = 'Load_Balancer' - super(OVNLBMemberEvent, self).__init__( - events, table, None) - self.event_name = self.__class__.__name__ diff --git a/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py b/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py index c888c6ec..7e19b64a 100644 --- a/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py +++ b/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py @@ -17,7 +17,6 @@ from oslo_log import log as logging from ovsdbapp.backend.ovs_idl import event as row_event from ovn_bgp_agent import constants -from ovn_bgp_agent.drivers.openstack.utils import driver_utils from ovn_bgp_agent.drivers.openstack.watchers import base_watcher @@ -315,18 +314,20 @@ class TenantPortDeletedEvent(base_watcher.PortBindingChassisEvent): self.agent.withdraw_remote_ip(ips, row, chassis) -class OVNLBTenantPortEvent(base_watcher.PortBindingChassisEvent): +class OVNLBVIPPortEvent(base_watcher.PortBindingChassisEvent): def __init__(self, bgp_agent): events = (self.ROW_CREATE, self.ROW_DELETE,) - super(OVNLBTenantPortEvent, self).__init__( + super(OVNLBVIPPortEvent, self).__init__( bgp_agent, events) def match_fn(self, event, row, old): + # The ovn lb balancers are exposed through the cr-lrp, so if the + # local agent does not have any cr-lrp associated there is no need + # to process the event try: # it should not have mac, no chassis, and status down if not row.mac and not row.chassis and not row.up[0]: - if self.agent.ovn_local_lrps != []: - return True + return bool(self.agent.ovn_local_cr_lrps) return False except (IndexError, AttributeError): return False @@ -344,111 +345,9 @@ class OVNLBTenantPortEvent(base_watcher.PortBindingChassisEvent): ovn_lb_ip = ext_n_cidr.split(" ")[0].split("/")[0] if event == self.ROW_DELETE: - self.agent.withdraw_remote_ip([ovn_lb_ip], row) + self.agent.withdraw_ovn_lb(ovn_lb_ip, row) if event == self.ROW_CREATE: - self.agent.expose_remote_ip([ovn_lb_ip], row) - - -class OVNLBMemberUpdateEvent(base_watcher.OVNLBMemberEvent): - def __init__(self, bgp_agent): - events = (self.ROW_UPDATE, self.ROW_DELETE,) - super(OVNLBMemberUpdateEvent, self).__init__( - bgp_agent, events) - - def match_fn(self, event, row, old): - # Only interested in update events related to associated datapaths - if event == self.ROW_DELETE: - return bool(self.agent.ovn_local_cr_lrps) - - if hasattr(row, 'datapath_group'): - try: - # NOTE(ltomasbo): We need to account for datapaths on - # datapath_group being updated instead of changing the group - if row.datapath_group and old.datapath_group: - if (row.datapath_group[0].datapaths == - old.datapath_group[0].datapaths): - return False - except (IndexError, AttributeError): - return False - else: - # TODO(ltomasbo): Once usage of datapath_group is common, we - # should remove the checks for datapaths - try: - if row.datapaths == old.datapaths: - return False - except AttributeError: - return False - - # Only process event if the local node has a cr-lrp ports associated - return bool(self.agent.ovn_local_cr_lrps) - - def run(self, event, row, old): - # Only process event if the local node has a cr-lrp port whose provider - # datapath is included into the loadbalancer. This means the - # loadbalancer has the VIP on a provider network - # Also, the cr-lrp port needs to have subnet datapaths (LS) associated - # to it that include the load balancer - try: - row_dp = row.datapaths - except AttributeError: - row_dp = [] - try: - old_dp = old.datapaths - except AttributeError: - old_dp = [] - if hasattr(row, 'datapath_group'): - if row.datapath_group: - dp_datapaths = row.datapath_group[0].datapaths - if dp_datapaths: - row_dp = dp_datapaths - if hasattr(old, 'datapath_group'): - if old.datapath_group: - dp_datapaths = old.datapath_group[0].datapaths - if dp_datapaths: - old_dp = dp_datapaths - - ovn_lb_cr_lrp = "" - for cr_lrp_port, cr_lrp_info in self.agent.ovn_local_cr_lrps.items(): - if cr_lrp_info.get('provider_datapath') not in row_dp: - continue - if event == self.ROW_DELETE or not row.vips: - if row.name in cr_lrp_info['ovn_lbs']: - ovn_lb_cr_lrp = cr_lrp_port - break - else: - # old.datapath needed for members deletion on different subnet - match_subnets_datapaths = [ - subnet_dp for subnet_dp in cr_lrp_info[ - 'subnets_datapath'].values() - if (subnet_dp in row_dp)] - if match_subnets_datapaths: - ovn_lb_cr_lrp = cr_lrp_port - break - if not ovn_lb_cr_lrp: - return - - with _SYNC_STATE_LOCK.read_lock(): - if event == self.ROW_DELETE: - # loadbalancer deleted. Withdraw the VIP through the cr-lrp - return self.agent.withdraw_ovn_lb_on_provider(row.name, - ovn_lb_cr_lrp) - - if not row.vips: - # last member deleted. Withdraw the VIP through the cr-lrp - return self.agent.withdraw_ovn_lb_on_provider(row.name, - ovn_lb_cr_lrp) - - # NOTE(ltomasbo): It is assumed that the rest of the datapaths in - # the datapaths fields belongs to networks (Logical_Switch) - # connected to the provider network datapath through a single - # router (cr-lrp) - if len(old_dp) <= 1 and len(row_dp) > 1: - # first member added, time to expose the VIP through the cr-lrp - for vip in row.vips.keys(): - ip = driver_utils.parse_vip_from_lb_table(vip) - if ip: - return self.agent.expose_ovn_lb_on_provider( - row.name, ip, ovn_lb_cr_lrp) + self.agent.expose_ovn_lb(ovn_lb_ip, row) class ChassisCreateEventBase(row_event.RowEvent): 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 3b9cfd21..93a1e2af 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 @@ -53,9 +53,9 @@ class TestOVNBGPDriver(test_base.TestCase): self.ipv6 = '2002::1234:abcd:ffff:c0a8:101' self.fip = '172.24.4.33' self.mac = 'aa:bb:cc:dd:ee:ff' - self.loadbalancer = 'fake-lb' + self.loadbalancer_vip_port = 'fake-lb-vip-port' self.bgp_driver.ovn_lb_vips = { - self.loadbalancer: [self.ipv4, self.ipv6]} + self.loadbalancer_vip_port: [self.ipv4, self.ipv6]} self.bgp_driver.ovs_idl = self.mock_ovs_idl self.cr_lrp0 = 'cr-fake-logical-port' @@ -67,7 +67,7 @@ class TestOVNBGPDriver(test_base.TestCase): 'ips': [self.fip], 'subnets_datapath': {self.lrp0: 'fake-lrp-dp'}, 'subnets_cidr': ['192.168.1.1/24'], - 'ovn_lbs': [], + 'ovn_lb_vips': [], 'bridge_device': self.bridge, 'bridge_vlan': None}, self.cr_lrp1: {'provider_datapath': 'fake-provider-dp2'}} @@ -816,11 +816,82 @@ class TestOVNBGPDriver(test_base.TestCase): ret = self.bgp_driver._get_bridge_for_datapath('fake-dp') self.assertEqual((None, None), ret) - def test_expose_ovn_lb_on_provider(self): + def test_expose_ovn_lb(self): + mock_process_ovn_lb = mock.patch.object( + self.bgp_driver, '_process_ovn_lb').start() + self.bgp_driver.expose_ovn_lb('fake-ip', 'fake-row') + mock_process_ovn_lb.assert_called_once_with( + 'fake-ip', 'fake-row', constants.EXPOSE) + + def test_withdraw_ovn_lb(self): + mock_process_ovn_lb = mock.patch.object( + self.bgp_driver, '_process_ovn_lb').start() + self.bgp_driver.withdraw_ovn_lb('fake-ip', 'fake-row') + mock_process_ovn_lb.assert_called_once_with( + 'fake-ip', 'fake-row', constants.WITHDRAW) + + def _test_process_ovn_lb(self, action, provider=False): + 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() + mock_expose_ovn_lb = mock.patch.object( + self.bgp_driver, '_expose_ovn_lb_on_provider').start() + mock_withdraw_ovn_lb = mock.patch.object( + self.bgp_driver, '_withdraw_ovn_lb_on_provider').start() + + self.sb_idl.is_provider_network.return_value = provider + ip = 'fake-vip-ip' + row = fakes.create_object({ + 'logical_port': 'fake-vip', + 'type': constants.OVN_VM_VIF_PORT_TYPE, + 'datapath': 'fake-provider-dp'}) + + 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() + mock_expose_ovn_lb.assert_called_once_with( + ip, row.logical_port, self.cr_lrp0) + mock_withdraw_ovn_lb.assert_not_called() + else: + mock_expose_ovn_lb.assert_not_called() + mock_withdraw_ovn_lb.assert_not_called() + 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() + mock_expose_ovn_lb.assert_not_called() + mock_withdraw_ovn_lb.assert_called_once_with( + row.logical_port, self.cr_lrp0) + else: + mock_expose_ovn_lb.assert_not_called() + mock_withdraw_ovn_lb.assert_not_called() + mock_expose_remote_ip.assert_not_called() + mock_withdraw_remote_ip.assert_called_once_with([ip], row) + + def test__process_ovn_lb_expose_provider(self): + self._test_process_ovn_lb(action=constants.EXPOSE, provider=True) + + def test__process_ovn_lb_expose_no_provider(self): + self._test_process_ovn_lb(action=constants.EXPOSE) + + def test__process_ovn_lb_withdraw_provider(self): + self._test_process_ovn_lb(action=constants.WITHDRAW, provider=True) + + 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): 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) + 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( @@ -829,10 +900,10 @@ class TestOVNBGPDriver(test_base.TestCase): @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.loadbalancer, self.cr_lrp0) + self.bgp_driver._withdraw_ovn_lb_on_provider( + self.loadbalancer_vip_port, self.cr_lrp0) # Assert that the del methods were called expected_calls = [mock.call(CONF.bgp_nic, [self.ipv4]), @@ -1048,17 +1119,11 @@ class TestOVNBGPDriver(test_base.TestCase): 'datapath': 'fake-router-dp'}) ovn_lb_vip = '172.24.4.5' - ovn_lb_vip_port = ovn_lb_vip + ':80' - ovn_lb1 = fakes.create_object({ - 'name': 'ovn_lb1', 'datapaths': [self.cr_lrp0, 'fake-lrp-dp'], - 'vips': {ovn_lb_vip_port: '192.168.100.5:::8080'}}) - ovn_lb2 = fakes.create_object({ - 'name': 'ovn_lb2', 'datapaths': [self.cr_lrp0, 'fake-lrp1-db'], - 'vips': {'172.24.4.6:80': '192.168.200.5:::8080'}}) - self.sb_idl.get_ovn_lb_on_provider_datapath.return_value = [ovn_lb1, - ovn_lb2] - mock_process_ovn_lb = mock.patch.object( - self.bgp_driver, '_process_ovn_lb').start() + ovn_lb_vips = {'fake-vip-port': ovn_lb_vip} + self.sb_idl.get_ovn_lb_vips_on_provider_datapath.return_value = ( + ovn_lb_vips) + mock_expose_ovn_lb = mock.patch.object( + self.bgp_driver, '_expose_ovn_lb_on_provider').start() ips = [self.ipv4, self.ipv6] self.bgp_driver.expose_ip(ips, row) @@ -1085,10 +1150,8 @@ class TestOVNBGPDriver(test_base.TestCase): mock.call(lrp1, self.cr_lrp0), mock.call(lrp2, self.cr_lrp0)] mock_process_lrp_port.assert_has_calls(expected_calls) - - expected_calls = [mock.call(ovn_lb1, self.cr_lrp0), - mock.call(ovn_lb2, self.cr_lrp0)] - mock_process_ovn_lb.assert_has_calls(expected_calls) + mock_expose_ovn_lb.assert_called_once_with( + ovn_lb_vip, 'fake-vip-port', self.cr_lrp0) @mock.patch.object(linux_net, 'del_ip_route') @mock.patch.object(linux_net, 'del_ip_rule') @@ -1490,16 +1553,17 @@ class TestOVNBGPDriver(test_base.TestCase): self.bgp_driver, '_expose_provider_port').start() mock_process_lrp_port = mock.patch.object( self.bgp_driver, '_process_lrp_port').start() - mock_process_ovn_lb = mock.patch.object( - self.bgp_driver, '_process_ovn_lb').start() + mock_expose_ovn_lb = mock.patch.object( + self.bgp_driver, '_expose_ovn_lb_on_provider').start() ips = [self.ipv4, self.ipv6] mock_ip_version.side_effect = [constants.IP_VERSION_4, constants.IP_VERSION_6] dp_port0 = mock.Mock() self.sb_idl.get_lrp_ports_for_router.return_value = [dp_port0] - ovn_lb = mock.Mock() - self.sb_idl.get_ovn_lb_on_provider_datapath.return_value = [ovn_lb] + ovn_lb_vips = {'fake-vip-port': 'fake-vip-ip'} + self.sb_idl.get_ovn_lb_vips_on_provider_datapath.return_value = ( + ovn_lb_vips) self.bgp_driver._expose_cr_lrp_port( ips, self.mac, self.bridge, None, router_datapath='fake-router-dp', @@ -1511,7 +1575,8 @@ class TestOVNBGPDriver(test_base.TestCase): lladdr=self.mac) mock_ndp_proxy.assert_called_once_with(self.ipv6, self.bridge, None) mock_process_lrp_port.assert_called_once_with(dp_port0, self.cr_lrp0) - mock_process_ovn_lb.assert_called_once_with(ovn_lb, self.cr_lrp0) + mock_expose_ovn_lb.assert_called_once_with( + 'fake-vip-ip', 'fake-vip-port', self.cr_lrp0) @mock.patch.object(linux_net, 'del_ndp_proxy') @mock.patch.object(linux_net, 'get_ip_version') @@ -1521,12 +1586,12 @@ class TestOVNBGPDriver(test_base.TestCase): 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() + 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 = mock.Mock() + ovn_lb_vip_port = mock.Mock() gateway = { 'ips': ips, 'provider_datapath': 'fake-provider-dp', @@ -1534,7 +1599,7 @@ class TestOVNBGPDriver(test_base.TestCase): 'bridge_device': self.bridge, 'bridge_vlan': 10, 'mac': self.mac, - 'ovn_lbs': [ovn_lb]} + 'ovn_lb_vips': [ovn_lb_vip_port]} self.bgp_driver.ovn_local_cr_lrps = {'gateway_port': gateway} self.bgp_driver._withdraw_cr_lrp_port( @@ -1549,7 +1614,7 @@ class TestOVNBGPDriver(test_base.TestCase): mock_withdraw_lrp_port.assert_called_once_with('192.168.1.1/24', None, 'gateway_port') mock_withdraw_ovn_lb_on_provider.assert_called_once_with( - ovn_lb, 'gateway_port') + ovn_lb_vip_port, 'gateway_port') @mock.patch.object(linux_net, 'add_ip_route') @mock.patch.object(linux_net, 'add_ip_rule') 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 31066e8a..b569b0fe 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 @@ -26,7 +26,6 @@ from ovn_bgp_agent.drivers.openstack.utils import ovn as ovn_utils from ovn_bgp_agent import exceptions from ovn_bgp_agent.tests import base as test_base from ovn_bgp_agent.tests.unit import fakes -from ovn_bgp_agent.tests import utils CONF = cfg.CONF @@ -489,46 +488,19 @@ class TestOvsdbSbOvnIdl(test_base.TestCase): self.assertEqual([port1], ret) - def test_get_ovn_lb_on_provider_datapath(self): - dp = 'fake-datapath' - dpg1 = utils.create_row(_uuid='fake_dp_group', - datapaths=['dp1']) - dpg2 = utils.create_row(_uuid='fake_dp_group', - datapaths=['dp1', dp]) - dpg3 = utils.create_row(_uuid='fake_dp_group', - datapaths=[dp]) + def test_get_ovn_lb_vips_on_provider_datapath(self): + port0 = fakes.create_object({ + 'logical_port': 'fake-port-0', + 'external_ids': {constants.OVN_CIDRS_EXT_ID_KEY: '10.0.0.15/24'}}) + port1 = fakes.create_object({'logical_port': 'fake-port-1', + 'external_ids': {}}) - ovn_lb1 = fakes.create_object( - {'name': 'ovn-lb1', 'datapath_group': [dpg1]}) - ovn_lb2 = fakes.create_object( - {'name': 'ovn-lb2', 'datapath_group': [dpg2]}) - ovn_lb3 = fakes.create_object( - {'name': 'ovn-lb3', 'datapath_group': [dpg3]}) + self.sb_idl.db_find_rows.return_value.execute.return_value = [ + port0, port1] - self.sb_idl.db_list_rows.return_value.execute.return_value = [ - ovn_lb1, ovn_lb2, ovn_lb3] - - ret = self.sb_idl.get_ovn_lb_on_provider_datapath(dp) - self.assertIn(ovn_lb2, ret) - self.assertNotIn(ovn_lb1, ret) - self.assertNotIn(ovn_lb3, ret) - - def test_get_ovn_lb_on_provider_datapath_no_dadtapath_group(self): - dp = 'fake-datapath' - ovn_lb1 = fakes.create_object( - {'name': 'ovn-lb1', 'datapaths': ['dp1']}) - ovn_lb2 = fakes.create_object( - {'name': 'ovn-lb2', 'datapaths': ['dp1', dp]}) - ovn_lb3 = fakes.create_object( - {'name': 'ovn-lb3', 'datapaths': [dp]}) - - self.sb_idl.db_list_rows.return_value.execute.return_value = [ - ovn_lb1, ovn_lb2, ovn_lb3] - - ret = self.sb_idl.get_ovn_lb_on_provider_datapath(dp) - self.assertIn(ovn_lb2, ret) - self.assertNotIn(ovn_lb1, ret) - self.assertNotIn(ovn_lb3, ret) + ret = self.sb_idl.get_ovn_lb_vips_on_provider_datapath('fake-datapath') + expected_return = {'fake-port-0': '10.0.0.15'} + self.assertEqual(expected_return, ret) class TestOvnSbIdl(test_base.TestCase): diff --git a/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_bgp_watcher.py b/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_bgp_watcher.py index 5a9ce886..a929eca7 100644 --- a/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_bgp_watcher.py +++ b/ovn_bgp_agent/tests/unit/drivers/openstack/watchers/test_bgp_watcher.py @@ -736,14 +736,14 @@ class TestTenantPortDeletedEvent(test_base.TestCase): ['10.10.1.16'], row, mock.ANY) -class TestOVNLBTenantPortEvent(test_base.TestCase): +class OVNLBVIPPortEvent(test_base.TestCase): def setUp(self): - super(TestOVNLBTenantPortEvent, self).setUp() + super(OVNLBVIPPortEvent, self).setUp() self.chassis = '935f91fa-b8f8-47b9-8b1b-3a7a90ef7c26' self.agent = mock.Mock(chassis=self.chassis) - self.agent.ovn_local_lrps = ['172.24.100.111'] - self.event = bgp_watcher.OVNLBTenantPortEvent(self.agent) + self.agent.ovn_local_cr_lrps = {'fake-cr-lrp-port': {}} + self.event = bgp_watcher.OVNLBVIPPortEvent(self.agent) def test_match_fn(self): row = utils.create_row(chassis=[], mac=[], up=[False]) @@ -763,12 +763,12 @@ class TestOVNLBTenantPortEvent(test_base.TestCase): row = utils.create_row(chassis=[], mac=[], up=[True]) self.assertFalse(self.event.match_fn(mock.Mock(), row, mock.Mock())) - def test_match_fn_empty_ovn_local_lrps(self): - self.agent.ovn_local_lrps = [] + def test_match_fn_empty_ovn_local_cr_lrps(self): + self.agent.ovn_local_cr_lrps = [] row = utils.create_row(chassis=[], mac=[], up=[False]) self.assertFalse(self.event.match_fn(mock.Mock(), row, mock.Mock())) - def test_match_fn_index_error(self): + def test_match_fn_attribute_error(self): row = utils.create_row(mac=[]) self.assertFalse(self.event.match_fn(mock.Mock(), row, mock.Mock())) @@ -779,8 +779,8 @@ class TestOVNLBTenantPortEvent(test_base.TestCase): up=[False], external_ids={ constants.OVN_CIDRS_EXT_ID_KEY: "10.10.1.16/24"}) self.event.run(event, row, mock.Mock()) - self.agent.expose_remote_ip.assert_called_once_with( - ['10.10.1.16'], row) + self.agent.expose_ovn_lb.assert_called_once_with( + '10.10.1.16', row) def test_run_delete(self): event = self.event.ROW_DELETE @@ -789,16 +789,16 @@ class TestOVNLBTenantPortEvent(test_base.TestCase): up=[False], external_ids={ constants.OVN_CIDRS_EXT_ID_KEY: "10.10.1.16/24"}) self.event.run(event, row, mock.Mock()) - self.agent.withdraw_remote_ip.assert_called_once_with( - ['10.10.1.16'], row) + self.agent.withdraw_ovn_lb.assert_called_once_with( + '10.10.1.16', row) def test_run_no_external_id(self): row = utils.create_row( type=constants.OVN_VM_VIF_PORT_TYPE, mac=[], chassis=[], up=[False], external_ids={}) self.event.run(mock.Mock(), row, mock.Mock()) - self.agent.expose_remote_ip.assert_not_called() - self.agent.withdraw_remote_ip.assert_not_called() + self.agent.expose_ovn_lb.assert_not_called() + self.agent.withdraw_ovn_lb.assert_not_called() def test_run_wrong_type(self): row = utils.create_row( @@ -806,176 +806,8 @@ class TestOVNLBTenantPortEvent(test_base.TestCase): up=[False], external_ids={ constants.OVN_CIDRS_EXT_ID_KEY: "10.10.1.16/24"}) self.event.run(mock.Mock(), row, mock.Mock()) - self.agent.expose_remote_ip.assert_not_called() - self.agent.withdraw_remote_ip.assert_not_called() - - -class TestOVNLBMemberUpdateEvent(test_base.TestCase): - - def setUp(self): - super(TestOVNLBMemberUpdateEvent, self).setUp() - self.chassis = '935f91fa-b8f8-47b9-8b1b-3a7a90ef7c26' - self.agent = mock.Mock(chassis=self.chassis) - self.agent.ovn_local_cr_lrps = { - 'cr-lrp1': {'provider_datapath': 'dp1', - 'subnets_datapath': {'lrp1': 's_dp1'}, - 'ovn_lbs': 'ovn-lb1'}} - self.event = bgp_watcher.OVNLBMemberUpdateEvent(self.agent) - - def test_match_fn(self): - row = utils.create_row(datapaths=['dp1', 'dp2']) - old = utils.create_row(datapaths=['dp1']) - self.assertTrue(self.event.match_fn(mock.Mock(), row, old)) - - def test_match_fn_no_dp_change(self): - row = utils.create_row(datapaths=['dp1']) - old = utils.create_row(datapaths=['dp1']) - self.assertFalse(self.event.match_fn(mock.Mock(), row, old)) - - def test_match_fn_removed_dp(self): - row = utils.create_row(datapaths=['dp1']) - old = utils.create_row(datapaths=[]) - self.assertTrue(self.event.match_fn(mock.Mock(), row, old)) - - def test_match_fn_no_cr_lrp(self): - self.agent.ovn_local_cr_lrps = {} - row = utils.create_row(datapaths=['dp1']) - old = utils.create_row(datapaths=['dp1', 'dp2']) - self.assertFalse(self.event.match_fn(mock.Mock(), row, old)) - - def test_match_fn_attribute_error(self): - row = utils.create_row(mac=[]) - self.assertFalse(self.event.match_fn(mock.Mock(), row, mock.Mock())) - - def test_match_fn_delete(self): - event = self.event.ROW_DELETE - row = utils.create_row(mac=[]) - self.assertTrue(self.event.match_fn(event, row, mock.Mock())) - - def test_match_fn_dp_group(self): - dpg1 = utils.create_row(_uuid='fake_dp_group', - datapaths=['dp1', 'dp2']) - dpg2 = utils.create_row(_uuid='fake_dp_group', - datapaths=['dp1']) - row = utils.create_row(datapath_group=[dpg1]) - old = utils.create_row(datapath_group=[dpg2]) - self.assertTrue(self.event.match_fn(mock.Mock(), row, old)) - - def test_match_fn_dp_group_attribute_error(self): - dpg1 = utils.create_row(_uuid='fake_dp_group') - row = utils.create_row(datapath_group=[dpg1]) - old = utils.create_row(datapath_group=[dpg1]) - self.assertFalse(self.event.match_fn(mock.Mock(), row, old)) - - def test_match_fn_dp_group_no_dp_change(self): - dpg1 = utils.create_row(_uuid='fake_dp_group', - datapaths=['dp1', 'dp2']) - row = utils.create_row(datapath_group=[dpg1]) - old = utils.create_row(datapath_group=[dpg1]) - self.assertFalse(self.event.match_fn(mock.Mock(), row, old)) - - def test_match_fn_dp_group_no_old_dp(self): - dpg1 = utils.create_row(_uuid='fake_dp_group', - datapaths=['dp1', 'dp2']) - row = utils.create_row(datapath_group=[dpg1]) - old = utils.create_row(datapath_group=[]) - self.assertTrue(self.event.match_fn(mock.Mock(), row, old)) - - def test_run(self): - dpg1 = utils.create_row(_uuid='fake_dp_group', - datapaths=['dp1', 's_dp1']) - dpg2 = utils.create_row(_uuid='fake_dp_group', - datapaths=['dp1']) - row = utils.create_row(name='ovn-lb1', - datapath_group=[dpg1], - vips={'172.24.100.66:80': '10.0.0.5:8080'}) - old = utils.create_row(name='ovn-lb1', - datapath_group=[dpg2], - vips={'172.24.100.66:80': '10.0.0.5:8080'}) - self.event.run(mock.Mock(), row, old) - self.agent.expose_ovn_lb_on_provider.assert_called_once_with( - 'ovn-lb1', '172.24.100.66', 'cr-lrp1') - self.agent.withdraw_ovn_lb_on_provider.assert_not_called() - - def test_run_no_provider_dp(self): - dpg1 = utils.create_row(_uuid='fake_dp_group', - datapaths=['s_dp2']) - row = utils.create_row(name='ovn-lb1', - datapath_group=[dpg1]) - self.event.run(mock.Mock(), row, row) - self.agent.expose_ovn_lb_on_provider.assert_not_called() - self.agent.withdraw_ovn_lb_on_provider.assert_not_called() - - def test_run_removed_dp(self): - dpg1 = utils.create_row(_uuid='fake_dp_group', - datapaths=['dp1']) - dpg2 = utils.create_row(_uuid='fake_dp_group', - datapaths=['dp1', 's_dp1']) - row = utils.create_row(name='ovn-lb1', - datapath_group=[dpg1], - vips={}) - old = utils.create_row(name='ovn-lb1', - datapath_group=[dpg2], - vips={'172.24.100.66:80': '10.0.0.5:8080'}) - self.event.run(mock.Mock(), row, old) - self.agent.expose_ovn_lb_on_provider.assert_not_called() - self.agent.withdraw_ovn_lb_on_provider.assert_called_once_with( - 'ovn-lb1', 'cr-lrp1') - - def test_run_no_match_subnets_dp(self): - dpg1 = utils.create_row(_uuid='fake_dp_group', - datapaths=['dp1', 's_dp2']) - dpg2 = utils.create_row(_uuid='fake_dp_group', - datapaths=['dp1']) - row = utils.create_row(name='ovn-lb1', - datapath_group=[dpg1], - vips={'172.24.100.66:80': '10.0.0.5:8080'}) - old = utils.create_row(name='ovn-lb1', - datapath_group=[dpg2], - vips={'172.24.100.66:80': '10.0.0.5:8080'}) - self.event.run(mock.Mock(), row, old) - self.agent.expose_ovn_lb_on_provider.assert_not_called() - self.agent.withdraw_ovn_lb_on_provider.assert_not_called() - - def test_run_no_member(self): - dpg1 = utils.create_row(_uuid='fake_dp_group', - datapaths=['dp1']) - row = utils.create_row(name='ovn-lb2', - datapath_group=[dpg1], - vips={}) - old = utils.create_row(name='ovn-lb2', - datapath_group=[dpg1]) - self.event.run(mock.Mock(), row, old) - self.agent.expose_ovn_lb_on_provider.assert_not_called() - self.agent.withdraw_ovn_lb_on_provider.assert_not_called() - - def test_run_member_removal(self): - dpg1 = utils.create_row(_uuid='fake_dp_group', - datapaths=['dp1', 's_dp1']) - dpg2 = utils.create_row(_uuid='fake_dp_group', - datapaths=['dp1', 's_dp1', 's_dp2']) - row = utils.create_row(name='ovn-lb1', - datapath_group=[dpg1], - vips={'172.24.100.66:80': '10.0.0.5:8080'}) - old = utils.create_row(name='ovn-lb1', - datapath_group=[dpg2], - vips={'172.24.100.66:80': - '10.0.0.5:8080,10.0.0.6:8080'}) - self.event.run(mock.Mock(), row, old) - self.agent.expose_ovn_lb_on_provider.assert_not_called() - self.agent.withdraw_ovn_lb_on_provider.assert_not_called() - - def test_run_delete(self): - event = self.event.ROW_DELETE - dpg1 = utils.create_row(_uuid='fake_dp_group', - datapaths=['dp1', 's_dp1']) - row = utils.create_row(name='ovn-lb1', - datapath_group=[dpg1]) - old = utils.create_row() - self.event.run(event, row, old) - self.agent.expose_ovn_lb_on_provider.assert_not_called() - self.agent.withdraw_ovn_lb_on_provider.assert_called_once_with( - 'ovn-lb1', 'cr-lrp1') + self.agent.expose_ovn_lb.assert_not_called() + self.agent.withdraw_ovn_lb.assert_not_called() class TestChassisCreateEvent(test_base.TestCase):