diff --git a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py index 9877cbfa..37a85262 100644 --- a/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py +++ b/ovn_bgp_agent/drivers/openstack/ovn_bgp_driver.py @@ -182,12 +182,15 @@ class OVNBGPDriver(driver_api.AgentDriverBase): ovn_ip_rules = linux_net.get_ovn_ip_rules( self.ovn_routing_tables.values()) - # add missing routes/ips for fips/provider VMs + # add missing routes/ips for IPs on provider network ports = self.sb_idl.get_ports_on_chassis(self.chassis) for port in ports: self._ensure_port_exposed(port, exposed_ips, ovn_ip_rules) - cr_lrp_ports = self.sb_idl.get_cr_lrp_ports_on_chassis(self.chassis) + # this information is only available when there are cr-lrps add + # missing routes/ips for FIPs associated to VMs/LBs on the chassis + cr_lrp_ports = self.sb_idl.get_cr_lrp_ports_on_chassis( + self.chassis) for cr_lrp_port in cr_lrp_ports: self._ensure_cr_lrp_associated_ports_exposed( cr_lrp_port, exposed_ips, ovn_ip_rules) @@ -196,45 +199,16 @@ class OVNBGPDriver(driver_api.AgentDriverBase): lrp_ports = self.sb_idl.get_lrp_ports_for_router( cr_lrp_info['router_datapath']) for lrp in lrp_ports: - if (lrp.chassis or - not lrp.logical_port.startswith('lrp-') or - "chassis-redirect-port" in lrp.options.keys()): - continue - subnet_datapath = self.sb_idl.get_port_datapath( - lrp.logical_port.split('lrp-')[1]) - self.ovn_local_cr_lrps[cr_lrp_port]['subnets_datapath'].update( - {lrp.logical_port: subnet_datapath}) - # add missing route/ips for tenant network VMs - if self._expose_tenant_networks: - self._ensure_network_exposed( - lrp, cr_lrp_port, exposed_ips, ovn_ip_rules) + self._process_lrp_port(lrp, cr_lrp_port, exposed_ips, + ovn_ip_rules) # add missing routes/ips related to ovn-octavia loadbalancers # on the provider networks ovn_lbs = self.sb_idl.get_ovn_lb_on_provider_datapath( cr_lrp_info['provider_datapath']) for ovn_lb in ovn_lbs: - match_subnets_datapaths = [ - subnet_dp for subnet_dp in cr_lrp_info[ - 'subnets_datapath'].values() - if subnet_dp in ovn_lb.datapaths] - if not match_subnets_datapaths: - continue - - 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 ip in exposed_ips: - exposed_ips.remove(ip) - 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) - self.ovn_local_cr_lrps[cr_lrp_port]['ovn_lbs'].append( - ovn_lb.name) + self._process_ovn_lb(ovn_lb, cr_lrp_port, exposed_ips, + ovn_ip_rules) # remove extra routes/ips # remove all the leftovers on the list of current ips on dev OVN @@ -334,6 +308,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase): (port.type == constants.OVN_VM_VIF_PORT_TYPE and not port.chassis)): return + try: port_ips = port.mac[0].split(' ')[1:] except IndexError: @@ -354,61 +329,6 @@ class OVNBGPDriver(driver_api.AgentDriverBase): ip_dst = "{}/32".format(port_ip) ovn_ip_rules.pop(ip_dst, None) - def _ensure_network_exposed(self, router_port, gateway_port, - exposed_ips=[], ovn_ip_rules={}): - gateway = self.ovn_local_cr_lrps[gateway_port] - gateway_ips = [ip.split('/')[0] for ip in gateway['ips']] - try: - router_port_ip = router_port.mac[0].split(' ')[1] - except IndexError: - return - if not CONF.expose_tenant_networks: - # This means CONF.expose_ipv6_gua_tenant_networks is enabled - if not driver_utils.is_ipv6_gua(router_port_ip): - return - router_ip = router_port_ip.split('/')[0] - if router_ip in gateway_ips: - return - self.ovn_local_lrps.update({router_port.logical_port: gateway_port}) - bridge_device, bridge_vlan = self._get_bridge_for_datapath( - gateway['provider_datapath']) - - try: - linux_net.add_ip_rule(router_port_ip, - self.ovn_routing_tables[bridge_device], - bridge_device) - except agent_exc.InvalidPortIP: - LOG.exception("Invalid IP to create a rule for the network router" - " interface port: %s", router_port_ip) - return - - ovn_ip_rules.pop(router_port_ip, None) - - # NOTE(ltomasbo): This assumes the provider network can only have - # (at most) 2 subnets, one for IPv4, one for IPv6 - router_port_ip_version = linux_net.get_ip_version(router_port_ip) - for gateway_ip in gateway_ips: - if linux_net.get_ip_version(gateway_ip) == router_port_ip_version: - linux_net.add_ip_route( - self.ovn_routing_tables_routes, - router_ip, - self.ovn_routing_tables[bridge_device], - bridge_device, - vlan=bridge_vlan, - mask=router_port_ip.split("/")[1], - via=gateway_ip) - break - - network_port_datapath = self.sb_idl.get_port_datapath( - router_port.options['peer']) - if network_port_datapath: - ports = self.sb_idl.get_ports_on_datapath( - network_port_datapath) - for port in ports: - self._expose_tenant_port( - port, ip_version=router_port_ip_version, - exposed_ips=exposed_ips, ovn_ip_rules=ovn_ip_rules) - def _withdraw_provider_port(self, port_ips, provider_datapath, bridge_device=None, bridge_vlan=None, lladdr=None): @@ -435,42 +355,6 @@ class OVNBGPDriver(driver_api.AgentDriverBase): self.ovn_routing_tables[bridge_device], bridge_device, vlan=bridge_vlan) - def _remove_network_exposed(self, subnet_cidr, gateway): - gateway_ips = [ip.split('/')[0] for ip in gateway['ips']] - subnet_ip = subnet_cidr.split('/')[0] - - for subnet_logical_port in gateway['subnets_datapath'].keys(): - if subnet_logical_port in self.ovn_local_lrps.keys(): - self.ovn_local_lrps.pop(subnet_logical_port) - - linux_net.del_ip_rule( - subnet_cidr, self.ovn_routing_tables[gateway['bridge_device']], - gateway['bridge_device']) - - subnet_ip_version = linux_net.get_ip_version(subnet_cidr) - for gateway_ip in gateway_ips: - if linux_net.get_ip_version(gateway_ip) == subnet_ip_version: - linux_net.del_ip_route( - self.ovn_routing_tables_routes, - subnet_ip, - self.ovn_routing_tables[gateway['bridge_device']], - gateway['bridge_device'], - vlan=gateway['bridge_vlan'], - mask=subnet_cidr.split("/")[1], - via=gateway_ip) - if (linux_net.get_ip_version(gateway_ip) == - constants.IP_VERSION_6): - net = ipaddress.IPv6Network(subnet_ip, strict=False) - else: - net = ipaddress.IPv4Network(subnet_ip, strict=False) - break - # Check if there are VMs on the network - # and if so withdraw the routes - vms_on_net = linux_net.get_exposed_ips_on_network( - CONF.bgp_nic, net) - if vms_on_net: - linux_net.delete_exposed_ips(vms_on_net, CONF.bgp_nic) - def _get_bridge_for_datapath(self, datapath): network_name, network_tag = self.sb_idl.get_network_name_and_tag( datapath, self.ovn_bridge_mappings.keys()) @@ -571,68 +455,28 @@ class OVNBGPDriver(driver_api.AgentDriverBase): if not cr_lrp_datapath: return [] - LOG.debug("Adding BGP route for CR-LRP Port %s", ips) - # Keeping information about the associated network for - # tenant network advertisement bridge_device, bridge_vlan = self._get_bridge_for_datapath( cr_lrp_datapath) + mac = row.mac[0].split(' ')[0] + # Keeping information about the associated network for + # tenant network advertisement self.ovn_local_cr_lrps[row.logical_port] = { 'router_datapath': row.datapath, 'provider_datapath': cr_lrp_datapath, 'ips': ips, + 'mac': mac, 'subnets_datapath': {}, 'subnets_cidr': [], 'ovn_lbs': [], 'bridge_vlan': bridge_vlan, 'bridge_device': bridge_device } - ips_without_mask = [ip.split("/")[0] for ip in ips] - self._expose_provider_port(ips_without_mask, cr_lrp_datapath, - bridge_device, bridge_vlan, - lladdr=row.mac[0].split(' ')[0]) - # add proxy ndp config for ipv6 - for ip in ips: - if linux_net.get_ip_version(ip) == constants.IP_VERSION_6: - linux_net.add_ndp_proxy(ip, bridge_device, bridge_vlan) - LOG.debug("Added BGP route for CR-LRP Port %s", ips) - # Check if there are networks attached to the router, - # and if so, add the needed routes/rules - lrp_ports = self.sb_idl.get_lrp_ports_for_router(row.datapath) - for lrp in lrp_ports: - if (lrp.chassis or - not lrp.logical_port.startswith('lrp-') or - "chassis-redirect-port" in lrp.options.keys()): - continue - subnet_datapath = self.sb_idl.get_port_datapath( - lrp.logical_port.split('lrp-')[1]) - self.ovn_local_cr_lrps[row.logical_port][ - 'subnets_datapath'].update( - {lrp.logical_port: subnet_datapath}) - try: - subnet_cidr = lrp.mac[0].split(" ")[1] - except IndexError: - # This should not happen: subnet without CIDR - subnet_cidr = None - if subnet_cidr: - self.ovn_local_cr_lrps[row.logical_port][ - 'subnets_cidr'].append(subnet_cidr) - if self._expose_tenant_networks: - self._ensure_network_exposed( - lrp, row.logical_port) + 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) - ovn_lbs = self.sb_idl.get_ovn_lb_on_provider_datapath( - cr_lrp_datapath) - for ovn_lb in ovn_lbs: - if any([True for ovn_dp in ovn_lb.datapaths - if ovn_dp in self.ovn_local_cr_lrps[ - row.logical_port]['subnets_datapath'].values()]): - 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, row.logical_port) - self.ovn_local_cr_lrps[row.logical_port][ - 'ovn_lbs'].append(ovn_lb.name) return ips return [] @@ -692,48 +536,14 @@ class OVNBGPDriver(driver_api.AgentDriverBase): if not cr_lrp_datapath: return - LOG.debug("Deleting BGP route for CR-LRP Port %s", ips) - # Removing information about the associated network for - # tenant network advertisement - ips_without_mask = [ip.split("/")[0] for ip in ips] bridge_vlan = self.ovn_local_cr_lrps[row.logical_port].get( 'bridge_vlan') bridge_device = self.ovn_local_cr_lrps[row.logical_port].get( 'bridge_device') - self._withdraw_provider_port(ips_without_mask, cr_lrp_datapath, - bridge_device=bridge_device, - bridge_vlan=bridge_vlan, - lladdr=row.mac[0].split(' ')[0]) - # del proxy ndp config for ipv6 - for ip in ips_without_mask: - if linux_net.get_ip_version(ip) == constants.IP_VERSION_6: - cr_lrps_on_same_provider = [ - p for p in self.ovn_local_cr_lrps.values() - if p['provider_datapath'] == cr_lrp_datapath] - if (len(cr_lrps_on_same_provider) > 1): - linux_net.del_ndp_proxy(ip, bridge_device, bridge_vlan) - LOG.debug("Deleted BGP route for CR-LRP Port %s", ips) - - # Check if there are networks attached to the router, - # and if so delete the needed routes/rules - local_cr_lrp_info = self.ovn_local_cr_lrps.get(row.logical_port) - for subnet_cidr in local_cr_lrp_info['subnets_cidr']: - self._remove_network_exposed(subnet_cidr, local_cr_lrp_info) - - # check if there are loadbalancers associated to the router, - # and if so delete the needed routes/rules - ovn_lbs = self.ovn_local_cr_lrps[row.logical_port][ - 'ovn_lbs'].copy() - for ovn_lb in ovn_lbs: - self.withdraw_ovn_lb_on_provider( - ovn_lb, row.logical_port) - self.ovn_local_cr_lrps[row.logical_port][ - 'ovn_lbs'].remove(ovn_lb) - try: - del self.ovn_local_cr_lrps[row.logical_port] - except KeyError: - LOG.debug("Gateway port %s already cleanup from the " - "agent", row.logical_port) + mac = row.mac[0].split(' ')[0] + self._withdraw_cr_lrp_port(ips, mac, bridge_device, bridge_vlan, + provider_datapath=cr_lrp_datapath, + cr_lrp_port=row.logical_port) @lockutils.synchronized('bgp') def expose_remote_ip(self, ips, row): @@ -758,7 +568,7 @@ class OVNBGPDriver(driver_api.AgentDriverBase): ips, self.chassis) @lockutils.synchronized('bgp') - def withdraw_remote_ip(self, ips, row): + 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 @@ -779,51 +589,181 @@ class OVNBGPDriver(driver_api.AgentDriverBase): LOG.debug("Deleted BGP route for tenant IP %s on chassis %s", ips, self.chassis) - @lockutils.synchronized('bgp') - def expose_subnet(self, ip, row): - cr_lrp = self.sb_idl.is_router_gateway_on_chassis(row.datapath, - self.chassis) - if not cr_lrp: + def _process_cr_lrp_port(self, cr_lrp_port_name, provider_datapath, + router_port): + ips = router_port.mac[0].split(' ')[1:] + bridge_device, bridge_vlan = self._get_bridge_for_datapath( + provider_datapath) + mac = router_port.mac[0].split(' ')[0] + self.ovn_local_cr_lrps[cr_lrp_port_name] = { + 'router_datapath': router_port.datapath, + 'provider_datapath': provider_datapath, + 'ips': ips, + 'mac': mac, + 'subnets_datapath': {}, + 'subnets_cidr': [], + 'ovn_lbs': [], + 'bridge_vlan': bridge_vlan, + 'bridge_device': bridge_device + } + # NOTE: This is like if it was the cr-lrp action on expose_ip + return self._expose_cr_lrp_port( + ips, mac, bridge_device, bridge_vlan, + router_datapath=router_port.datapath, + provider_datapath=provider_datapath, + cr_lrp_port=cr_lrp_port_name) + + def _process_lrp_port(self, lrp, associated_cr_lrp, exposed_ips=[], + ovn_ip_rules={}): + if (lrp.chassis or + not lrp.logical_port.startswith('lrp-') or + "chassis-redirect-port" in lrp.options.keys() or + associated_cr_lrp.strip('cr-') == lrp.logical_port): return + # add missing route/ips for tenant network VMs + if self._expose_tenant_networks: + try: + lrp_ip = lrp.mac[0].split(' ')[1] + except IndexError: + # This should not happen: subnet without CIDR + return - cr_lrp_info = self.ovn_local_cr_lrps.get(cr_lrp, {}) - cr_lrp_datapath = cr_lrp_info.get('provider_datapath') - if not cr_lrp_datapath: - return + subnet_datapath = self.sb_idl.get_port_datapath( + lrp.options['peer']) + self._expose_lrp_port(lrp_ip, lrp.logical_port, + associated_cr_lrp, subnet_datapath, + exposed_ips=exposed_ips, + ovn_ip_rules=ovn_ip_rules) - network_port_datapath = self.sb_idl.get_port_datapath( - row.options['peer']) - # update information needed for the loadbalancers - self.ovn_local_cr_lrps[cr_lrp]['subnets_datapath'].update( - {row.logical_port: network_port_datapath}) + def _process_ovn_lb(self, ovn_lb, cr_lrp_port, exposed_ips=[], + ovn_ip_rules={}): + if any([True for ovn_dp in ovn_lb.datapaths + if ovn_dp in self.ovn_local_cr_lrps[ + cr_lrp_port]['subnets_datapath'].values()]): + 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) + 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) + # add proxy ndp config for ipv6 + for ip in ips: + if linux_net.get_ip_version(ip) == constants.IP_VERSION_6: + linux_net.add_ndp_proxy(ip, bridge_device, bridge_vlan) + LOG.debug("Added BGP route for CR-LRP Port %s", ips) + + # Check if there are networks attached to the router, + # and if so, add the needed routes/rules + lrp_ports = self.sb_idl.get_lrp_ports_for_router(router_datapath) + for lrp in lrp_ports: + self._process_lrp_port(lrp, cr_lrp_port) + + ovn_lbs = self.sb_idl.get_ovn_lb_on_provider_datapath( + provider_datapath) + for ovn_lb in ovn_lbs: + self._process_ovn_lb(ovn_lb, cr_lrp_port) + + def _withdraw_cr_lrp_port(self, ips, mac, bridge_device, bridge_vlan, + provider_datapath, cr_lrp_port): + LOG.debug("Deleting BGP route for CR-LRP Port %s", ips) + # Removing information about the associated network for + # tenant network advertisement + ips_without_mask = [ip.split("/")[0] for ip in ips] + self._withdraw_provider_port(ips_without_mask, provider_datapath, + bridge_device=bridge_device, + bridge_vlan=bridge_vlan, + lladdr=mac) + # del proxy ndp config for ipv6 + for ip in ips_without_mask: + if linux_net.get_ip_version(ip) == constants.IP_VERSION_6: + cr_lrps_on_same_provider = [ + p for p in self.ovn_local_cr_lrps.values() + if p['provider_datapath'] == provider_datapath] + # if no other cr-lrp port on the same provider + # delete the ndp proxy + if (len(cr_lrps_on_same_provider) <= 1): + linux_net.del_ndp_proxy(ip, bridge_device, bridge_vlan) + LOG.debug("Deleted BGP route for CR-LRP Port %s", ips) + + # Check if there are networks attached to the router, + # and if so delete the needed routes/rules + local_cr_lrp_info = self.ovn_local_cr_lrps.get(cr_lrp_port) + for subnet_cidr in local_cr_lrp_info['subnets_cidr']: + self._withdraw_lrp_port(subnet_cidr, None, cr_lrp_port) + + # 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) + try: + del self.ovn_local_cr_lrps[cr_lrp_port] + except KeyError: + LOG.debug("Gateway port %s already cleanup from the agent.", + cr_lrp_port) + + def _expose_lrp_port(self, ip, lrp, associated_cr_lrp, subnet_datapath, + exposed_ips=[], ovn_ip_rules={}): if not self._expose_tenant_networks: return if not CONF.expose_tenant_networks: # This means CONF.expose_ipv6_gua_tenant_networks is enabled if not driver_utils.is_ipv6_gua(ip): return + cr_lrp_info = self.ovn_local_cr_lrps.get(associated_cr_lrp, {}) + cr_lrp_ips = [ip_address.split('/')[0] + for ip_address in cr_lrp_info.get('ips', [])] + + # this is the router gateway port + if ip.split('/')[0] in cr_lrp_ips: + return + + cr_lrp_datapath = cr_lrp_info.get('provider_datapath') + if not cr_lrp_datapath: + return + + bridge_device = cr_lrp_info.get('bridge_device') + bridge_vlan = cr_lrp_info.get('bridge_vlan') + + # update information needed for the loadbalancers + self.ovn_local_cr_lrps[associated_cr_lrp]['subnets_datapath'].update( + {lrp: subnet_datapath}) + self.ovn_local_cr_lrps[associated_cr_lrp]['subnets_cidr'].append(ip) + self.ovn_local_lrps.update({lrp: associated_cr_lrp}) LOG.debug("Adding IP Rules for network %s on chassis %s", ip, self.chassis) - self.ovn_local_lrps.update({row.logical_port: cr_lrp}) - - cr_lrp_ips = [ip_address.split('/')[0] - for ip_address in cr_lrp_info.get('ips', [])] - bridge_device = self.ovn_local_cr_lrps[cr_lrp].get('bridge_device') - bridge_vlan = self.ovn_local_cr_lrps[cr_lrp].get('bridge_vlan') try: linux_net.add_ip_rule( ip, self.ovn_routing_tables[bridge_device], bridge_device) except agent_exc.InvalidPortIP: LOG.exception("Invalid IP to create a rule for the " - "network router interface port: %s", ip) + "lrp (network router interface) port: %s", ip) return LOG.debug("Added IP Rules for network %s on chassis %s", ip, self.chassis) + if ovn_ip_rules: + ovn_ip_rules.pop(ip, None) LOG.debug("Adding IP Routes for network %s on chassis %s", ip, self.chassis) + # NOTE(ltomasbo): This assumes the provider network can only have + # (at most) 2 subnets, one for IPv4, one for IPv6 ip_version = linux_net.get_ip_version(ip) for cr_lrp_ip in cr_lrp_ips: if linux_net.get_ip_version(cr_lrp_ip) == ip_version: @@ -841,64 +781,40 @@ class OVNBGPDriver(driver_api.AgentDriverBase): # Check if there are VMs on the network # and if so expose the route - ports = self.sb_idl.get_ports_on_datapath(network_port_datapath) + ports = self.sb_idl.get_ports_on_datapath(subnet_datapath) for port in ports: - self._expose_tenant_port(port, ip_version=ip_version) - - @lockutils.synchronized('bgp') - def withdraw_subnet(self, ip, row): - try: - cr_lrp = self.sb_idl.is_router_gateway_on_chassis(row.datapath, - self.chassis) - except ValueError: - # NOTE(ltomasbo): This happens when the router (datapath) gets - # deleted at the same time as subnets are detached from it. - # Usually this will be hit when router is deleted without - # removing its gateway. In that case we don't need to withdraw - # the subnet as it is not exposed, just the cr-lrp which is - # handle in a different event/method (withdraw_ip) - LOG.debug("Router is being deleted, so it's datapath does not " - "exists any more. Checking if port %s belongs to " - "chassis redirect and skip in that case.", - row.logical_port) - cr_lrp = [cr_lrp_name - for cr_lrp_name in self.ovn_local_cr_lrps.keys() - if row.logical_port in cr_lrp_name] - # if cr_lrp exists, this means the lrp port is for the router - # gateway, so there is no need to proceed - if cr_lrp: - LOG.debug("Port %s is related to chassis redirect, so there " - "is no need to do further actions for subnet " - "withdrawal, as this port was not triggering a " - "subnet exposure.", row.logical_port) - return - if not cr_lrp: - return - - cr_lrp_info = self.ovn_local_cr_lrps.get(cr_lrp, {}) - cr_lrp_datapath = cr_lrp_info.get('provider_datapath') - if not cr_lrp_datapath: - return - - self.ovn_local_cr_lrps[cr_lrp]['subnets_datapath'].pop( - row.logical_port, None) + self._expose_tenant_port(port, ip_version=ip_version, + exposed_ips=exposed_ips, + ovn_ip_rules=ovn_ip_rules) + def _withdraw_lrp_port(self, ip, lrp, associated_cr_lrp): if not self._expose_tenant_networks: return if not CONF.expose_tenant_networks: # This means CONF.expose_ipv6_gua_tenant_networks is enabled if not driver_utils.is_ipv6_gua(ip): return + cr_lrp_info = self.ovn_local_cr_lrps.get(associated_cr_lrp, {}) LOG.debug("Deleting IP Rules for network %s on chassis %s", ip, self.chassis) - if row.logical_port in self.ovn_local_lrps.keys(): - self.ovn_local_lrps.pop(row.logical_port) + if lrp: + if lrp in self.ovn_local_lrps.keys(): + self.ovn_local_lrps.pop(lrp) + else: + for subnet_lp in cr_lrp_info['subnets_datapath'].keys(): + if subnet_lp in self.ovn_local_lrps.keys(): + self.ovn_local_lrps.pop(subnet_lp) + break + self.ovn_local_cr_lrps[associated_cr_lrp]['subnets_datapath'].pop( + lrp, None) cr_lrp_ips = [ip_address.split('/')[0] for ip_address in cr_lrp_info.get('ips', [])] - bridge_device = self.ovn_local_cr_lrps[cr_lrp].get('bridge_device') - bridge_vlan = self.ovn_local_cr_lrps[cr_lrp].get('bridge_vlan') + bridge_device = self.ovn_local_cr_lrps[associated_cr_lrp].get( + 'bridge_device') + bridge_vlan = self.ovn_local_cr_lrps[associated_cr_lrp].get( + 'bridge_vlan') linux_net.del_ip_rule(ip, self.ovn_routing_tables[bridge_device], bridge_device) @@ -929,6 +845,52 @@ class OVNBGPDriver(driver_api.AgentDriverBase): # Check if there are VMs on the network # and if so withdraw the routes - vms_on_net = linux_net.get_exposed_ips_on_network( - CONF.bgp_nic, net) - linux_net.delete_exposed_ips(vms_on_net, CONF.bgp_nic) + if net: + vms_on_net = linux_net.get_exposed_ips_on_network( + CONF.bgp_nic, net) + linux_net.delete_exposed_ips(vms_on_net, CONF.bgp_nic) + + @lockutils.synchronized('bgp') + def expose_subnet(self, ip, row): + cr_lrp = self.sb_idl.is_router_gateway_on_chassis( + row.datapath, self.chassis) + subnet_datapath = self.sb_idl.get_port_datapath( + row.options['peer']) + + if not cr_lrp: + return + + self._expose_lrp_port(ip, row.logical_port, cr_lrp, subnet_datapath) + + @lockutils.synchronized('bgp') + def withdraw_subnet(self, ip, row): + try: + cr_lrp = self.sb_idl.is_router_gateway_on_chassis( + row.datapath, self.chassis) + except ValueError: + # NOTE(ltomasbo): This happens when the router (datapath) gets + # deleted at the same time as subnets are detached from it. + # Usually this will be hit when router is deleted without + # removing its gateway. In that case we don't need to withdraw + # the subnet as it is not exposed, just the cr-lrp which is + # handle in a different event/method (withdraw_ip) + LOG.debug("Router is being deleted, so it's datapath does " + "not exists any more. Checking if port %s belongs " + "to chassis redirect and skip in that case.", + row.logical_port) + cr_lrp = [cr_lrp_name + for cr_lrp_name in self.ovn_local_cr_lrps.keys() + if row.logical_port in cr_lrp_name] + # if cr_lrp exists, this means the lrp port is for the router + # gateway, so there is no need to proceed + if cr_lrp: + LOG.debug("Port %s is related to chassis redirect, so " + "there is no need to do further actions for " + "subnet withdrawal, as this port was not " + "triggering a subnet exposure.", + row.logical_port) + return + if not cr_lrp: + return + + self._withdraw_lrp_port(ip, row.logical_port, cr_lrp) diff --git a/ovn_bgp_agent/drivers/openstack/utils/ovn.py b/ovn_bgp_agent/drivers/openstack/utils/ovn.py index a502b668..1fdcca93 100644 --- a/ovn_bgp_agent/drivers/openstack/utils/ovn.py +++ b/ovn_bgp_agent/drivers/openstack/utils/ovn.py @@ -114,7 +114,7 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend): super(OvsdbSbOvnIdl, self).__init__(connection) self.idl._session.reconnect.set_probe_interval(60000) - def _get_port_by_name(self, port): + def get_port_by_name(self, port): cmd = self.db_find_rows('Port_Binding', ('logical_port', '=', port)) port_info = cmd.execute(check_error=True) return port_info[0] if port_info else [] @@ -129,6 +129,11 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend): ('datapath', '=', datapath)) return cmd.execute(check_error=True) + def get_ports_by_type(self, port_type): + cmd = self.db_find_rows('Port_Binding', + ('type', '=', port_type)) + return cmd.execute(check_error=True) + def is_provider_network(self, datapath): cmd = self.db_find_rows('Port_Binding', ('datapath', '=', datapath), ('type', '=', @@ -145,7 +150,7 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend): return None, None def is_port_on_chassis(self, port_name, chassis): - port_info = self._get_port_by_name(port_name) + port_info = self.get_port_by_name(port_name) try: return (port_info and port_info.chassis[0].name == chassis) @@ -154,7 +159,7 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend): return False def is_port_deleted(self, port_name): - return False if self._get_port_by_name(port_name) else True + return False if self.get_port_by_name(port_name) else True def get_ports_on_chassis(self, chassis): rows = self.db_list_rows('Port_Binding').execute(check_error=True) @@ -171,7 +176,7 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend): def get_cr_lrp_nat_addresses_info(self, cr_lrp_port_name, chassis, sb_idl): # NOTE: Assuming logical_port format is "cr-lrp-XXXX" patch_port_name = cr_lrp_port_name.split("cr-lrp-")[1] - patch_port_row = self._get_port_by_name(patch_port_name) + patch_port_row = self.get_port_by_name(patch_port_name) if not patch_port_row: return [], None ips = [] @@ -185,11 +190,13 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend): def get_provider_datapath_from_cr_lrp(self, cr_lrp): if cr_lrp.startswith('cr-lrp'): provider_port = cr_lrp.split("cr-lrp-")[1] - port_info = self._get_port_by_name(provider_port) - if port_info: - return port_info.datapath + return self.get_port_datapath(provider_port) return None + def get_datapath_from_port_peer(self, port): + peer_name = port.options['peer'] + return self.get_port_datapath(peer_name) + def get_network_name_and_tag(self, datapath, bridge_mappings): for row in self.get_ports_on_datapath( datapath, constants.OVN_LOCALNET_VIF_PORT_TYPE): @@ -225,14 +232,24 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend): return self.get_ports_on_datapath( datapath, constants.OVN_PATCH_VIF_PORT_TYPE) + def get_lrp_ports_on_provider(self): + provider_lrp_ports = [] + lrp_ports = self.get_ports_by_type(constants.OVN_PATCH_VIF_PORT_TYPE) + for lrp_port in lrp_ports: + if lrp_port.logical_port.startswith( + constants.OVN_LRP_PORT_NAME_PREFIX): + continue + if self.is_provider_network(lrp_port.datapath): + provider_lrp_ports.append(lrp_port) + def get_port_datapath(self, port_name): - port_info = self._get_port_by_name(port_name) + port_info = self.get_port_by_name(port_name) if port_info: return port_info.datapath def get_ip_from_port_peer(self, port): peer_name = port.options['peer'] - peer_port = self._get_port_by_name(peer_name) + peer_port = self.get_port_by_name(peer_name) try: return peer_port.mac[0].split(' ')[1] except AttributeError: @@ -245,7 +262,7 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend): elif port_name.startswith(constants.OVN_LRP_PORT_NAME_PREFIX): port_name = port_name.split(constants.OVN_LRP_PORT_NAME_PREFIX)[1] - port = self._get_port_by_name(port_name) + port = self.get_port_by_name(port_name) return self.get_evpn_info(port) def get_evpn_info(self, port): @@ -263,7 +280,7 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend): return {} def get_port_if_local_chassis(self, port_name, chassis): - port = self._get_port_by_name(port_name) + port = self.get_port_by_name(port_name) if port.chassis[0].name == chassis: return port diff --git a/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py b/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py index d9e3ff75..c403a40c 100644 --- a/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py +++ b/ovn_bgp_agent/drivers/openstack/watchers/bgp_watcher.py @@ -250,7 +250,7 @@ class TenantPortDeletedEvent(base_watcher.PortBindingChassisEvent): return with _SYNC_STATE_LOCK.read_lock(): ips = row.mac[0].split(' ')[1:] - self.agent.withdraw_remote_ip(ips, row) + self.agent.withdraw_remote_ip(ips, row, old.chassis) class OVNLBTenantPortEvent(base_watcher.PortBindingChassisEvent): 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 2a447937..b6ca8b2e 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 @@ -63,13 +63,14 @@ class TestOVNBGPDriver(test_base.TestCase): self.lrp0 = 'lrp-fake-logical-port' self.bgp_driver.ovn_local_cr_lrps = { self.cr_lrp0: {'provider_datapath': 'fake-provider-dp', + 'router_datapath': 'fake-router-dp', 'ips': [self.fip], 'subnets_datapath': {self.lrp0: 'fake-lrp-dp'}, 'subnets_cidr': ['192.168.1.1/24'], 'ovn_lbs': [], 'bridge_device': self.bridge, 'bridge_vlan': None}, - self.cr_lrp1: {'provider_datapath': 'fake-provider-dp'}} + self.cr_lrp1: {'provider_datapath': 'fake-provider-dp2'}} # Mock pyroute2.NDB context manager object self.mock_ndb = mock.patch.object(linux_net.pyroute2, 'NDB').start() @@ -487,19 +488,23 @@ 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, 'get_ip_version') - def test__ensure_network_exposed(self, mock_ip_version, mock_add_rule, - mock_add_route, mock_add_ips_dev): + def test__process_lrp_port(self, mock_ip_version, mock_add_rule, + mock_add_route, mock_add_ips_dev): mock_ip_version.return_value = constants.IP_VERSION_4 gateway = {} gateway['ips'] = ['{}/32'.format(self.fip), '2003::1234:abcd:ffff:c0a8:102/128'] gateway['provider_datapath'] = 'bc6780f4-9510-4270-b4d2-b8d5c6802713' + gateway['subnets_datapath'] = {} + gateway['subnets_cidr'] = [] + gateway['bridge_device'] = self.bridge + gateway['bridge_vlan'] = 10 self.bgp_driver.ovn_local_cr_lrps = {'gateway_port': gateway} router_port = fakes.create_object({ - 'name': 'fake-router-port', + 'chassis': [], 'mac': ['{} {}/32'.format(self.mac, self.ipv4)], - 'logical_port': 'fake-logical-port', + 'logical_port': 'lrp-fake-logical-port', 'options': {'peer': 'fake-peer'}}) mock_get_bridge = mock.patch.object( @@ -543,7 +548,7 @@ class TestOVNBGPDriver(test_base.TestCase): dp_port2, dp_port3, dp_port4] - self.bgp_driver._ensure_network_exposed(router_port, 'gateway_port') + self.bgp_driver._process_lrp_port(router_port, 'gateway_port') # Assert that the add methods were called mock_add_rule.assert_called_once_with( @@ -561,9 +566,9 @@ class TestOVNBGPDriver(test_base.TestCase): @mock.patch.object(linux_net, 'add_ip_rule') @mock.patch.object(linux_net, 'get_ip_version') @mock.patch.object(driver_utils, 'is_ipv6_gua') - def test__ensure_network_exposed_gua(self, mock_ipv6_gua, mock_ip_version, - mock_add_rule, mock_add_route, - mock_add_ips_dev): + def test__process_lrp_port_gua(self, mock_ipv6_gua, mock_ip_version, + mock_add_rule, mock_add_route, + mock_add_ips_dev): CONF.set_override('expose_tenant_networks', False) self.addCleanup(CONF.clear_override, 'expose_tenant_networks') CONF.set_override('expose_ipv6_gua_tenant_networks', True) @@ -575,12 +580,16 @@ class TestOVNBGPDriver(test_base.TestCase): gateway['ips'] = ['{}/32'.format(self.fip), '2003::1234:abcd:ffff:c0a8:102/128'] gateway['provider_datapath'] = 'bc6780f4-9510-4270-b4d2-b8d5c6802713' + gateway['subnets_datapath'] = {} + gateway['subnets_cidr'] = [] + gateway['bridge_device'] = self.bridge + gateway['bridge_vlan'] = 10 self.bgp_driver.ovn_local_cr_lrps = {'gateway_port': gateway} router_port = fakes.create_object({ - 'name': 'fake-router-port', + 'chassis': [], 'mac': ['{} {}/128'.format(self.mac, self.ipv6)], - 'logical_port': 'fake-logical-port', + 'logical_port': 'lrp-fake-logical-port', 'options': {'peer': 'fake-peer'}}) mock_get_bridge = mock.patch.object( @@ -625,7 +634,7 @@ class TestOVNBGPDriver(test_base.TestCase): dp_port2, dp_port3, dp_port4] - self.bgp_driver._ensure_network_exposed(router_port, 'gateway_port') + self.bgp_driver._process_lrp_port(router_port, 'gateway_port') # Assert that the add methods were called mock_ipv6_gua.assert_called_once_with('{}/128'.format(self.ipv6)) @@ -644,7 +653,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(driver_utils, 'is_ipv6_gua') - def test__ensure_network_exposed_not_gua( + def test__process_lrp_port_not_gua( self, mock_ipv6_gua, mock_add_rule, mock_add_route, mock_add_ips_dev): CONF.set_override('expose_tenant_networks', False) @@ -656,20 +665,24 @@ class TestOVNBGPDriver(test_base.TestCase): gateway['ips'] = ['{}/32'.format(self.fip), '2003::1234:abcd:ffff:c0a8:102/128'] gateway['provider_datapath'] = 'bc6780f4-9510-4270-b4d2-b8d5c6802713' + gateway['subnets_datapath'] = {} + gateway['subnets_cidr'] = [] + gateway['bridge_device'] = self.bridge + gateway['bridge_vlan'] = 10 self.bgp_driver.ovn_local_cr_lrps = {'gateway_port': gateway} router_port = fakes.create_object({ - 'name': 'fake-router-port', + 'chassis': [], 'mac': ['{} fdab:4ad8:e8fb:0:f816:3eff:fec6:469c/128'.format( self.mac)], - 'logical_port': 'fake-logical-port', + 'logical_port': 'lrp-fake-logical-port', 'options': {'peer': 'fake-peer'}}) mock_get_bridge = mock.patch.object( self.bgp_driver, '_get_bridge_for_datapath').start() mock_get_bridge.return_value = (self.bridge, 10) - self.bgp_driver._ensure_network_exposed(router_port, 'gateway_port') + self.bgp_driver._process_lrp_port(router_port, 'gateway_port') # Assert that the add methods were called mock_ipv6_gua.assert_called_once_with( @@ -681,19 +694,23 @@ 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, 'get_ip_version') - def test__ensure_network_exposed_invalid_ip( + def test__process_lrp_port_invalid_ip( self, mock_ip_version, mock_add_rule, mock_add_route): mock_ip_version.return_value = constants.IP_VERSION_4 gateway = {} gateway['ips'] = ['{}/32'.format(self.fip), '2003::1234:abcd:ffff:c0a8:102/128'] gateway['provider_datapath'] = 'bc6780f4-9510-4270-b4d2-b8d5c6802713' + gateway['subnets_datapath'] = {} + gateway['subnets_cidr'] = [] + gateway['bridge_device'] = self.bridge + gateway['bridge_vlan'] = 10 self.bgp_driver.ovn_local_cr_lrps = {'gateway_port': gateway} router_port = fakes.create_object({ - 'name': 'fake-router-port', + 'chassis': [], 'mac': ['{} {}/32'.format(self.mac, self.ipv4)], - 'logical_port': 'fake-logical-port', + 'logical_port': 'lrp-fake-logical-port', 'options': {'peer': 'fake-peer'}}) mock_get_bridge = mock.patch.object( @@ -703,40 +720,13 @@ class TestOVNBGPDriver(test_base.TestCase): # Raise an exception on add_ip_rule() mock_add_rule.side_effect = agent_exc.InvalidPortIP(ip=self.ipv4) - self.bgp_driver._ensure_network_exposed(router_port, 'gateway_port') + self.bgp_driver._process_lrp_port(router_port, 'gateway_port') mock_add_rule.assert_called_once_with( '{}/32'.format(self.ipv4), 'fake-table', self.bridge) # Assert that add_ip_route() was not called mock_add_route.assert_not_called() - @mock.patch.object(linux_net, 'del_ip_route') - @mock.patch.object(linux_net, 'del_ip_rule') - @mock.patch.object(linux_net, 'get_ip_version') - def test__remove_network_exposed( - self, mock_ip_version, mock_del_rule, mock_del_route): - mock_ip_version.return_value = constants.IP_VERSION_4 - lrp = 'fake-lrp' - self.bgp_driver.ovn_local_lrps = {lrp: 'fake-subnet-datapath'} - gateway = { - 'provider_datapath': 'bc6780f4-9510-4270-b4d2-b8d5c6802713', - 'subnets_datapath': {lrp: 'fake-subnet-datapath'}, - 'bridge_device': self.bridge, - 'bridge_vlan': 10 - } - gateway['ips'] = ['{}/32'.format(self.fip), - '2003::1234:abcd:ffff:c0a8:102/128'] - subnet_cidr = "192.168.1.1/24" - - self.bgp_driver._remove_network_exposed(subnet_cidr, gateway) - - # Assert that the del methods were called - mock_del_rule.assert_called_once_with( - subnet_cidr, 'fake-table', self.bridge) - mock_del_route.assert_called_once_with( - mock.ANY, subnet_cidr.split('/')[0], 'fake-table', self.bridge, - vlan=10, mask='24', via=self.fip) - def test__get_bridge_for_datapath(self): self.sb_idl.get_network_name_and_tag.return_value = ( 'fake-network', [10]) @@ -906,7 +896,6 @@ class TestOVNBGPDriver(test_base.TestCase): self.bridge, vlan=10)] mock_add_route.assert_has_calls(expected_calls) - @mock.patch.object(driver_utils, 'parse_vip_from_lb_table') @mock.patch.object(linux_net, 'add_ndp_proxy') @mock.patch.object(linux_net, 'get_ip_version') @mock.patch.object(linux_net, 'add_ip_route') @@ -914,7 +903,7 @@ class TestOVNBGPDriver(test_base.TestCase): @mock.patch.object(linux_net, 'add_ips_to_dev') def test_expose_ip_chassisredirect_port( self, mock_add_ip_dev, mock_add_rule, mock_add_route, - mock_ip_version, mock_ndp_proxy, mock_parse_vip): + mock_ip_version, mock_ndp_proxy): self.sb_idl.get_provider_datapath_from_cr_lrp.return_value = ( 'fake-provider-dp') mock_get_bridge = mock.patch.object( @@ -936,8 +925,8 @@ class TestOVNBGPDriver(test_base.TestCase): self.sb_idl.get_port_datapath.return_value = 'fake-lrp-dp' - mock_ensure_net_exposed = mock.patch.object( - self.bgp_driver, '_ensure_network_exposed').start() + mock_process_lrp_port = mock.patch.object( + self.bgp_driver, '_process_lrp_port').start() mock_ip_version.side_effect = (constants.IP_VERSION_4, constants.IP_VERSION_6) @@ -958,9 +947,8 @@ class TestOVNBGPDriver(test_base.TestCase): '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_expose_ovn_lb_on_provider = mock.patch.object( - self.bgp_driver, '_expose_ovn_lb_on_provider').start() - mock_parse_vip.return_value = ovn_lb_vip + mock_process_ovn_lb = mock.patch.object( + self.bgp_driver, '_process_ovn_lb').start() ips = [self.ipv4, self.ipv6] self.bgp_driver.expose_ip(ips, row) @@ -983,12 +971,14 @@ class TestOVNBGPDriver(test_base.TestCase): mock_ndp_proxy.assert_called_once_with(self.ipv6, self.bridge, 10) - mock_ensure_net_exposed.assert_called_once_with( - lrp0, self.cr_lrp0) + expected_calls = [mock.call(lrp0, self.cr_lrp0), + mock.call(lrp1, self.cr_lrp0), + mock.call(lrp2, self.cr_lrp0)] + mock_process_lrp_port.assert_has_calls(expected_calls) - mock_parse_vip.assert_called_once_with(ovn_lb_vip_port) - mock_expose_ovn_lb_on_provider.assert_called_once_with( - 'ovn_lb1', ovn_lb_vip, row.logical_port) + 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.patch.object(linux_net, 'del_ip_route') @mock.patch.object(linux_net, 'del_ip_rule') @@ -1111,8 +1101,8 @@ class TestOVNBGPDriver(test_base.TestCase): def test_withdraw_ip_chassisredirect_port( self, mock_del_ip_dev, mock_del_rule, mock_del_route, mock_ip_version, mock_ndp_proxy): - mock_remove_net_exposed = mock.patch.object( - self.bgp_driver, '_remove_network_exposed').start() + mock_withdraw_lrp_port = mock.patch.object( + self.bgp_driver, '_withdraw_lrp_port').start() mock_ip_version.side_effect = (constants.IP_VERSION_4, constants.IP_VERSION_6, @@ -1146,13 +1136,8 @@ class TestOVNBGPDriver(test_base.TestCase): mock_ndp_proxy.assert_called_once_with(self.ipv6, self.bridge, None) - mock_remove_net_exposed.assert_called_once_with( - '192.168.1.1/24', - {'provider_datapath': 'fake-provider-dp', 'ips': [self.fip], - 'subnets_datapath': {self.lrp0: 'fake-lrp-dp'}, - 'subnets_cidr': ['192.168.1.1/24'], - 'ovn_lbs': [], 'bridge_vlan': None, - 'bridge_device': self.bridge}) + mock_withdraw_lrp_port.assert_called_once_with( + '192.168.1.1/24', None, self.cr_lrp0) @mock.patch.object(linux_net, 'add_ips_to_dev') def test_expose_remote_ip(self, mock_add_ip_dev): @@ -1312,19 +1297,80 @@ class TestOVNBGPDriver(test_base.TestCase): mock_del_ip_dev.assert_not_called() + @mock.patch.object(linux_net, 'add_ndp_proxy') + @mock.patch.object(linux_net, 'get_ip_version') + def test__expose_cr_lrp_port(self, mock_ip_version, mock_ndp_proxy): + mock_expose_provider_port = mock.patch.object( + 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() + 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] + + 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) + + 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) + 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.patch.object(linux_net, 'del_ndp_proxy') + @mock.patch.object(linux_net, 'get_ip_version') + def test__withdraw_cr_lrp_port(self, mock_ip_version, mock_ndp_proxy): + mock_withdraw_provider_port = mock.patch.object( + self.bgp_driver, '_withdraw_provider_port').start() + 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 = 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_lbs': [ovn_lb]} + self.bgp_driver.ovn_local_cr_lrps = {'gateway_port': gateway} + + self.bgp_driver._withdraw_cr_lrp_port( + ips, self.mac, self.bridge, 10, + provider_datapath='fake-provider-dp', cr_lrp_port='gateway_port') + + 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) + mock_ndp_proxy.assert_called_once_with(self.ipv6, self.bridge, 10) + 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') + @mock.patch.object(linux_net, 'add_ip_route') @mock.patch.object(linux_net, 'add_ip_rule') @mock.patch.object(linux_net, 'get_ip_version') - def test_expose_subnet( + def test__expose_lrp_port( self, mock_ip_version, mock_add_rule, mock_add_route): mock_ip_version.return_value = constants.IP_VERSION_4 - self.sb_idl.is_router_gateway_on_chassis.return_value = self.cr_lrp0 - self.sb_idl.get_port_datapath.return_value = 'fake-port-dp' - row = fakes.create_object({ - 'name': 'fake-row', - 'logical_port': self.cr_lrp0, - 'datapath': 'fake-dp', - 'options': {'peer': 'fake-peer'}}) dp_port0 = fakes.create_object({ 'name': 'fake-port-dp0', 'type': constants.OVN_VM_VIF_PORT_TYPE, @@ -1345,7 +1391,8 @@ class TestOVNBGPDriver(test_base.TestCase): mock_expose_tenant_port = mock.patch.object( self.bgp_driver, '_expose_tenant_port').start() - self.bgp_driver.expose_subnet('{}/32'.format(self.ipv4), row) + self.bgp_driver._expose_lrp_port( + '{}/32'.format(self.ipv4), self.lrp0, self.cr_lrp0, 'fake-lrp-dp') mock_add_rule.assert_called_once_with( '{}/32'.format(self.ipv4), 'fake-table', self.bridge) @@ -1353,16 +1400,55 @@ class TestOVNBGPDriver(test_base.TestCase): mock.ANY, self.ipv4, 'fake-table', self.bridge, vlan=None, mask='32', via=self.fip) expected_calls = [ - mock.call(dp_port0, ip_version=constants.IP_VERSION_4), - mock.call(dp_port1, ip_version=constants.IP_VERSION_4), - mock.call(dp_port2, ip_version=constants.IP_VERSION_4)] + mock.call(dp_port0, ip_version=constants.IP_VERSION_4, + exposed_ips=[], ovn_ip_rules={}), + mock.call(dp_port1, ip_version=constants.IP_VERSION_4, + exposed_ips=[], ovn_ip_rules={}), + mock.call(dp_port2, ip_version=constants.IP_VERSION_4, + exposed_ips=[], ovn_ip_rules={})] mock_expose_tenant_port.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, 'get_ip_version') + def test__expose_lrp_port_invalid_ip( + self, mock_ip_version, mock_add_rule, mock_add_route): + mock_ip_version.return_value = constants.IP_VERSION_4 + dp_port0 = fakes.create_object({ + 'name': 'fake-port-dp0', + 'type': constants.OVN_VM_VIF_PORT_TYPE, + 'mac': ['aa:bb:cc:dd:ee:ee 192.168.1.10 192.168.1.11'], + 'chassis': 'fake-chassis1'}) + dp_port1 = fakes.create_object({ + 'name': 'fake-port-dp1', + 'type': 'fake-type', + 'mac': ['aa:bb:cc:dd:ee:ee 192.168.1.12 192.168.1.13'], + 'chassis': 'fake-chassis2'}) + dp_port2 = fakes.create_object({ + 'name': 'fake-port-dp1', + 'type': constants.OVN_VM_VIF_PORT_TYPE, + 'mac': [], + 'chassis': 'fake-chassis2'}) + self.sb_idl.get_ports_on_datapath.return_value = [dp_port0, dp_port1, + dp_port2] + mock_expose_tenant_port = mock.patch.object( + self.bgp_driver, '_expose_tenant_port').start() + + mock_add_rule.side_effect = agent_exc.InvalidPortIP(ip=self.ipv4) + + self.bgp_driver._expose_lrp_port( + '{}/32'.format(self.ipv4), self.lrp0, self.cr_lrp0, 'fake-lrp-dp') + + mock_add_rule.assert_called_once_with( + '{}/32'.format(self.ipv4), 'fake-table', self.bridge) + mock_add_route.assert_not_called() + mock_expose_tenant_port.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') @mock.patch.object(driver_utils, 'is_ipv6_gua') - def test_expose_subnet_gua( + def test__expose_lrp_port_gua( self, mock_ipv6_gua, mock_ip_version, mock_add_rule, mock_add_route): CONF.set_override('expose_tenant_networks', False) @@ -1371,13 +1457,6 @@ class TestOVNBGPDriver(test_base.TestCase): self.addCleanup(CONF.clear_override, 'expose_ipv6_gua_tenant_networks') mock_ipv6_gua.return_value = True mock_ip_version.return_value = constants.IP_VERSION_6 - self.sb_idl.is_router_gateway_on_chassis.return_value = self.cr_lrp0 - self.sb_idl.get_port_datapath.return_value = 'fake-port-dp' - row = fakes.create_object({ - 'name': 'fake-row', - 'logical_port': self.cr_lrp0, - 'datapath': 'fake-dp', - 'options': {'peer': 'fake-peer'}}) dp_port0 = fakes.create_object({ 'name': 'fake-port-dp0', 'type': constants.OVN_VM_VIF_PORT_TYPE, @@ -1398,7 +1477,8 @@ class TestOVNBGPDriver(test_base.TestCase): mock_expose_tenant_port = mock.patch.object( self.bgp_driver, '_expose_tenant_port').start() - self.bgp_driver.expose_subnet('{}/128'.format(self.ipv6), row) + self.bgp_driver._expose_lrp_port( + '{}/128'.format(self.ipv6), self.lrp0, self.cr_lrp0, 'fake-lrp-dp') mock_add_rule.assert_called_once_with( '{}/128'.format(self.ipv6), 'fake-table', self.bridge) @@ -1406,28 +1486,24 @@ class TestOVNBGPDriver(test_base.TestCase): mock.ANY, self.ipv6, 'fake-table', self.bridge, vlan=None, mask='128', via=self.fip) expected_calls = [ - mock.call(dp_port0, ip_version=constants.IP_VERSION_6), - mock.call(dp_port1, ip_version=constants.IP_VERSION_6), - mock.call(dp_port2, ip_version=constants.IP_VERSION_6)] + mock.call(dp_port0, ip_version=constants.IP_VERSION_6, + exposed_ips=[], ovn_ip_rules={}), + mock.call(dp_port1, ip_version=constants.IP_VERSION_6, + exposed_ips=[], ovn_ip_rules={}), + mock.call(dp_port2, ip_version=constants.IP_VERSION_6, + exposed_ips=[], ovn_ip_rules={})] mock_expose_tenant_port.assert_has_calls(expected_calls) @mock.patch.object(linux_net, 'add_ip_route') @mock.patch.object(linux_net, 'add_ip_rule') @mock.patch.object(driver_utils, 'is_ipv6_gua') - def test_expose_subnet_no_gua( + def test__expose_lrp_port_no_gua( self, mock_ipv6_gua, mock_add_rule, mock_add_route): CONF.set_override('expose_tenant_networks', False) self.addCleanup(CONF.clear_override, 'expose_tenant_networks') CONF.set_override('expose_ipv6_gua_tenant_networks', True) self.addCleanup(CONF.clear_override, 'expose_ipv6_gua_tenant_networks') mock_ipv6_gua.return_value = False - self.sb_idl.is_router_gateway_on_chassis.return_value = self.cr_lrp0 - self.sb_idl.get_port_datapath.return_value = 'fake-port-dp' - row = fakes.create_object({ - 'name': 'fake-row', - 'logical_port': self.cr_lrp0, - 'datapath': 'fake-dp', - 'options': {'peer': 'fake-peer'}}) dp_port0 = fakes.create_object({ 'name': 'fake-port-dp0', 'type': constants.OVN_VM_VIF_PORT_TYPE, @@ -1448,31 +1524,101 @@ class TestOVNBGPDriver(test_base.TestCase): mock_expose_tenant_port = mock.patch.object( self.bgp_driver, '_expose_tenant_port').start() - self.bgp_driver.expose_subnet( - 'fdab:4ad8:e8fb:0:f816:3eff:fec6:469c/128', row) + self.bgp_driver._expose_lrp_port( + 'fdab:4ad8:e8fb:0:f816:3eff:fec6:469c/128', self.lrp0, + self.cr_lrp0, 'fake-lrp-dp') mock_add_rule.assert_not_called() mock_add_route.assert_not_called() mock_expose_tenant_port.assert_not_called() + def test_expose_subnet(self): + self.sb_idl.is_router_gateway_on_chassis.return_value = self.cr_lrp0 + self.sb_idl.get_port_datapath.return_value = 'fake-port-dp' + row = fakes.create_object({ + 'name': 'fake-row', + 'logical_port': 'subnet_port', + 'datapath': 'fake-dp', + 'options': {'peer': 'fake-peer'}}) + + mock_expose_lrp_port = mock.patch.object( + self.bgp_driver, '_expose_lrp_port').start() + + self.bgp_driver.expose_subnet('fake-ip', row) + + mock_expose_lrp_port.assert_called_once_with( + 'fake-ip', row.logical_port, self.cr_lrp0, 'fake-port-dp') + + def test_expose_subnet_no_cr_lrp(self): + self.sb_idl.is_router_gateway_on_chassis.return_value = None + self.sb_idl.get_port_datapath.return_value = 'fake-port-dp' + row = fakes.create_object({ + 'name': 'fake-row', + 'logical_port': 'subnet_port', + 'datapath': 'fake-dp', + 'options': {'peer': 'fake-peer'}}) + + mock_expose_lrp_port = mock.patch.object( + self.bgp_driver, '_expose_lrp_port').start() + + self.bgp_driver.expose_subnet('fake-ip', row) + + mock_expose_lrp_port.assert_not_called() + + def test_withdraw_subnet(self): + row = fakes.create_object({ + 'name': 'fake-row', + 'logical_port': 'subnet_port', + 'datapath': 'fake-dp'}) + self.sb_idl.is_router_gateway_on_chassis.return_value = self.cr_lrp0 + mock_withdraw_lrp_port = mock.patch.object( + self.bgp_driver, '_withdraw_lrp_port').start() + + self.bgp_driver.withdraw_subnet('{}/32'.format(self.ipv4), row) + + mock_withdraw_lrp_port.assert_called_once_with( + '{}/32'.format(self.ipv4), row.logical_port, self.cr_lrp0) + + def test_withdraw_subnet_no_cr_lrp(self): + row = fakes.create_object({ + 'name': 'fake-row', + 'logical_port': 'subnet_port', + 'datapath': 'fake-dp'}) + self.sb_idl.is_router_gateway_on_chassis.return_value = None + mock_withdraw_lrp_port = mock.patch.object( + self.bgp_driver, '_withdraw_lrp_port').start() + + self.bgp_driver.withdraw_subnet('{}/32'.format(self.ipv4), row) + + mock_withdraw_lrp_port.assert_not_called() + + def test_withdraw_subnet_no_value_error(self): + row = fakes.create_object({ + 'name': 'fake-row', + 'logical_port': 'fake-logical-port', # to match the cr-lrp name + 'datapath': 'fake-dp'}) + self.sb_idl.is_router_gateway_on_chassis.side_effect = ValueError + mock_withdraw_lrp_port = mock.patch.object( + self.bgp_driver, '_withdraw_lrp_port').start() + + self.bgp_driver.withdraw_subnet('{}/32'.format(self.ipv4), row) + + mock_withdraw_lrp_port.assert_not_called() + @mock.patch.object(linux_net, 'get_exposed_ips_on_network') @mock.patch.object(linux_net, 'delete_exposed_ips') @mock.patch.object(linux_net, 'del_ip_route') @mock.patch.object(linux_net, 'del_ip_rule') @mock.patch.object(linux_net, 'get_ip_version') - def test_withdraw_subnet( + def test__withdraw_lrp_port( self, mock_ip_version, mock_del_rule, mock_del_route, mock_del_exposed_ips, mock_get_exposed_ips): mock_ip_version.return_value = constants.IP_VERSION_4 mock_get_exposed_ips.return_value = [self.ipv4] - self.sb_idl.is_router_gateway_on_chassis.return_value = self.cr_lrp0 self.bgp_driver.ovn_local_lrps = {self.lrp0: self.cr_lrp0} - row = fakes.create_object({ - 'name': 'fake-row', - 'logical_port': self.cr_lrp0, - 'datapath': 'fake-dp'}) - self.bgp_driver.withdraw_subnet('{}/32'.format(self.ipv4), row) + self.bgp_driver._withdraw_lrp_port( + '{}/32'.format(self.ipv4), self.lrp0, self.cr_lrp0) mock_del_rule.assert_called_once_with( '{}/32'.format(self.ipv4), 'fake-table', self.bridge) @@ -1488,7 +1634,7 @@ class TestOVNBGPDriver(test_base.TestCase): @mock.patch.object(linux_net, 'del_ip_rule') @mock.patch.object(linux_net, 'get_ip_version') @mock.patch.object(driver_utils, 'is_ipv6_gua') - def test_withdraw_subnet_gua( + def test__withdraw_lrp_port_gua( self, mock_ipv6_gua, mock_ip_version, mock_del_rule, mock_del_route, mock_del_exposed_ips, mock_get_exposed_ips): CONF.set_override('expose_tenant_networks', False) @@ -1498,14 +1644,10 @@ class TestOVNBGPDriver(test_base.TestCase): mock_ipv6_gua.return_value = True mock_ip_version.return_value = constants.IP_VERSION_6 mock_get_exposed_ips.return_value = [self.ipv6] - self.sb_idl.is_router_gateway_on_chassis.return_value = self.cr_lrp0 self.bgp_driver.ovn_local_lrps = {self.lrp0: self.cr_lrp0} - row = fakes.create_object({ - 'name': 'fake-row', - 'logical_port': self.cr_lrp0, - 'datapath': 'fake-dp'}) - self.bgp_driver.withdraw_subnet('{}/128'.format(self.ipv6), row) + self.bgp_driver._withdraw_lrp_port( + '{}/128'.format(self.ipv6), self.lrp0, self.cr_lrp0) mock_del_rule.assert_called_once_with( '{}/128'.format(self.ipv6), 'fake-table', self.bridge) @@ -1520,7 +1662,7 @@ class TestOVNBGPDriver(test_base.TestCase): @mock.patch.object(linux_net, 'del_ip_route') @mock.patch.object(linux_net, 'del_ip_rule') @mock.patch.object(driver_utils, 'is_ipv6_gua') - def test_withdraw_subnet_no_gua( + def test__withdraw_lrp_port_no_gua( self, mock_ipv6_gua, mock_del_rule, mock_del_route, mock_del_exposed_ips, mock_get_exposed_ips): CONF.set_override('expose_tenant_networks', False) @@ -1529,15 +1671,11 @@ class TestOVNBGPDriver(test_base.TestCase): self.addCleanup(CONF.clear_override, 'expose_ipv6_gua_tenant_networks') mock_ipv6_gua.return_value = False mock_get_exposed_ips.return_value = [self.ipv6] - self.sb_idl.is_router_gateway_on_chassis.return_value = self.cr_lrp0 self.bgp_driver.ovn_local_lrps = {self.lrp0: self.cr_lrp0} - row = fakes.create_object({ - 'name': 'fake-row', - 'logical_port': self.cr_lrp0, - 'datapath': 'fake-dp'}) - self.bgp_driver.withdraw_subnet( - 'fdab:4ad8:e8fb:0:f816:3eff:fec6:469c/128', row) + self.bgp_driver._withdraw_lrp_port( + 'fdab:4ad8:e8fb:0:f816:3eff:fec6:469c/128', self.lrp0, + self.cr_lrp0) mock_del_rule.assert_not_called() mock_del_route.assert_not_called() 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 64800cd5..e887812d 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 @@ -40,21 +40,21 @@ class TestOvsdbSbOvnIdl(test_base.TestCase): self.sb_idl.db_find_rows = mock.Mock() self.sb_idl.db_list_rows = mock.Mock() - def test__get_port_by_name(self): + def test_get_port_by_name(self): fake_p_info = 'fake-port-info' port = 'fake-port' self.sb_idl.db_find_rows.return_value.execute.return_value = [ fake_p_info] - ret = self.sb_idl._get_port_by_name(port) + ret = self.sb_idl.get_port_by_name(port) self.assertEqual(fake_p_info, ret) self.sb_idl.db_find_rows.assert_called_once_with( 'Port_Binding', ('logical_port', '=', port)) - def test__get_port_by_name_empty(self): + def test_get_port_by_name_empty(self): port = 'fake-port' self.sb_idl.db_find_rows.return_value.execute.return_value = [] - ret = self.sb_idl._get_port_by_name(port) + ret = self.sb_idl.get_port_by_name(port) self.assertEqual([], ret) self.sb_idl.db_find_rows.assert_called_once_with( @@ -81,6 +81,17 @@ class TestOvsdbSbOvnIdl(test_base.TestCase): self.sb_idl.db_find_rows.assert_called_once_with( 'Port_Binding', ('datapath', '=', dp), ('type', '=', p_type)) + def test_get_ports_by_type(self): + fake_p_info = 'fake-port-info' + port_type = 'fake-type' + self.sb_idl.db_find_rows.return_value.execute.return_value = [ + fake_p_info] + ret = self.sb_idl.get_ports_by_type(port_type) + + self.assertEqual([fake_p_info], ret) + self.sb_idl.db_find_rows.assert_called_once_with( + 'Port_Binding', ('type', '=', port_type)) + def test_is_provider_network(self): dp = 'fake-datapath' self.sb_idl.db_find_rows.return_value.execute.return_value = ['fake'] @@ -124,7 +135,7 @@ class TestOvsdbSbOvnIdl(test_base.TestCase): def _test_is_port_on_chassis(self, should_match=True): chassis_name = 'fake-chassis' - with mock.patch.object(self.sb_idl, '_get_port_by_name') as mock_p: + with mock.patch.object(self.sb_idl, 'get_port_by_name') as mock_p: ch = fakes.create_object({'name': chassis_name}) mock_p.return_value = fakes.create_object( {'type': constants.OVN_VM_VIF_PORT_TYPE, @@ -143,14 +154,14 @@ class TestOvsdbSbOvnIdl(test_base.TestCase): self._test_is_port_on_chassis(should_match=False) def test_is_port_on_chassis_port_not_found(self): - with mock.patch.object(self.sb_idl, '_get_port_by_name') as mock_p: + with mock.patch.object(self.sb_idl, 'get_port_by_name') as mock_p: mock_p.return_value = [] self.assertFalse(self.sb_idl.is_port_on_chassis( 'fake-port', 'fake-chassis')) def _test_is_port_deleted(self, port_exist=True): ret_value = mock.Mock() if port_exist else [] - with mock.patch.object(self.sb_idl, '_get_port_by_name') as mock_p: + with mock.patch.object(self.sb_idl, 'get_port_by_name') as mock_p: mock_p.return_value = ret_value if port_exist: # Should return False as the port is not deleted @@ -182,7 +193,7 @@ class TestOvsdbSbOvnIdl(test_base.TestCase): def _test_get_provider_datapath_from_cr_lrp(self, port, found_port=True): ret_value = (fakes.create_object({'datapath': 'dp1'}) if found_port else None) - with mock.patch.object(self.sb_idl, '_get_port_by_name') as mock_p: + with mock.patch.object(self.sb_idl, 'get_port_by_name') as mock_p: mock_p.return_value = ret_value if found_port: self.assertEqual( @@ -208,6 +219,13 @@ class TestOvsdbSbOvnIdl(test_base.TestCase): port = 'cr-lrp-port' self._test_get_provider_datapath_from_cr_lrp(port, found_port=False) + def test_get_datapath_from_port_peer(self): + with mock.patch.object(self.sb_idl, 'get_port_datapath') as m_dp: + port0 = fakes.create_object({'name': 'port-0', + 'options': {'peer': 'port-peer'}}) + self.sb_idl.get_datapath_from_port_peer(port0) + m_dp.assert_called_once_with('port-peer') + def _test_get_network_name_and_tag(self, network_in_bridge_map=True): tag = 1001 network = 'public' if network_in_bridge_map else 'spongebob' @@ -293,9 +311,41 @@ class TestOvsdbSbOvnIdl(test_base.TestCase): def test_get_lrp_port_for_datapath_no_options(self): self._test_get_lrp_port_for_datapath(has_options=False) + def test_get_lrp_ports_for_router(self): + with mock.patch.object(self.sb_idl, 'get_ports_on_datapath') as m_dp: + datapath = 'router-dp' + self.sb_idl.get_lrp_ports_for_router(datapath) + m_dp.assert_called_once_with(datapath, + constants.OVN_PATCH_VIF_PORT_TYPE) + + def test_get_lrp_ports_on_provider(self): + port = '39c38ce6-f0ea-484e-a57c-aec0d4e961a5' + with mock.patch.object(self.sb_idl, 'get_ports_by_type') as m_pt: + ch = fakes.create_object({'name': 'chassis-0'}) + row = fakes.create_object({'logical_port': port, 'chassis': [ch], + 'datapath': 'fake-dp'}) + m_pt.return_value = [row, ] + + with mock.patch.object(self.sb_idl, 'is_provider_network') as m_pn: + self.sb_idl.get_lrp_ports_on_provider() + m_pt.assert_called_once_with(constants.OVN_PATCH_VIF_PORT_TYPE) + m_pn.assert_called_once_with(row.datapath) + + def test_get_lrp_ports_on_provider_starts_with_lrp(self): + port = 'lrp-39c38ce6-f0ea-484e-a57c-aec0d4e961a5' + with mock.patch.object(self.sb_idl, 'get_ports_by_type') as m_pt: + ch = fakes.create_object({'name': 'chassis-0'}) + row = fakes.create_object({'logical_port': port, 'chassis': [ch]}) + m_pt.return_value = [row, ] + + with mock.patch.object(self.sb_idl, 'is_provider_network') as m_pn: + self.sb_idl.get_lrp_ports_on_provider() + m_pt.assert_called_once_with(constants.OVN_PATCH_VIF_PORT_TYPE) + m_pn.assert_not_called() + def _test_get_port_datapath(self, port_found=True): dp = '3fce2c5f-7801-469b-894e-05561e3bda15' - with mock.patch.object(self.sb_idl, '_get_port_by_name') as mock_p: + with mock.patch.object(self.sb_idl, 'get_port_by_name') as mock_p: port_info = None if port_found: port_info = fakes.create_object({'datapath': dp}) @@ -316,7 +366,7 @@ class TestOvsdbSbOvnIdl(test_base.TestCase): def test_get_ip_from_port_peer(self): ip = '172.24.200.7' port = fakes.create_object({'options': {'peer': 'fake-peer'}}) - with mock.patch.object(self.sb_idl, '_get_port_by_name') as mock_p: + with mock.patch.object(self.sb_idl, 'get_port_by_name') as mock_p: port_peer = fakes.create_object({ 'mac': ['aa:bb:cc:dd:ee:ff 172.24.200.7']}) mock_p.return_value = port_peer @@ -326,7 +376,7 @@ class TestOvsdbSbOvnIdl(test_base.TestCase): def test_get_ip_from_port_peer_port_not_found(self): port = fakes.create_object({'options': {'peer': 'fake-peer'}}) - with mock.patch.object(self.sb_idl, '_get_port_by_name') as mock_p: + with mock.patch.object(self.sb_idl, 'get_port_by_name') as mock_p: mock_p.return_value = [] self.assertRaises(exceptions.PortNotFound, @@ -342,7 +392,7 @@ class TestOvsdbSbOvnIdl(test_base.TestCase): port_name = port expected_return = 'spongebob' - with mock.patch.object(self.sb_idl, '_get_port_by_name') as mock_p: + with mock.patch.object(self.sb_idl, 'get_port_by_name') as mock_p: with mock.patch.object(self.sb_idl, 'get_evpn_info') as mock_evpn: mock_evpn.return_value = expected_return ret = self.sb_idl.get_evpn_info_from_port_name(port_name) @@ -387,7 +437,7 @@ class TestOvsdbSbOvnIdl(test_base.TestCase): def _test_get_port_if_local_chassis(self, wrong_chassis=False): chassis = 'wrong-chassis' if wrong_chassis else 'chassis-0' - with mock.patch.object(self.sb_idl, '_get_port_by_name') as mock_p: + with mock.patch.object(self.sb_idl, 'get_port_by_name') as mock_p: ch = fakes.create_object({'name': 'chassis-0'}) port = fakes.create_object({'chassis': [ch]}) mock_p.return_value = port 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 f544a1a7..1c54a59a 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 @@ -553,7 +553,7 @@ class TestTenantPortDeletedEvent(test_base.TestCase): mac=['aa:bb:cc:dd:ee:ff 10.10.1.16']) self.event.run(mock.Mock(), row, mock.Mock()) self.agent.withdraw_remote_ip.assert_called_once_with( - ['10.10.1.16'], row) + ['10.10.1.16'], row, mock.ANY) def test_run_dual_stack(self): row = utils.create_row( @@ -561,7 +561,7 @@ class TestTenantPortDeletedEvent(test_base.TestCase): mac=['aa:bb:cc:dd:ee:ff 10.10.1.16 2002::1234:abcd:ffff:c0a8:101']) self.event.run(mock.Mock(), row, mock.Mock()) self.agent.withdraw_remote_ip.assert_called_once_with( - ['10.10.1.16', '2002::1234:abcd:ffff:c0a8:101'], row) + ['10.10.1.16', '2002::1234:abcd:ffff:c0a8:101'], row, mock.ANY) def test_run_wrong_type(self): row = utils.create_row(type='brigadeiro') @@ -601,6 +601,10 @@ class TestOVNLBTenantPortEvent(test_base.TestCase): 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): + row = utils.create_row(mac=[]) + self.assertFalse(self.event.match_fn(mock.Mock(), row, mock.Mock())) + def test_run(self): event = self.event.ROW_CREATE row = utils.create_row( @@ -671,6 +675,15 @@ class TestOVNLBMemberUpdateEvent(test_base.TestCase): 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_run(self): row = utils.create_row(name='ovn-lb1', datapaths=['dp1', 's_dp1'], diff --git a/tox.ini b/tox.ini index 51126226..c956a524 100644 --- a/tox.ini +++ b/tox.ini @@ -34,7 +34,7 @@ setenv = commands = stestr run {posargs} coverage combine - coverage report --fail-under=85 --skip-covered --omit='*tests*' + coverage report --fail-under=87 --skip-covered --omit='*tests*' coverage html -d cover --omit='*tests*' coverage xml -o cover/coverage.xml --omit='*tests*'