From 7cf7ae05659b363e54fbb04d2c02f91b17763921 Mon Sep 17 00:00:00 2001 From: Edward Hope-Morley Date: Thu, 17 Mar 2022 12:56:21 +0000 Subject: [PATCH 1/5] Partially revert "Do not link up HA router gateway in backup node" This partially reverts commit c52029c39aa824a67095fbbf9e59eff769d92587. We revert everything except one minor addition to neutron/agent/l3/ha_router.py which ensures that ha_confs path is created when the keepalived manager is initialised. Closes-Bug: #1965297 Change-Id: I14ad015c4344b32f7210c924902dac4e6ad1ae88 (cherry picked from commit 36bf1df46df4de8f9ed0c19e1118480ce2e55d8a) --- neutron/agent/l3/dvr_edge_ha_router.py | 4 +-- neutron/agent/l3/ha.py | 9 ----- neutron/agent/l3/ha_router.py | 32 +++-------------- neutron/agent/l3/router_info.py | 20 ++++------- neutron/agent/linux/interface.py | 34 +++++++------------ .../unit/agent/l3/test_dvr_local_router.py | 1 - 6 files changed, 24 insertions(+), 76 deletions(-) diff --git a/neutron/agent/l3/dvr_edge_ha_router.py b/neutron/agent/l3/dvr_edge_ha_router.py index 4bbac99931f..49a8b54d12a 100644 --- a/neutron/agent/l3/dvr_edge_ha_router.py +++ b/neutron/agent/l3/dvr_edge_ha_router.py @@ -134,9 +134,7 @@ def external_gateway_updated(self, ex_gw_port, interface_name): def _external_gateway_added(self, ex_gw_port, interface_name, ns_name, preserve_ips): - link_up = self.external_gateway_link_up() - self._plug_external_gateway(ex_gw_port, interface_name, ns_name, - link_up=link_up) + self._plug_external_gateway(ex_gw_port, interface_name, ns_name) def _is_this_snat_host(self): return self.agent_conf.agent_mode == constants.L3_AGENT_MODE_DVR_SNAT diff --git a/neutron/agent/l3/ha.py b/neutron/agent/l3/ha.py index 062f94818af..863808be77e 100644 --- a/neutron/agent/l3/ha.py +++ b/neutron/agent/l3/ha.py @@ -188,15 +188,6 @@ def _enqueue_state_change(self, router_id, state): 'agent %(host)s', state_change_data) - # Set external gateway port link up or down according to state - if state == 'primary': - ri.set_external_gw_port_link_status(link_up=True, set_gw=True) - elif state == 'backup': - ri.set_external_gw_port_link_status(link_up=False) - else: - LOG.warning('Router %s has status %s, ' - 'no action to router gateway device.', - router_id, state) # TODO(dalvarez): Fix bug 1677279 by moving the IPv6 parameters # configuration to keepalived-state-change in order to remove the # dependency that currently exists on l3-agent running for the IPv6 diff --git a/neutron/agent/l3/ha_router.py b/neutron/agent/l3/ha_router.py index c4288803530..c204f5f9b64 100644 --- a/neutron/agent/l3/ha_router.py +++ b/neutron/agent/l3/ha_router.py @@ -170,6 +170,10 @@ def _init_keepalived_manager(self, process_monitor): throttle_restart_value=( self.agent_conf.ha_vrrp_advert_int * THROTTLER_MULTIPLIER)) + # The following call is required to ensure that if the state path does + # not exist it gets created. + self.keepalived_manager.get_full_config_file_path('test') + config = self.keepalived_manager.config interface_name = self.get_ha_device_name() @@ -469,9 +473,7 @@ def _get_filtered_dict(d, ignore): return port1_filtered == port2_filtered def external_gateway_added(self, ex_gw_port, interface_name): - link_up = self.external_gateway_link_up() - self._plug_external_gateway(ex_gw_port, interface_name, - self.ns_name, link_up=link_up) + self._plug_external_gateway(ex_gw_port, interface_name, self.ns_name) self._add_gateway_vip(ex_gw_port, interface_name) self._disable_ipv6_addressing_on_interface(interface_name) @@ -535,27 +537,3 @@ def enable_radvd(self, internal_ports=None): if (self.keepalived_manager.get_process().active and self.ha_state == 'primary'): super(HaRouter, self).enable_radvd(internal_ports) - - def external_gateway_link_up(self): - # Check HA router ha_state for its gateway port link state. - # 'backup' instance will not link up the gateway port. - return self.ha_state == 'primary' - - def set_external_gw_port_link_status(self, link_up, set_gw=False): - link_state = "up" if link_up else "down" - LOG.info('Set router %s gateway device link state to %s.', - self.router_id, link_state) - - ex_gw_port = self.get_ex_gw_port() - ex_gw_port_id = (ex_gw_port and ex_gw_port['id'] or - self.ex_gw_port and self.ex_gw_port['id']) - if ex_gw_port_id: - interface_name = self.get_external_device_name(ex_gw_port_id) - ns_name = self.get_gw_ns_name() - self.driver.set_link_status(interface_name, ns_name, - link_up=link_up) - if link_up and set_gw: - preserve_ips = self.get_router_preserve_ips() - self._external_gateway_settings(ex_gw_port, interface_name, - ns_name, preserve_ips) - self.routes_updated([], self.routes) diff --git a/neutron/agent/l3/router_info.py b/neutron/agent/l3/router_info.py index f77135b36d7..e85d8bf362c 100644 --- a/neutron/agent/l3/router_info.py +++ b/neutron/agent/l3/router_info.py @@ -750,16 +750,14 @@ def _list_floating_ip_cidrs(self): return [common_utils.ip_to_cidr(ip['floating_ip_address']) for ip in floating_ips] - def _plug_external_gateway(self, ex_gw_port, interface_name, ns_name, - link_up=True): + def _plug_external_gateway(self, ex_gw_port, interface_name, ns_name): self.driver.plug(ex_gw_port['network_id'], ex_gw_port['id'], interface_name, ex_gw_port['mac_address'], namespace=ns_name, prefix=EXTERNAL_DEV_PREFIX, - mtu=ex_gw_port.get('mtu'), - link_up=link_up) + mtu=ex_gw_port.get('mtu')) def _get_external_gw_ips(self, ex_gw_port): gateway_ips = [] @@ -819,11 +817,7 @@ def _external_gateway_added(self, ex_gw_port, interface_name, LOG.debug("External gateway added: port(%s), interface(%s), ns(%s)", ex_gw_port, interface_name, ns_name) self._plug_external_gateway(ex_gw_port, interface_name, ns_name) - self._external_gateway_settings(ex_gw_port, interface_name, - ns_name, preserve_ips) - def _external_gateway_settings(self, ex_gw_port, interface_name, - ns_name, preserve_ips): # Build up the interface and gateway IP addresses that # will be added to the interface. ip_cidrs = common_utils.fixed_ip_cidrs(ex_gw_port['fixed_ips']) @@ -868,19 +862,17 @@ def is_v6_gateway_set(self, gateway_ips): return any(netaddr.IPAddress(gw_ip).version == 6 for gw_ip in gateway_ips) - def get_router_preserve_ips(self): + def external_gateway_added(self, ex_gw_port, interface_name): preserve_ips = self._list_floating_ip_cidrs() + list( self.centralized_port_forwarding_fip_set) preserve_ips.extend(self.agent.pd.get_preserve_ips(self.router_id)) - return preserve_ips - - def external_gateway_added(self, ex_gw_port, interface_name): - preserve_ips = self.get_router_preserve_ips() self._external_gateway_added( ex_gw_port, interface_name, self.ns_name, preserve_ips) def external_gateway_updated(self, ex_gw_port, interface_name): - preserve_ips = self.get_router_preserve_ips() + preserve_ips = self._list_floating_ip_cidrs() + list( + self.centralized_port_forwarding_fip_set) + preserve_ips.extend(self.agent.pd.get_preserve_ips(self.router_id)) self._external_gateway_added( ex_gw_port, interface_name, self.ns_name, preserve_ips) diff --git a/neutron/agent/linux/interface.py b/neutron/agent/linux/interface.py index 1076edd94a6..41d50ee4b21 100644 --- a/neutron/agent/linux/interface.py +++ b/neutron/agent/linux/interface.py @@ -256,12 +256,17 @@ def configure_ipv6_forwarding(namespace, dev_name, enabled): {'dev': dev_name, 'enabled': int(enabled)}] ip_lib.sysctl(cmd, namespace=namespace) + @abc.abstractmethod + def plug_new(self, network_id, port_id, device_name, mac_address, + bridge=None, namespace=None, prefix=None, mtu=None): + """Plug in the interface only for new devices that don't exist yet.""" + def plug(self, network_id, port_id, device_name, mac_address, - bridge=None, namespace=None, prefix=None, mtu=None, link_up=True): + bridge=None, namespace=None, prefix=None, mtu=None): if not ip_lib.device_exists(device_name, namespace=namespace): self.plug_new(network_id, port_id, device_name, mac_address, - bridge, namespace, prefix, mtu, link_up) + bridge, namespace, prefix, mtu) else: LOG.info("Device %s already exists", device_name) if mtu: @@ -293,21 +298,10 @@ def set_mtu(self, device_name, mtu, namespace=None, prefix=None): LOG.warning("Interface driver cannot update MTU for ports") self._mtu_update_warn_logged = True - def set_link_status(self, device_name, namespace=None, link_up=True): - ns_dev = ip_lib.IPWrapper(namespace=namespace).device(device_name) - if not ns_dev.exists(): - LOG.debug("Device %s may concurrently be deleted.", device_name) - return - if link_up: - ns_dev.link.set_up() - else: - ns_dev.link.set_down() - class NullDriver(LinuxInterfaceDriver): def plug_new(self, network_id, port_id, device_name, mac_address, - bridge=None, namespace=None, prefix=None, mtu=None, - link_up=True): + bridge=None, namespace=None, prefix=None, mtu=None): pass def unplug(self, device_name, bridge=None, namespace=None, prefix=None): @@ -385,8 +379,7 @@ def _add_device_to_namespace(self, ip_wrapper, device, namespace): namespace_obj.add_device_to_namespace(device) def plug_new(self, network_id, port_id, device_name, mac_address, - bridge=None, namespace=None, prefix=None, mtu=None, - link_up=True): + bridge=None, namespace=None, prefix=None, mtu=None): """Plug in the interface.""" if not bridge: bridge = self.conf.OVS.integration_bridge @@ -442,8 +435,7 @@ def plug_new(self, network_id, port_id, device_name, mac_address, else: LOG.warning("No MTU configured for port %s", port_id) - if link_up: - ns_dev.link.set_up() + ns_dev.link.set_up() if self.conf.ovs_use_veth: # ovs-dpdk does not do checksum calculations for veth interface # (bug 1832021) @@ -488,8 +480,7 @@ class BridgeInterfaceDriver(LinuxInterfaceDriver): DEV_NAME_PREFIX = 'ns-' def plug_new(self, network_id, port_id, device_name, mac_address, - bridge=None, namespace=None, prefix=None, mtu=None, - link_up=True): + bridge=None, namespace=None, prefix=None, mtu=None): """Plugin the interface.""" ip = ip_lib.IPWrapper() @@ -508,8 +499,7 @@ def plug_new(self, network_id, port_id, device_name, mac_address, LOG.warning("No MTU configured for port %s", port_id) root_veth.link.set_up() - if link_up: - ns_veth.link.set_up() + ns_veth.link.set_up() def unplug(self, device_name, bridge=None, namespace=None, prefix=None): """Unplug the interface.""" diff --git a/neutron/tests/unit/agent/l3/test_dvr_local_router.py b/neutron/tests/unit/agent/l3/test_dvr_local_router.py index ed0902b4835..982597beffd 100644 --- a/neutron/tests/unit/agent/l3/test_dvr_local_router.py +++ b/neutron/tests/unit/agent/l3/test_dvr_local_router.py @@ -983,7 +983,6 @@ def test_initialize_dvr_ha_router_snat_ns_once(self): self.mock_driver.unplug.reset_mock() self._set_ri_kwargs(agent, router['id'], router) ri = dvr_edge_ha_rtr.DvrEdgeHaRouter(HOSTNAME, **self.ri_kwargs) - ri._ha_state_path = self.get_temp_file_path('router_ha_state') ri._create_snat_namespace = mock.Mock() ri._plug_external_gateway = mock.Mock() ri.initialize(mock.Mock()) From 1d9ce0406883dc9ec386a01eba826af6fde6ceb1 Mon Sep 17 00:00:00 2001 From: Miro Tomaska Date: Wed, 12 Oct 2022 08:42:18 -0500 Subject: [PATCH 2/5] Improve agent provision performance for large networks Before this patch, the metadata agent would provision network namespace for all subnets under a network(datapath) as soon as the first VM(vif port) was mounted on the chassis. This operation can take very long time for networks with lots of subnets. See the linked bug for more details. This patch changes this mechanism to "lazy load" where metadata agent provisions metadata namespace with only the subnets belonging to the active ports on the chassis. This results in virtually constant throughput not effected by the number of subnets. Merge Conflict: Using datapath_uuid :str in addition to net_name for teardown_datapath method to remain compatible with the method implementation in Yoga and before. Updated unit tests accordingly neutron/agent/ovn/metadata/agent.py neutron/tests/unit/agent/ovn/metadata/test_agent.py Closes-Bug: #1981113 Change-Id: Ia2a66cfd3fd1380c5204109742d44f09160548d2 (cherry picked from commit edf3b3f191c2eae229a754dcbfc448fa41bd8bc3) --- neutron/agent/ovn/metadata/agent.py | 202 +++++++---- .../unit/agent/ovn/metadata/test_agent.py | 317 ++++++++++++++---- 2 files changed, 382 insertions(+), 137 deletions(-) diff --git a/neutron/agent/ovn/metadata/agent.py b/neutron/agent/ovn/metadata/agent.py index d40aa26f5af..b90fed0fdb9 100644 --- a/neutron/agent/ovn/metadata/agent.py +++ b/neutron/agent/ovn/metadata/agent.py @@ -18,6 +18,7 @@ import threading import uuid +import netaddr from neutron_lib import constants as n_const from oslo_concurrency import lockutils from oslo_log import log @@ -46,7 +47,8 @@ OVN_VIF_PORT_TYPES = ("", "external", ) MetadataPortInfo = collections.namedtuple('MetadataPortInfo', ['mac', - 'ip_addresses']) + 'ip_addresses', + 'logical_port']) OVN_METADATA_UUID_NAMESPACE = uuid.UUID('d34bf9f6-da32-4871-9af8-15a4626b41ab') @@ -87,7 +89,7 @@ def run(self, event, row, old): net_name = ovn_utils.get_network_name_from_datapath( row.datapath) LOG.info(self.LOG_MSG, row.logical_port, net_name) - self.agent.update_datapath(str(row.datapath.uuid), net_name) + self.agent.provision_datapath(row.datapath) except ConfigException: # We're now in the reader lock mode, we need to exit the # context and then use writer lock @@ -315,11 +317,12 @@ def _get_ovn_bridge(self): "br-int instead.") return 'br-int' - def get_networks(self): + def get_networks_datapaths(self): + """Return a set of datapath objects of the VIF ports on the current + chassis. + """ ports = self.sb_idl.get_ports_on_chassis(self.chassis) - return {(str(p.datapath.uuid), - ovn_utils.get_network_name_from_datapath(p.datapath)) - for p in self._vif_ports(ports)} + return set(p.datapath for p in self._vif_ports(ports)) @_sync_lock def sync(self): @@ -334,10 +337,10 @@ def sync(self): system_namespaces = tuple( ns.decode('utf-8') if isinstance(ns, bytes) else ns for ns in ip_lib.list_network_namespaces()) - nets = self.get_networks() + net_datapaths = self.get_networks_datapaths() metadata_namespaces = [ - self._get_namespace_name(net[1]) - for net in nets + self._get_namespace_name(str(datapath.uuid)) + for datapath in net_datapaths ] unused_namespaces = [ns for ns in system_namespaces if ns.startswith(NS_PREFIX) and @@ -347,7 +350,8 @@ def sync(self): # now that all obsolete namespaces are cleaned up, deploy required # networks - self.ensure_all_networks_provisioned(nets) + for datapath in net_datapaths: + self.provision_datapath(datapath) @staticmethod def _get_veth_name(datapath): @@ -395,25 +399,6 @@ def teardown_datapath(self, datapath, net_name=None): ip.garbage_collect_namespace() - def update_datapath(self, datapath, net_name): - """Update the metadata service for this datapath. - - This function will: - * Provision the namespace if it wasn't already in place. - * Update the namespace if it was already serving metadata (for example, - after binding/unbinding the first/last port of a subnet in our - chassis). - * Tear down the namespace if there are no more ports in our chassis - for this datapath. - """ - ports = self.sb_idl.get_ports_on_chassis(self.chassis) - datapath_ports = [p for p in self._vif_ports(ports) if - str(p.datapath.uuid) == datapath] - if datapath_ports: - self.provision_datapath(datapath, net_name) - else: - self.teardown_datapath(datapath, net_name) - def _ensure_datapath_checksum(self, namespace): """Ensure the correct checksum in the metadata packets in DPDK bridges @@ -433,45 +418,130 @@ def _ensure_datapath_checksum(self, namespace): iptables_mgr.ipv4['mangle'].add_rule('POSTROUTING', rule, wrap=False) iptables_mgr.apply() - def provision_datapath(self, datapath, net_name): - """Provision the datapath so that it can serve metadata. + def _get_port_ips(self, port): + # Retrieve IPs from the port mac column which is in form + # [" ... "] + mac_field_attrs = port.mac[0].split() + ips = mac_field_attrs[1:] + if not ips: + LOG.debug("Port %s IP addresses were not retrieved from the " + "Port_Binding MAC column %s", port.uuid, mac_field_attrs) + return ips + + def _active_subnets_cidrs(self, datapath_ports_ips, metadata_port_cidrs): + active_subnets_cidrs = set() + # Prepopulate a dictionary where each metadata_port_cidr(string) maps + # to its netaddr.IPNetwork object. This is so we dont have to + # reconstruct IPNetwork objects repeatedly in the for loop + metadata_cidrs_to_network_objects = { + metadata_port_cidr: netaddr.IPNetwork(metadata_port_cidr) + for metadata_port_cidr in metadata_port_cidrs + } + + for datapath_port_ip in datapath_ports_ips: + ip_obj = netaddr.IPAddress(datapath_port_ip) + for metadata_cidr, metadata_cidr_obj in \ + metadata_cidrs_to_network_objects.items(): + if ip_obj in metadata_cidr_obj: + active_subnets_cidrs.add(metadata_cidr) + break + return active_subnets_cidrs + + def _process_cidrs(self, current_namespace_cidrs, + datapath_ports_ips, metadata_port_subnet_cidrs): + active_subnets_cidrs = self._active_subnets_cidrs( + datapath_ports_ips, metadata_port_subnet_cidrs) + + cidrs_to_add = active_subnets_cidrs - current_namespace_cidrs + + if n_const.METADATA_CIDR not in current_namespace_cidrs: + cidrs_to_add.add(n_const.METADATA_CIDR) + else: + active_subnets_cidrs.add(n_const.METADATA_CIDR) - This function will create the namespace and VETH pair if needed - and assign the IP addresses to the interface corresponding to the - metadata port of the network. It will also remove existing IP - addresses that are no longer needed. + cidrs_to_delete = current_namespace_cidrs - active_subnets_cidrs + + return cidrs_to_add, cidrs_to_delete + + def _get_provision_params(self, datapath): + """Performs datapath preprovision checks and returns paremeters + needed to provision namespace. + + Function will confirm that: + 1. Datapath metadata port has valid MAC and subnet CIDRs + 2. There are datapath port IPs + + If any of those rules are not valid the nemaspace for the + provided datapath will be tore down. + If successful, returns datapath's network name, ports IPs + and meta port info """ - LOG.debug("Provisioning metadata for network %s", net_name) - port = self.sb_idl.get_metadata_port_network(datapath) + net_name = ovn_utils.get_network_name_from_datapath(datapath) + datapath_uuid = str(datapath.uuid) + + metadata_port = self.sb_idl.get_metadata_port_network(datapath_uuid) # If there's no metadata port or it doesn't have a MAC or IP # addresses, then tear the namespace down if needed. This might happen # when there are no subnets yet created so metadata port doesn't have # an IP address. - if not (port and port.mac and - port.external_ids.get(ovn_const.OVN_CIDRS_EXT_ID_KEY, None)): + if not (metadata_port and metadata_port.mac and + metadata_port.external_ids.get( + ovn_const.OVN_CIDRS_EXT_ID_KEY, None)): LOG.debug("There is no metadata port for network %s or it has no " "MAC or IP addresses configured, tearing the namespace " "down if needed", net_name) - self.teardown_datapath(datapath, net_name) + self.teardown_datapath(datapath_uuid, net_name) return # First entry of the mac field must be the MAC address. - match = MAC_PATTERN.match(port.mac[0].split(' ')[0]) - # If it is not, we can't provision the namespace. Tear it down if - # needed and log the error. + match = MAC_PATTERN.match(metadata_port.mac[0].split(' ')[0]) if not match: LOG.error("Metadata port for network %s doesn't have a MAC " "address, tearing the namespace down if needed", net_name) - self.teardown_datapath(datapath) + self.teardown_datapath(datapath_uuid, net_name) return mac = match.group() ip_addresses = set( - port.external_ids[ovn_const.OVN_CIDRS_EXT_ID_KEY].split(' ')) - ip_addresses.add(n_const.METADATA_CIDR) - metadata_port = MetadataPortInfo(mac, ip_addresses) + metadata_port.external_ids[ + ovn_const.OVN_CIDRS_EXT_ID_KEY].split(' ')) + metadata_port_info = MetadataPortInfo(mac, ip_addresses, + metadata_port.logical_port) + + chassis_ports = self.sb_idl.get_ports_on_chassis(self.chassis) + datapath_ports_ips = [] + for chassis_port in self._vif_ports(chassis_ports): + if str(chassis_port.datapath.uuid) == datapath_uuid: + datapath_ports_ips.extend(self._get_port_ips(chassis_port)) + + if not datapath_ports_ips: + LOG.debug("No valid VIF ports were found for network %s, " + "tearing the namespace down if needed", net_name) + self.teardown_datapath(datapath_uuid, net_name) + return + + return net_name, datapath_ports_ips, metadata_port_info + + def provision_datapath(self, datapath): + """Provision the datapath so that it can serve metadata. + + This function will create the namespace and VETH pair if needed + and assign the IP addresses to the interface corresponding to the + metadata port of the network. It will also remove existing IP from + the namespace if they are no longer needed. + :param datapath: datapath object. + :return: The metadata namespace name for the datapath or None + if namespace was not provisioned + """ + + provision_params = self._get_provision_params(datapath) + if not provision_params: + return + net_name, datapath_ports_ips, metadata_port_info = provision_params + + LOG.info("Provisioning metadata for network %s", net_name) # Create the VETH pair if it's not created. Also the add_veth function # will create the namespace for us. namespace = self._get_namespace_name(net_name) @@ -496,22 +566,24 @@ def provision_datapath(self, datapath, net_name): ip2.link.set_up() # Configure the MAC address. - ip2.link.set_address(metadata_port.mac) - dev_info = ip2.addr.list() - - # Configure the IP addresses on the VETH pair and remove those - # that we no longer need. - current_cidrs = {dev['cidr'] for dev in dev_info} - for ipaddr in current_cidrs - metadata_port.ip_addresses: - ip2.addr.delete(ipaddr) - for ipaddr in metadata_port.ip_addresses - current_cidrs: + ip2.link.set_address(metadata_port_info.mac) + + cidrs_to_add, cidrs_to_delete = self._process_cidrs( + {dev['cidr'] for dev in ip2.addr.list()}, + datapath_ports_ips, + metadata_port_info.ip_addresses + ) + # Delete any non active addresses from the network namespace + for cidr in cidrs_to_delete: + ip2.addr.delete(cidr) + for cidr in cidrs_to_add: # NOTE(dalvarez): metadata only works on IPv4. We're doing this # extra check here because it could be that the metadata port has # an IPv6 address if there's an IPv6 subnet with SLAAC in its # network. Neutron IPAM will autoallocate an IPv6 address for every # port in the network. - if utils.get_ip_version(ipaddr) == 4: - ip2.addr.add(ipaddr) + if utils.get_ip_version(cidr) == n_const.IP_VERSION_4: + ip2.addr.add(cidr) # Check that this port is not attached to any other OVS bridge. This # can happen when the OVN bridge changes (for example, during a @@ -536,7 +608,8 @@ def provision_datapath(self, datapath, net_name): veth_name[0]).execute() self.ovs_idl.db_set( 'Interface', veth_name[0], - ('external_ids', {'iface-id': port.logical_port})).execute() + ('external_ids', {'iface-id': + metadata_port_info.logical_port})).execute() # Ensure the correct checksum in the metadata traffic. self._ensure_datapath_checksum(namespace) @@ -546,14 +619,3 @@ def provision_datapath(self, datapath, net_name): self._process_monitor, namespace, n_const.METADATA_PORT, self.conf, bind_address=n_const.METADATA_V4_IP, network_id=net_name) - - def ensure_all_networks_provisioned(self, nets): - """Ensure that all requested datapaths are provisioned. - - This function will make sure that requested datapaths have their - namespaces, VETH pair and OVS ports created and metadata proxies are up - and running. - """ - # Make sure that all those datapaths are serving metadata - for datapath, net_name in nets: - self.provision_datapath(datapath, net_name) diff --git a/neutron/tests/unit/agent/ovn/metadata/test_agent.py b/neutron/tests/unit/agent/ovn/metadata/test_agent.py index 79357c1ffa8..8cc13aecdfa 100644 --- a/neutron/tests/unit/agent/ovn/metadata/test_agent.py +++ b/neutron/tests/unit/agent/ovn/metadata/test_agent.py @@ -36,7 +36,15 @@ OvnPortInfo = collections.namedtuple( 'OvnPortInfo', ['datapath', 'type', 'mac', 'external_ids', 'logical_port']) -DatapathInfo = collections.namedtuple('DatapathInfo', ['uuid', 'external_ids']) + + +class DatapathInfo: + def __init__(self, uuid, external_ids): + self.uuid = uuid + self.external_ids = external_ids + + def __hash__(self): + return hash(self.uuid) def makePort(datapath=None, type='', mac=None, external_ids=None, @@ -82,7 +90,7 @@ def setUp(self): def test_sync(self): with mock.patch.object( - self.agent, 'ensure_all_networks_provisioned') as enp,\ + self.agent, 'provision_datapath') as pdp,\ mock.patch.object( ip_lib, 'list_network_namespaces') as lnn,\ mock.patch.object( @@ -91,10 +99,13 @@ def test_sync(self): self.agent.sync() - enp.assert_called_once_with({ - (p.datapath.uuid, p.datapath.uuid) - for p in self.ports - }) + pdp.assert_has_calls( + [ + mock.call(p.datapath) + for p in self.ports + ], + any_order=True + ) lnn.assert_called_once_with() tdp.assert_not_called() @@ -102,7 +113,7 @@ def test_sync(self): def test_sync_teardown_namespace(self): """Test that sync tears down unneeded metadata namespaces.""" with mock.patch.object( - self.agent, 'ensure_all_networks_provisioned') as enp,\ + self.agent, 'provision_datapath') as pdp,\ mock.patch.object( ip_lib, 'list_network_namespaces') as lnn,\ mock.patch.object( @@ -112,57 +123,54 @@ def test_sync_teardown_namespace(self): self.agent.sync() - enp.assert_called_once_with({ - (p.datapath.uuid, p.datapath.uuid) - for p in self.ports - }) + pdp.assert_has_calls( + [ + mock.call(p.datapath) + for p in self.ports + ], + any_order=True + ) lnn.assert_called_once_with() tdp.assert_called_once_with('3') - def test_get_networks(self): - """Test which networks are provisioned. - + def test_get_networks_datapaths(self): + """Test get_networks_datapaths returns only datapath objects for the + networks containing vif ports of type ''(blank) and 'external'. This test simulates that this chassis has the following ports: - * datapath '0': 1 port - * datapath '1': 2 ports - * datapath '2': 1 port + * datapath '1': 1 port type '' , 1 port 'external' and + 1 port 'unknown' + * datapath '2': 1 port type '' * datapath '3': 1 port with type 'external' - * datapath '5': 1 port with type 'unknown' + * datapath '4': 1 port with type 'unknown' - It is expected that only datapaths '0', '1' and '2' are scheduled for - provisioning. + It is expected that only datapaths '1', '2' and '3' are returned """ - self.ports.append(makePort(datapath=DatapathInfo(uuid='1', - external_ids={'name': 'neutron-1'}))) - self.ports.append(makePort(datapath=DatapathInfo(uuid='3', - external_ids={'name': 'neutron-3'}), type='external')) - self.ports.append(makePort(datapath=DatapathInfo(uuid='5', - external_ids={'name': 'neutron-5'}), type='unknown')) - - expected_networks = {(str(i), str(i)) for i in range(0, 4)} - self.assertEqual(expected_networks, self.agent.get_networks()) - - def test_update_datapath_provision(self): - self.ports.append(makePort(datapath=DatapathInfo(uuid='3', - external_ids={'name': 'neutron-3'}), type='external')) - - with mock.patch.object(self.agent, 'provision_datapath', - return_value=None) as pdp,\ - mock.patch.object(self.agent, 'teardown_datapath') as tdp: - self.agent.update_datapath('1', 'a') - self.agent.update_datapath('3', 'b') - expected_calls = [mock.call('1', 'a'), mock.call('3', 'b')] - pdp.assert_has_calls(expected_calls) - tdp.assert_not_called() - - def test_update_datapath_teardown(self): - with mock.patch.object(self.agent, 'provision_datapath', - return_value=None) as pdp,\ - mock.patch.object(self.agent, 'teardown_datapath') as tdp: - self.agent.update_datapath('5', 'a') - tdp.assert_called_once_with('5', 'a') - pdp.assert_not_called() + datapath_1 = DatapathInfo(uuid='uuid1', + external_ids={'name': 'neutron-1'}) + datapath_2 = DatapathInfo(uuid='uuid2', + external_ids={'name': 'neutron-2'}) + datapath_3 = DatapathInfo(uuid='uuid3', + external_ids={'name': 'neutron-3'}) + datapath_4 = DatapathInfo(uuid='uuid4', + external_ids={'name': 'neutron-4'}) + + ports = [ + makePort(datapath_1, type=''), + makePort(datapath_1, type='external'), + makePort(datapath_1, type='unknown'), + makePort(datapath_2, type=''), + makePort(datapath_3, type='external'), + makePort(datapath_4, type='unknown') + ] + + with mock.patch.object(self.agent.sb_idl, 'get_ports_on_chassis', + return_value=ports): + expected_datapaths = set([datapath_1, datapath_2, datapath_3]) + self.assertSetEqual( + expected_datapaths, + self.agent.get_networks_datapaths() + ) def test_teardown_datapath(self): """Test teardown datapath. @@ -191,6 +199,174 @@ def test_teardown_datapath(self): del_veth.assert_called_once_with('veth_0') garbage_collect.assert_called_once_with() + def test__process_cidrs_when_current_namespace_empty(self): + current_namespace_cidrs = set() + datapath_port_ips = ['10.0.0.2', '10.0.0.3', '10.0.1.5'] + metadaport_subnet_cidrs = ['10.0.0.0/30', '10.0.1.0/28', '11.0.1.2/24'] + + expected_cidrs_to_add = set(['10.0.0.0/30', '10.0.1.0/28', + n_const.METADATA_CIDR]) + expected_cidrs_to_delete = set() + + actual_result = self.agent._process_cidrs(current_namespace_cidrs, + datapath_port_ips, + metadaport_subnet_cidrs) + actual_cidrs_to_add, actual_cidrs_to_delete = actual_result + + self.assertSetEqual(actual_cidrs_to_add, expected_cidrs_to_add) + self.assertSetEqual(actual_cidrs_to_delete, expected_cidrs_to_delete) + + def test__process_cidrs_when_current_namespace_only_contains_metadata_cidr( + self): + current_namespace_cidrs = set([n_const.METADATA_CIDR]) + datapath_port_ips = ['10.0.0.2', '10.0.0.3', '10.0.1.5'] + metadaport_subnet_cidrs = ['10.0.0.0/30', '10.0.1.0/28', '11.0.1.2/24'] + + expected_cidrs_to_add = set(['10.0.0.0/30', '10.0.1.0/28']) + expected_cidrs_to_delete = set() + + actual_result = self.agent._process_cidrs(current_namespace_cidrs, + datapath_port_ips, + metadaport_subnet_cidrs) + actual_cidrs_to_add, actual_cidrs_to_delete = actual_result + + self.assertSetEqual(actual_cidrs_to_add, expected_cidrs_to_add) + self.assertSetEqual(actual_cidrs_to_delete, expected_cidrs_to_delete) + + def test__process_cidrs_when_current_namespace_contains_stale_cidr(self): + current_namespace_cidrs = set([n_const.METADATA_CIDR, '10.0.1.0/31']) + datapath_port_ips = ['10.0.0.2', '10.0.0.3', '10.0.1.5'] + metadaport_subnet_cidrs = ['10.0.0.0/30', '10.0.1.0/28', '11.0.1.2/24'] + + expected_cidrs_to_add = set(['10.0.0.0/30', '10.0.1.0/28']) + expected_cidrs_to_delete = set(['10.0.1.0/31']) + + actual_result = self.agent._process_cidrs(current_namespace_cidrs, + datapath_port_ips, + metadaport_subnet_cidrs) + actual_cidrs_to_add, actual_cidrs_to_delete = actual_result + + self.assertSetEqual(actual_cidrs_to_add, expected_cidrs_to_add) + self.assertSetEqual(actual_cidrs_to_delete, expected_cidrs_to_delete) + + def test__process_cidrs_when_current_namespace_contains_mix_cidrs(self): + """Current namespace cidrs contains stale cidrs and it is missing + new required cidrs. + """ + current_namespace_cidrs = set([n_const.METADATA_CIDR, + '10.0.1.0/31', + '10.0.1.0/28']) + datapath_port_ips = ['10.0.0.2', '10.0.1.5'] + metadaport_subnet_cidrs = ['10.0.0.0/30', '10.0.1.0/28', '11.0.1.2/24'] + + expected_cidrs_to_add = set(['10.0.0.0/30']) + expected_cidrs_to_delete = set(['10.0.1.0/31']) + + actual_result = self.agent._process_cidrs(current_namespace_cidrs, + datapath_port_ips, + metadaport_subnet_cidrs) + actual_cidrs_to_add, actual_cidrs_to_delete = actual_result + + self.assertSetEqual(actual_cidrs_to_add, expected_cidrs_to_add) + self.assertSetEqual(actual_cidrs_to_delete, expected_cidrs_to_delete) + + def test__get_provision_params_returns_none_when_metadata_port_is_missing( + self): + """Should return None when there is no metadata port in datapath and + call teardown datapath. + """ + network_id = '1' + datapath = DatapathInfo(uuid='test123', + external_ids={'name': 'neutron-{}'.format(network_id)}) + + with mock.patch.object( + self.agent.sb_idl, 'get_metadata_port_network', + return_value=None),\ + mock.patch.object( + self.agent, 'teardown_datapath') as tdp: + self.assertIsNone(self.agent._get_provision_params(datapath)) + tdp.assert_called_once_with(datapath.uuid, network_id) + + def test__get_provision_params_returns_none_when_metadata_port_missing_mac( + self): + """Should return None when metadata port is missing MAC and + call teardown datapath. + """ + network_id = '1' + datapath = DatapathInfo(uuid='test123', + external_ids={'name': 'neutron-{}'.format(network_id)}) + metadadata_port = makePort(datapath, + mac=['NO_MAC_HERE 1.2.3.4'], + external_ids={'neutron:cidrs': + '10.204.0.10/29'}) + + with mock.patch.object( + self.agent.sb_idl, 'get_metadata_port_network', + return_value=metadadata_port),\ + mock.patch.object( + self.agent, 'teardown_datapath') as tdp: + self.assertIsNone(self.agent._get_provision_params(datapath)) + tdp.assert_called_once_with(datapath.uuid, network_id) + + def test__get_provision_params_returns_none_when_no_vif_ports(self): + """Should return None when there are no datapath ports with type + "external" or ""(blank) and call teardown datapath. + """ + network_id = '1' + datapath = DatapathInfo(uuid='test123', + external_ids={'name': 'neutron-{}'.format(network_id)}) + datapath_ports = [makePort(datapath, type='not_vif_type')] + metadadata_port = makePort(datapath, + mac=['fa:16:3e:22:65:18 1.2.3.4'], + external_ids={'neutron:cidrs': + '10.204.0.10/29'}) + + with mock.patch.object(self.agent.sb_idl, 'get_metadata_port_network', + return_value=metadadata_port),\ + mock.patch.object(self.agent.sb_idl, 'get_ports_on_chassis', + return_value=datapath_ports),\ + mock.patch.object(self.agent, 'teardown_datapath') as tdp: + self.assertIsNone(self.agent._get_provision_params(datapath)) + tdp.assert_called_once_with(datapath.uuid, network_id) + + def test__get_provision_params_returns_provision_parameters(self): + """The happy path when datapath has ports with "external" or ""(blank) + types and metadata port contains MAC and subnet CIDRs. + """ + network_id = '1' + port_ip = '1.2.3.4' + metada_port_mac = "fa:16:3e:22:65:18" + metada_port_subnet_cidr = "10.204.0.10/29" + metada_port_logical_port = "3b66c176-199b-48ec-8331-c1fd3f6e2b44" + + datapath = DatapathInfo(uuid='test123', + external_ids={'name': 'neutron-{}'.format(network_id)}) + datapath_ports = [makePort(datapath, + mac=['fa:16:3e:e7:ac {}'.format(port_ip)])] + metadadata_port = makePort(datapath, + mac=[ + '{} 10.204.0.1'.format(metada_port_mac) + ], + external_ids={'neutron:cidrs': + metada_port_subnet_cidr}, + logical_port=metada_port_logical_port) + + with mock.patch.object(self.agent.sb_idl, 'get_metadata_port_network', + return_value=metadadata_port),\ + mock.patch.object(self.agent.sb_idl, 'get_ports_on_chassis', + return_value=datapath_ports): + actual_params = self.agent._get_provision_params(datapath) + + net_name, datapath_port_ips, metadata_port_info = actual_params + + self.assertEqual(network_id, net_name) + self.assertListEqual([port_ip], datapath_port_ips) + self.assertEqual(metada_port_mac, metadata_port_info.mac) + self.assertSetEqual(set([metada_port_subnet_cidr]), + metadata_port_info.ip_addresses) + self.assertEqual(metada_port_logical_port, + metadata_port_info.logical_port) + def test_provision_datapath(self): """Test datapath provisioning. @@ -198,16 +374,21 @@ def test_provision_datapath(self): namespace are created, that the interface is properly configured with the right IP addresses and that the metadata proxy is spawned. """ - - metadata_port = makePort(mac=['aa:bb:cc:dd:ee:ff'], - external_ids={ - 'neutron:cidrs': '10.0.0.1/23 ' - '2001:470:9:1224:5595:dd51:6ba2:e788/64'}, - logical_port='port') - - with mock.patch.object(self.agent.sb_idl, - 'get_metadata_port_network', - return_value=metadata_port),\ + net_name = '123' + metadaport_logical_port = '123-abc-456' + datapath_ports_ips = ['10.0.0.1', '10.0.0.2'] + metada_port_info = agent.MetadataPortInfo( + mac='aa:bb:cc:dd:ee:ff', + ip_addresses=['10.0.0.1/23', + '2001:470:9:1224:5595:dd51:6ba2:e788/64'], + logical_port=metadaport_logical_port + ) + provision_params = (net_name, datapath_ports_ips, metada_port_info,) + nemaspace_name = 'namespace' + + with mock.patch.object(self.agent, + '_get_provision_params', + return_value=provision_params),\ mock.patch.object( ip_lib, 'device_exists', return_value=False),\ mock.patch.object( @@ -215,7 +396,7 @@ def test_provision_datapath(self): mock.patch.object(agent.MetadataAgent, '_get_veth_name', return_value=['veth_0', 'veth_1']),\ mock.patch.object(agent.MetadataAgent, '_get_namespace_name', - return_value='namespace'),\ + return_value=nemaspace_name),\ mock.patch.object(ip_link, 'set_up') as link_set_up,\ mock.patch.object(ip_link, 'set_address') as link_set_addr,\ mock.patch.object(ip_addr, 'list', return_value=[]),\ @@ -234,13 +415,14 @@ def test_provision_datapath(self): # We need to assert that it was deleted first. self.agent.ovs_idl.list_br.return_value.execute.return_value = ( ['br-int', 'br-fake']) - self.agent.provision_datapath('1', '1') + self.agent.provision_datapath('fake_datapath') # Check that the port was deleted from br-fake self.agent.ovs_idl.del_port.assert_called_once_with( 'veth_0', bridge='br-fake', if_exists=True) # Check that the VETH pair is created - add_veth.assert_called_once_with('veth_0', 'veth_1', 'namespace') + add_veth.assert_called_once_with('veth_0', 'veth_1', + nemaspace_name) # Make sure that the two ends of the VETH pair have been set as up. self.assertEqual(2, link_set_up.call_count) link_set_addr.assert_called_once_with('aa:bb:cc:dd:ee:ff') @@ -248,7 +430,8 @@ def test_provision_datapath(self): self.agent.ovs_idl.add_port.assert_called_once_with( 'br-int', 'veth_0') self.agent.ovs_idl.db_set.assert_called_once_with( - 'Interface', 'veth_0', ('external_ids', {'iface-id': 'port'})) + 'Interface', 'veth_0', + ('external_ids', {'iface-id': metadaport_logical_port})) # Check that the metadata port has the IP addresses properly # configured and that IPv6 address has been skipped. expected_calls = [mock.call('10.0.0.1/23'), @@ -257,9 +440,9 @@ def test_provision_datapath(self): sorted(ip_addr_add.call_args_list)) # Check that metadata proxy has been spawned spawn_mdp.assert_called_once_with( - mock.ANY, 'namespace', 80, mock.ANY, - bind_address=n_const.METADATA_V4_IP, network_id='1') - mock_checksum.assert_called_once_with('namespace') + mock.ANY, nemaspace_name, 80, mock.ANY, + bind_address=n_const.METADATA_V4_IP, network_id=net_name) + mock_checksum.assert_called_once_with(nemaspace_name) def test__load_config(self): # Chassis name UUID formatted string. OVN bridge "br-ovn". From b0081ea6db6df2bc1abe32cb381377889aae658b Mon Sep 17 00:00:00 2001 From: Slawek Kaplonski Date: Fri, 13 Jan 2023 15:13:42 +0100 Subject: [PATCH 3/5] Increase fullstack job's timeout It seems that recently we are often timing out the fullstack job when it's executed on some busy node. To avoid that, lets give 20 more minutes to the timeout of the fullstack jobs. Change-Id: I37ec0ba04bafb3b7baec6003155b7d9c43092667 (cherry picked from commit 3354b43d5ad7fd8e950b2d875fc3e017bc87b0a7) --- zuul.d/base.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/zuul.d/base.yaml b/zuul.d/base.yaml index 302538f6f31..216044f3388 100644 --- a/zuul.d/base.yaml +++ b/zuul.d/base.yaml @@ -58,6 +58,7 @@ - job: name: neutron-fullstack parent: neutron-functional + timeout: 9000 vars: tox_envlist: dsvm-fullstack-gate Q_BUILD_OVS_FROM_GIT: False From 254d3d0e5cc5fc0fb3fbc28606cde1f36ab57ba9 Mon Sep 17 00:00:00 2001 From: Arnau Verdaguer Date: Fri, 19 Aug 2022 16:40:50 +0200 Subject: [PATCH 4/5] [Trunk] Update the trunk status with the parent status After a trunk VM has been migrated the trunk status remains DOWN, After the parent port is back to active modify the trunk status. Closes-Bug: #1988549 Change-Id: Ia0f7a6e8510af2c3545993e0d0d4bb06a9b70b79 (cherry picked from commit 178ee6fd3d76802cd7f577ad3d0d190117e78962) --- neutron/services/trunk/plugin.py | 10 ++++++- .../tests/unit/services/trunk/test_plugin.py | 27 +++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/neutron/services/trunk/plugin.py b/neutron/services/trunk/plugin.py index 4a2fc297391..15cf7c066a2 100644 --- a/neutron/services/trunk/plugin.py +++ b/neutron/services/trunk/plugin.py @@ -21,6 +21,7 @@ from neutron_lib.callbacks import events from neutron_lib.callbacks import registry from neutron_lib.callbacks import resources +from neutron_lib import constants as const from neutron_lib import context from neutron_lib.db import api as db_api from neutron_lib.db import resource_extend @@ -463,12 +464,19 @@ def _trigger_trunk_status_change(self, resource, event, trigger, payload): original_port = payload.states[0] orig_vif_type = original_port.get(portbindings.VIF_TYPE) new_vif_type = updated_port.get(portbindings.VIF_TYPE) + orig_status = original_port.get('status') + new_status = updated_port.get('status') vif_type_changed = orig_vif_type != new_vif_type + trunk_id = trunk_details['trunk_id'] if vif_type_changed and new_vif_type == portbindings.VIF_TYPE_UNBOUND: - trunk_id = trunk_details['trunk_id'] # NOTE(status_police) Trunk status goes to DOWN when the parent # port is unbound. This means there are no more physical resources # associated with the logical resource. self.update_trunk( context, trunk_id, {'trunk': {'status': constants.TRUNK_DOWN_STATUS}}) + elif new_status == const.PORT_STATUS_ACTIVE and \ + new_status != orig_status: + self.update_trunk( + context, trunk_id, + {'trunk': {'status': constants.TRUNK_ACTIVE_STATUS}}) diff --git a/neutron/tests/unit/services/trunk/test_plugin.py b/neutron/tests/unit/services/trunk/test_plugin.py index ebeb169dea1..f2c26cdcef3 100644 --- a/neutron/tests/unit/services/trunk/test_plugin.py +++ b/neutron/tests/unit/services/trunk/test_plugin.py @@ -19,6 +19,7 @@ from neutron_lib.callbacks import events from neutron_lib.callbacks import registry from neutron_lib.callbacks import resources +from neutron_lib import constants as neutron_const from neutron_lib.plugins import directory from neutron_lib.services.trunk import constants import testtools @@ -285,6 +286,32 @@ def test_remove_subports_trunk_goes_to_down(self): {'sub_ports': [{'port_id': subport['port']['id']}]}) self.assertEqual(constants.TRUNK_DOWN_STATUS, trunk['status']) + def test__trigger_trunk_status_change_parent_port_status_down(self): + callback = register_mock_callback(resources.TRUNK, events.AFTER_UPDATE) + with self.port() as parent: + parent['status'] = neutron_const.PORT_STATUS_DOWN + original_port = {'status': neutron_const.PORT_STATUS_DOWN} + _, _ = ( + self._test__trigger_trunk_status_change( + parent, original_port, + constants.TRUNK_DOWN_STATUS, + constants.TRUNK_DOWN_STATUS)) + callback.assert_not_called() + + def test__trigger_trunk_status_change_parent_port_status_up(self): + callback = register_mock_callback(resources.TRUNK, events.AFTER_UPDATE) + with self.port() as parent: + parent['status'] = neutron_const.PORT_STATUS_ACTIVE + original_port = {'status': neutron_const.PORT_STATUS_DOWN} + _, _ = ( + self._test__trigger_trunk_status_change( + parent, original_port, + constants.TRUNK_DOWN_STATUS, + constants.TRUNK_ACTIVE_STATUS)) + callback.assert_called_once_with( + resources.TRUNK, events.AFTER_UPDATE, + self.trunk_plugin, payload=mock.ANY) + def test__trigger_trunk_status_change_vif_type_changed_unbound(self): callback = register_mock_callback(resources.TRUNK, events.AFTER_UPDATE) with self.port() as parent: From 5cb3428d770ccd5a95bd3a5fcf799ad07f30f64d Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Thu, 26 Jan 2023 08:37:24 -0600 Subject: [PATCH 5/5] Never raise an exception in notify() notify() is called from python-ovs code which is not built to recover from an exception in this user-overriden code. If there is an exception (e.g. the DB server is down when we process the hash ring), this exception can cause an unrecoverable error in processing OVSDB messages, rendering the neutron worker useless. Change-Id: I5f703d82175d71a222c76df37a82b5ccad890d14 (cherry picked from commit 67e616b2380d6549308a15077b2043721dbea5d0) --- .../ovn/mech_driver/ovsdb/ovsdb_monitor.py | 66 ++++++++++--------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py index 87e1fecb564..ca2698b441c 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovsdb_monitor.py @@ -718,39 +718,43 @@ def handle_db_schema_changes(self, event, row): self.driver.agent_chassis_table = 'Chassis_Private' def notify(self, event, row, updates=None): - self.handle_db_schema_changes(event, row) - self.notify_handler.notify(event, row, updates, global_=True) try: - target_node = self._hash_ring.get_node(str(row.uuid)) - except exceptions.HashRingIsEmpty as e: - LOG.error('HashRing is empty, error: %s', e) - return - if target_node != self._node_uuid: - return - - # If the worker hasn't been health checked by the maintenance - # thread (see bug #1834498), indicate that it's alive here - time_now = timeutils.utcnow() - touch_timeout = time_now - datetime.timedelta( - seconds=ovn_const.HASH_RING_TOUCH_INTERVAL) - if not self._last_touch or touch_timeout >= self._last_touch: - # NOTE(lucasagomes): Guard the db operation with an exception - # handler. If heartbeating fails for whatever reason, log - # the error and continue with processing the event + self.handle_db_schema_changes(event, row) + self.notify_handler.notify(event, row, updates, global_=True) try: - ctx = neutron_context.get_admin_context() - ovn_hash_ring_db.touch_node(ctx, self._node_uuid) - self._last_touch = time_now - except Exception: - LOG.exception('Hash Ring node %s failed to heartbeat', - self._node_uuid) - - LOG.debug('Hash Ring: Node %(node)s (host: %(hostname)s) ' - 'handling event "%(event)s" for row %(row)s ' - '(table: %(table)s)', - {'node': self._node_uuid, 'hostname': CONF.host, - 'event': event, 'row': row.uuid, 'table': row._table.name}) - self.notify_handler.notify(event, row, updates) + target_node = self._hash_ring.get_node(str(row.uuid)) + except exceptions.HashRingIsEmpty as e: + LOG.error('HashRing is empty, error: %s', e) + return + if target_node != self._node_uuid: + return + + # If the worker hasn't been health checked by the maintenance + # thread (see bug #1834498), indicate that it's alive here + time_now = timeutils.utcnow() + touch_timeout = time_now - datetime.timedelta( + seconds=ovn_const.HASH_RING_TOUCH_INTERVAL) + if not self._last_touch or touch_timeout >= self._last_touch: + # NOTE(lucasagomes): Guard the db operation with an exception + # handler. If heartbeating fails for whatever reason, log + # the error and continue with processing the event + try: + ctx = neutron_context.get_admin_context() + ovn_hash_ring_db.touch_node(ctx, self._node_uuid) + self._last_touch = time_now + except Exception: + LOG.exception('Hash Ring node %s failed to heartbeat', + self._node_uuid) + + LOG.debug('Hash Ring: Node %(node)s (host: %(hostname)s) ' + 'handling event "%(event)s" for row %(row)s ' + '(table: %(table)s)', + {'node': self._node_uuid, 'hostname': CONF.host, + 'event': event, 'row': row.uuid, + 'table': row._table.name}) + self.notify_handler.notify(event, row, updates) + except Exception as e: + LOG.exception(e) @abc.abstractmethod def post_connect(self):