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/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/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): 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/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()) 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". 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: 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