From 770914f98df9abda2b5b8ca2896ff05618ea0181 Mon Sep 17 00:00:00 2001 From: Lucas Alvares Gomes Date: Tue, 12 Sep 2023 11:36:59 +0100 Subject: [PATCH] [OVN] Enhanced external port scheduling This patch introduces a new configuration for OVN CMS Options called "enable-chassis-as-extport-host". This configuration can be used by ML2/OVN to identify nodes that are eligible for scheduling OVN's external ports. Prior to this patch, external ports were always scheduled on centralized networked nodes tagged with the "enable-chassis-as-gw" flag in the OVN CMS Options but, when it comes to deploying OpenStack on OpenShift requiring services such as the OVN Metadata Agent or DHCP Agent to serve those external ports and running them on control plane nodes are not ideal. This is where this patch comes handy allowing these ports to have more flexibility in where they are scheduled. The patch is also backward compatible and if the new configuration is not present on the OVN CMS Options, ML2/OVN will continue to schedule the external ports on nodes configured with the previous configuration like always. Documentation will be updated on a follow up patch. Closes-Bug: 2037294 Change-Id: Ic46d847e3aebfe543d5a7ab49d18d1f1abf1342e Signed-off-by: Lucas Alvares Gomes --- neutron/common/ovn/constants.py | 8 + neutron/common/ovn/utils.py | 180 +++++++++---- .../drivers/ovn/mech_driver/mech_driver.py | 16 ++ .../ml2/drivers/ovn/mech_driver/ovsdb/api.py | 22 ++ .../ovn/mech_driver/ovsdb/impl_idl_ovn.py | 24 +- .../ovn/mech_driver/ovsdb/maintenance.py | 3 +- .../ovn/mech_driver/ovsdb/ovn_client.py | 79 ++++-- neutron/tests/functional/base.py | 4 +- .../tests/functional/common/ovn/test_utils.py | 91 ++++++- neutron/tests/unit/common/ovn/test_utils.py | 33 ++- neutron/tests/unit/fake_resources.py | 7 +- .../ovn/mech_driver/ovsdb/test_maintenance.py | 4 +- .../ovn/mech_driver/test_mech_driver.py | 250 ++++++++++++++---- ...rnal-port-scheduling-a5419ac51d863087.yaml | 14 + 14 files changed, 603 insertions(+), 132 deletions(-) create mode 100644 releasenotes/notes/external-port-scheduling-a5419ac51d863087.yaml diff --git a/neutron/common/ovn/constants.py b/neutron/common/ovn/constants.py index 9f04d75f94f..0579987c762 100644 --- a/neutron/common/ovn/constants.py +++ b/neutron/common/ovn/constants.py @@ -69,6 +69,7 @@ OVN_ROUTER_PORT_GW_MTU_OPTION = 'gateway_mtu' OVN_PROVNET_PORT_NAME_PREFIX = 'provnet-' OVN_NAME_PREFIX = 'neutron-' +OVN_HA_CH_GROUP_EXTPORT_PREFIX = 'neutron-extport-' # TODO(froyo): Move this to neutron-lib as soon as possible, and when a new # release is created and pointed to in the requirements remove this code @@ -307,6 +308,12 @@ HASH_RING_ML2_GROUP = 'mechanism_driver' # Maximum chassis count where a gateway port can be hosted MAX_GW_CHASSIS = 5 +# Maximum number of Chassis in a HA Chassis Group. Limiting the number +# of members because OVN uses BFD to monitor the connectivity of each member +# in the group. Having an unlimited number of members can potentially +# put a lot of stress on OVN to monitor it all. +MAX_CHASSIS_IN_HA_GROUP = 5 + UNKNOWN_ADDR = 'unknown' PORT_CAP_SWITCHDEV = 'switchdev' @@ -424,6 +431,7 @@ EXTERNAL_PORT_TYPES = (portbindings.VNIC_DIRECT, NEUTRON_AVAILABILITY_ZONES = 'neutron-availability-zones' OVN_CMS_OPTIONS = 'ovn-cms-options' CMS_OPT_CHASSIS_AS_GW = 'enable-chassis-as-gw' +CMS_OPT_CHASSIS_AS_EXTPORT_HOST = 'enable-chassis-as-extport-host' CMS_OPT_AVAILABILITY_ZONES = 'availability-zones' CMS_OPT_CARD_SERIAL_NUMBER = 'card-serial-number' diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index 8613a91200d..0cabff93aa7 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -35,7 +35,7 @@ from oslo_log import log from oslo_serialization import jsonutils from oslo_utils import netutils from oslo_utils import strutils -from ovsdbapp.backend.ovs_idl import rowview +from ovsdbapp.backend.ovs_idl import idlutils from ovsdbapp import constants as ovsdbapp_const import tenacity @@ -61,6 +61,10 @@ PortExtraDHCPValidation = collections.namedtuple( BPInfo = collections.namedtuple( 'BPInfo', ['bp_param', 'vnic_type', 'capabilities']) +HAChassisGroupInfo = collections.namedtuple( + 'HAChassisGroupInfo', ['group_name', 'chassis_list', 'az_hints', + 'ignore_chassis']) + class OvsdbClientCommand(object): _CONNECTION = 0 @@ -159,6 +163,11 @@ def ovn_provnet_port_name(network_id): return constants.OVN_PROVNET_PORT_NAME_PREFIX + '%s' % network_id +def ovn_extport_chassis_group_name(port_id): + # The name of the HA Chassis Group entry will be neutron-extport- + return constants.OVN_HA_CH_GROUP_EXTPORT_PREFIX + '%s' % port_id + + def ovn_vhu_sockpath(sock_dir, port_id): # Frame the socket path of a virtio socket return os.path.join( @@ -703,6 +712,12 @@ def is_gateway_chassis(chassis): return constants.CMS_OPT_CHASSIS_AS_GW in get_ovn_cms_options(chassis) +def is_extport_host_chassis(chassis): + """Check if the given Chassis is marked to host external ports""" + return (constants.CMS_OPT_CHASSIS_AS_EXTPORT_HOST in + get_ovn_cms_options(chassis)) + + def get_port_capabilities(port): """Return a list of port's capabilities""" return port.get(portbindings.PROFILE, {}).get(constants.PORT_CAP_PARAM, []) @@ -756,7 +771,7 @@ def get_chassis_in_azs(chassis_list, az_list): return chassis -def get_gateway_chassis_without_azs(chassis_list): +def get_chassis_without_azs(chassis_list): """Return a set of Chassis that does not belong to any AZs. Filter a list of Chassis and return only the Chassis that does not @@ -765,7 +780,7 @@ def get_gateway_chassis_without_azs(chassis_list): :param chassis_list: A list of Chassis objects :returns: A set of Chassis names """ - return {ch.name for ch in chassis_list if is_gateway_chassis(ch) and not + return {ch.name for ch in chassis_list if not get_chassis_availability_zones(ch)} @@ -936,77 +951,150 @@ def get_subnets_address_scopes(context, subnets, fixed_ips, ml2_plugin): return address4_scope_id, address6_scope_id -def sync_ha_chassis_group(context, network_id, nb_idl, sb_idl, txn): +def _get_info_for_ha_chassis_group(context, port_id, network_id, sb_idl): + """Get the common required information to create a HA Chassis Group. + + :param context: Neutron API context + :param port_id: The port ID + :param network_id: The network ID + :param sb_idl: OVN SB IDL + :returns: An instance of HAChassisGroupInfo + """ + ignore_chassis = set() + # If there are Chassis marked for hosting external ports create a HA + # Chassis Group per external port, otherwise do it at the network level + chassis_list = sb_idl.get_extport_chassis_from_cms_options() + if chassis_list: + group_name = ovn_extport_chassis_group_name(port_id) + # Check if the port is bound to a chassis and if so, ignore that + # chassis when building the HA Chassis Group to ensure the + # external port is bound to a different chassis than the VM + ignore_chassis = sb_idl.get_chassis_host_for_port(port_id) + LOG.debug('HA Chassis Group %s is based on external port %s ' + '(network %s)', group_name, port_id, network_id) + else: + chassis_list = sb_idl.get_gateway_chassis_from_cms_options( + name_only=False) + group_name = ovn_name(network_id) + LOG.debug('HA Chassis Group %s is based on network %s', + group_name, network_id) + + # Get the Availability Zones hints + plugin = directory.get_plugin() + az_hints = common_utils.get_az_hints( + plugin.get_network(context, network_id)) + + return HAChassisGroupInfo( + group_name=group_name, chassis_list=chassis_list, az_hints=az_hints, + ignore_chassis=ignore_chassis) + + +def _filter_candidates_for_ha_chassis_group(hcg_info): + """Filter a list of chassis candidates for a given HA Chassis Group. + + Filter a list of chassis candidates for a given HA Chassis Group taking + in consideration availability zones if present. + + :param hcg_info: A instance of HAChassisGroupInfo + :returns: A list of chassis + """ + if hcg_info.az_hints: + candidates = get_chassis_in_azs(hcg_info.chassis_list, + hcg_info.az_hints) + LOG.debug('Taking in consideration the AZs "%s" for HA ' + 'Chassis Group %s', ','.join(hcg_info.az_hints), + hcg_info.group_name) + else: + candidates = get_chassis_without_azs(hcg_info.chassis_list) + + # Remove the ignored Chassis, if present + if hcg_info.ignore_chassis: + LOG.debug('Ignoring chassis %s for HA Chassis Group %s', + ', '.join(hcg_info.ignore_chassis), hcg_info.group_name) + candidates = candidates - hcg_info.ignore_chassis + + return candidates + + +def sync_ha_chassis_group(context, port_id, network_id, nb_idl, sb_idl, txn): """Return the UUID of the HA Chassis Group or the HA Chassis Group cmd. Given the Neutron Network ID, this method will return (or create and then return) the appropriate HA Chassis Group the external port (in that network) needs to be associated with. - :param context: Neutron API context. - :param network_id: The Neutron network ID. + :param context: Neutron API context + :param port_id: The port ID + :param network_id: The network ID :param nb_idl: OVN NB IDL :param sb_idl: OVN SB IDL - :param txn: The ovsdbapp transaction object. - :returns: The HA Chassis Group UUID or the HA Chassis Group command object. + :param txn: The ovsdbapp transaction object + :returns: The HA Chassis Group UUID or the HA Chassis Group command object """ - plugin = directory.get_plugin() - az_hints = common_utils.get_az_hints( - plugin.get_network(context, network_id)) + # If there are Chassis marked for hosting external ports create a HA + # Chassis Group per external port, otherwise do it at the network level + hcg_info = _get_info_for_ha_chassis_group(context, port_id, network_id, + sb_idl) + candidates = _filter_candidates_for_ha_chassis_group(hcg_info) - ha_ch_grp_name = ovn_name(network_id) - ext_ids = {constants.OVN_AZ_HINTS_EXT_ID_KEY: ','.join(az_hints)} - hcg_cmd = txn.add(nb_idl.ha_chassis_group_add( - ha_ch_grp_name, may_exist=True, external_ids=ext_ids)) - - if isinstance(hcg_cmd.result, rowview.RowView): - # The HA chassis group existed before this transaction. - ha_ch_grp = hcg_cmd.result - else: - # The HA chassis group is being created in this transaction. - ha_ch_grp = None - - # Get the chassis belonging to the AZ hints - ch_list = sb_idl.get_gateway_chassis_from_cms_options(name_only=False) - if not az_hints: - az_chassis = get_gateway_chassis_without_azs(ch_list) - else: - az_chassis = get_chassis_in_azs(ch_list, az_hints) + # Try to get the HA Chassis Group or create if it doesn't exist + ha_ch_grp = ha_ch_grp_cmd = None + try: + ha_ch_grp = nb_idl.ha_chassis_group_get( + hcg_info.group_name).execute(check_error=True) + except idlutils.RowNotFound: + ext_ids = {constants.OVN_AZ_HINTS_EXT_ID_KEY: ','.join( + hcg_info.az_hints)} + ha_ch_grp_cmd = txn.add(nb_idl.ha_chassis_group_add( + hcg_info.group_name, may_exist=True, external_ids=ext_ids)) + max_chassis_number = constants.MAX_CHASSIS_IN_HA_GROUP priority = constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY + + # Check if the HA Chassis Group existed before. If so, re-calculate + # the canditates in case something changed and keep the highest priority + # chassis in the group (if it's an eligible candidate) with the highest + # priority to avoid external ports from moving around if ha_ch_grp: # Remove any chassis that no longer belongs to the AZ hints + # or is ignored all_ch = {ch.chassis_name for ch in ha_ch_grp.ha_chassis} - ch_to_del = all_ch - az_chassis + ch_to_del = all_ch - candidates for ch in ch_to_del: txn.add(nb_idl.ha_chassis_group_del_chassis( - ha_ch_grp_name, ch, if_exists=True)) + hcg_info.group_name, ch, if_exists=True)) - # Find the highest priority chassis in the HA Chassis Group. If - # it exists and still belongs to the same AZ, keep it as the - # highest priority in the group to avoid ports already bond to it - # from moving to another chassis. + # Find the highest priority chassis in the HA Chassis Group high_prio_ch = max(ha_ch_grp.ha_chassis, key=lambda x: x.priority, default=None) - priority = constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY - if high_prio_ch and high_prio_ch.chassis_name in az_chassis: + if (high_prio_ch and + high_prio_ch.chassis_name in candidates): + # If found, keep it as the highest priority chassis in the group txn.add(nb_idl.ha_chassis_group_add_chassis( - ha_ch_grp_name, high_prio_ch.chassis_name, + hcg_info.group_name, high_prio_ch.chassis_name, priority=priority)) - az_chassis.remove(high_prio_ch.chassis_name) + candidates.remove(high_prio_ch.chassis_name) priority -= 1 + max_chassis_number -= 1 + LOG.debug('Keeping chassis %s as the highest priority chassis ' + 'for HA Chassis Group %s', high_prio_ch.chassis_name, + hcg_info.group_name) - # Randomize the order so that networks belonging to the same - # availability zones do not necessarily end up with the same - # Chassis as the highest priority one. - for ch in random.sample(list(az_chassis), len(az_chassis)): + # random.sample() second parameter needs to be <= the list size, + # that's why we need to check for the max value here + max_chassis_number = min(max_chassis_number, len(candidates)) + # Limit the number of members and randomize the order so each group, + # even if they belonging to the same availability zones do not + # necessarily end up with the same Chassis as the highest priority one. + for ch in random.sample(list(candidates), max_chassis_number): txn.add(nb_idl.ha_chassis_group_add_chassis( - hcg_cmd, ch, priority=priority)) + hcg_info.group_name, ch, priority=priority)) priority -= 1 + LOG.info('HA Chassis Group %s synchronized', hcg_info.group_name) # Return the existing register UUID or the HA chassis group creation # command (see ovsdbapp ``HAChassisGroupAddChassisCommand`` class). - return ha_ch_grp.uuid if ha_ch_grp else hcg_cmd + return ha_ch_grp.uuid if ha_ch_grp else ha_ch_grp_cmd def get_port_type_virtual_and_parents(subnets, fixed_ips, network_id, port_id, @@ -1124,5 +1212,7 @@ def get_requested_chassis(requested_chassis): return [] +# TODO(lucasagomes): Remove this function when the additional_chassis column +# becomes the norm and older versions of OVN are no longer supported def is_additional_chassis_supported(idl): return idl.is_col_present('Port_Binding', 'additional_chassis') diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py index 7be585c9aab..b8011f446cd 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -1163,6 +1163,22 @@ class OVNMechanismDriver(api.MechanismDriver): LOG.debug('Port not found during OVN status up report: %s', port_id) + # NOTE(lucasagomes): If needed, re-sync the HA Chassis Group for + # the external port removing the chassis which the port is bound + # to from the group so the external port does not live in the + # same chassis as the VM + if (ovn_utils.is_port_external(db_port) and + self.sb_ovn.get_extport_chassis_from_cms_options()): + try: + with self.nb_ovn.transaction(check_error=True) as txn: + ovn_utils.sync_ha_chassis_group( + admin_context, db_port['id'], db_port['network_id'], + self.nb_ovn, self.sb_ovn, txn) + except Exception as e: + LOG.error('Error while syncing the HA Chassis Group for the ' + 'external port %s during set port status up. ' + 'Error: %s', db_port['id'], e) + def set_port_status_down(self, port_id): # Port provisioning is required now that OVN has reported that the # port is down. Insert a provisioning block and mark the port down diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py index 4bd2774f782..2f05b2e68bd 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py @@ -663,6 +663,20 @@ class SbAPI(api.API, metaclass=abc.ABCMeta): :returns: List with chassis. """ + @abc.abstractmethod + def get_extport_chassis_from_cms_options(self): + """Get chassis eligible for hosting external ports from CMS options. + + When admin wants to enable hosting external ports on different + chassis than gateway chassis as + + ovs-vsctl set open . + external_ids:ovn-cms-options="enable-chassis-as-extport-host" + In this function, we parse ovn-cms-options and return these chassis + + :returns: List with chassis + """ + @abc.abstractmethod def get_chassis_and_physnets(self): """Return a dict contains chassis name and physnets mapping. @@ -678,3 +692,11 @@ class SbAPI(api.API, metaclass=abc.ABCMeta): :param chassis_type: The type of chassis :type chassis_type: string """ + + @abc.abstractmethod + def get_chassis_host_for_port(self, port_id): + """Return a list of Chassis name hosting the port + + :param port_id: The port ID + :type port_id: string + """ diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py index bb6587e8f4c..c788a54711b 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py @@ -891,9 +891,11 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend): def get_gateway_chassis_from_cms_options(self, name_only=True): return [ch.name if name_only else ch for ch in self.chassis_list().execute(check_error=True) - if ovn_const.CMS_OPT_CHASSIS_AS_GW in - utils.get_ovn_chassis_other_config(ch).get( - ovn_const.OVN_CMS_OPTIONS, '').split(',')] + if utils.is_gateway_chassis(ch)] + + def get_extport_chassis_from_cms_options(self): + return [ch for ch in self.chassis_list().execute(check_error=True) + if utils.is_extport_host_chassis(ch)] def get_chassis_and_physnets(self): chassis_info_dict = {} @@ -983,3 +985,19 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend): else: return [r for r in rows if r.chassis and r.chassis[0].name == chassis] + + def get_chassis_host_for_port(self, port_id): + chassis = set() + cmd = self.db_find_rows('Port_Binding', ('logical_port', '=', port_id)) + for row in cmd.execute(check_error=True): + try: + chassis.add(row.chassis[0].name) + except IndexError: + # Do not short-circuit here. Proceed to additional + # chassis handling + pass + + if utils.is_additional_chassis_supported(self): + for ch in row.additional_chassis: + chassis.add(ch.name) + return chassis diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py index 7c1407192f5..b1ca2a0f889 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -599,7 +599,8 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY].replace( ovn_const.OVN_NAME_PREFIX, '') ha_ch_grp = utils.sync_ha_chassis_group( - context, network_id, self._nb_idl, self._sb_idl, txn) + context, port.name, network_id, self._nb_idl, + self._sb_idl, txn) txn.add(self._nb_idl.set_lswitch_port( port.name, ha_chassis_group=ha_ch_grp)) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py index a4f42320ace..8a49acdf37f 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py @@ -550,8 +550,8 @@ class OVNClient(object): if (self.is_external_ports_supported() and port_info.type == ovn_const.LSP_TYPE_EXTERNAL): kwargs['ha_chassis_group'] = utils.sync_ha_chassis_group( - context, port['network_id'], self._nb_idl, self._sb_idl, - txn) + context, port['id'], port['network_id'], self._nb_idl, + self._sb_idl, txn) # NOTE(mjozefcz): Do not set addresses if the port is not # bound, has no device_owner and it is OVN LB VIP port. @@ -673,8 +673,8 @@ class OVNClient(object): if port_info.type == ovn_const.LSP_TYPE_EXTERNAL: columns_dict['ha_chassis_group'] = ( utils.sync_ha_chassis_group( - context, port['network_id'], self._nb_idl, - self._sb_idl, txn)) + context, port['id'], port['network_id'], + self._nb_idl, self._sb_idl, txn)) else: # Clear the ha_chassis_group field columns_dict['ha_chassis_group'] = [] @@ -776,6 +776,15 @@ class OVNClient(object): ovn_const.LSP_OPTIONS_VIRTUAL_PARENTS_KEY, ''): txn.add(cmd(lsp.name, port_id, if_exists=True)) + # NOTE(lucasagomes): We need to delete the LSP before we attempt + # to remove the HA Chassis Group or it will fail with a violation + # error due to the LSP reference in the group + with self._nb_idl.transaction(check_error=True) as txn: + if ovn_port.type == ovn_const.LSP_TYPE_EXTERNAL: + ha_ch_grp_name = utils.ovn_extport_chassis_group_name(port_id) + txn.add(self._nb_idl.ha_chassis_group_del( + ha_ch_grp_name, if_exists=True)) + # TODO(lucasagomes): The ``port_object`` parameter was added to # keep things backward compatible. Remove it in the Rocky release. def delete_port(self, context, port_id, port_object=None): @@ -1961,6 +1970,52 @@ class OVNClient(object): commands.append(self._nb_idl.lrp_set_options(lrp_name, **options)) self._transaction(commands, txn=txn) + def _check_network_changes_in_ha_chassis_groups(self, + context, lswitch, lswitch_params, txn): + """Check for changes in the HA Chassis Groups. + + Check for changes in the HA Chassis Groups upon a network update. + """ + # If there are no external ports in this network, there's + # no need to check the AZs + if self.is_external_ports_supported(): + return + + # Check for changes in the network Availability Zones + ovn_ls_azs = lswitch.external_ids.get( + ovn_const.OVN_AZ_HINTS_EXT_ID_KEY, '') + neutron_net_azs = lswitch_params['external_ids'].get( + ovn_const.OVN_AZ_HINTS_EXT_ID_KEY, '') + + # Check if there are changes to the AZs + if ovn_ls_azs != neutron_net_azs: + return + + extport_list = [p for p in lswitch.ports if + p.type == ovn_const.LSP_TYPE_EXTERNAL] + + # Check if there are dedicated chassis for external ports + if self._sb_idl.get_extport_chassis_from_cms_options(): + for extport in extport_list: + port_id = extport.name + network_id = extport.external_ids[ + ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY].replace( + ovn_const.OVN_NAME_PREFIX, '') + utils.sync_ha_chassis_group( + context, port_id, network_id, self._nb_idl, + self._sb_idl, txn) + elif extport_list: + # If there's no dedicated chassis for external ports, there will + # be 1 HA Chassis Group per network, so the sync is at the network + # level. Just pass any external port from that network to the + # sync method + port_id = extport_list[0].name + network_id = extport_list[0].external_ids[ + ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY].replace( + ovn_const.OVN_NAME_PREFIX, '') + utils.sync_ha_chassis_group( + context, port_id, network_id, self._nb_idl, self._sb_idl, txn) + def update_network(self, context, network, original_network=None): lswitch_name = utils.ovn_name(network['id']) check_rev_cmd = self._nb_idl.check_revision_number( @@ -2016,20 +2071,8 @@ class OVNClient(object): self.set_gateway_mtu(n_context.get_admin_context(), network, txn) - if self.is_external_ports_supported(): - # If there are no external ports in this network, there's - # no need to check the AZs - if any(p for p in lswitch.ports if - p.type == ovn_const.LSP_TYPE_EXTERNAL): - # Check for changes in the network Availability Zones - ovn_ls_azs = lswitch.external_ids.get( - ovn_const.OVN_AZ_HINTS_EXT_ID_KEY, '') - neutron_net_azs = lswitch_params['external_ids'].get( - ovn_const.OVN_AZ_HINTS_EXT_ID_KEY, '') - if ovn_ls_azs != neutron_net_azs: - utils.sync_ha_chassis_group( - context, network['id'], self._nb_idl, - self._sb_idl, txn) + self._check_network_changes_in_ha_chassis_groups( + context, lswitch, lswitch_params, txn) # Update the segment tags, if any segments = segments_db.get_network_segments(context, network['id']) diff --git a/neutron/tests/functional/base.py b/neutron/tests/functional/base.py index 7b456e8229b..79875f14851 100644 --- a/neutron/tests/functional/base.py +++ b/neutron/tests/functional/base.py @@ -396,7 +396,7 @@ class TestOVNFunctionalBase(test_plugin.Ml2PluginV2TestCase, def add_fake_chassis(self, host, physical_nets=None, external_ids=None, name=None, azs=None, enable_chassis_as_gw=False, - other_config=None): + enable_chassis_as_extport=False, other_config=None): def append_cms_options(ext_ids, value): if 'ovn-cms-options' not in ext_ids: ext_ids['ovn-cms-options'] = value @@ -413,6 +413,8 @@ class TestOVNFunctionalBase(test_plugin.Ml2PluginV2TestCase, other_config['ovn-cms-options'] += ':'.join(azs) if enable_chassis_as_gw: append_cms_options(other_config, 'enable-chassis-as-gw') + if enable_chassis_as_extport: + append_cms_options(other_config, 'enable-chassis-as-extport-host') bridge_mapping = ",".join(["%s:br-provider%s" % (phys_net, i) for i, phys_net in enumerate(physical_nets)]) diff --git a/neutron/tests/functional/common/ovn/test_utils.py b/neutron/tests/functional/common/ovn/test_utils.py index abb05ca5c81..88d16c11988 100644 --- a/neutron/tests/functional/common/ovn/test_utils.py +++ b/neutron/tests/functional/common/ovn/test_utils.py @@ -12,6 +12,9 @@ # License for the specific language governing permissions and limitations # under the License. +from neutron_lib.api.definitions import portbindings +from ovsdbapp.backend.ovs_idl import idlutils + from neutron.common.ovn import constants as ovn_const from neutron.common.ovn import utils from neutron.tests.functional import base @@ -64,6 +67,7 @@ class TestSyncHaChassisGroup(base.TestOVNFunctionalBase): def test_sync_ha_chassis_group(self): net = self._make_network(self.fmt, 'n1', True)['network'] + port_id = 'fake-port-id' hcg_name = utils.ovn_name(net['id']) chassis1 = self.add_fake_chassis('host1', azs=[], enable_chassis_as_gw=True) @@ -72,8 +76,9 @@ class TestSyncHaChassisGroup(base.TestOVNFunctionalBase): self.add_fake_chassis('host3') with self.nb_api.transaction(check_error=True) as txn: - utils.sync_ha_chassis_group(self.context, net['id'], self.nb_api, - self.sb_api, txn) + utils.sync_ha_chassis_group( + self.context, port_id, net['id'], self.nb_api, + self.sb_api, txn) ha_chassis = self.nb_api.db_find('HA_Chassis').execute( check_error=True) @@ -94,8 +99,9 @@ class TestSyncHaChassisGroup(base.TestOVNFunctionalBase): # HA Chassis Group register but will update the "ha_chassis" list. self.del_fake_chassis(chassis2) with self.nb_api.transaction(check_error=True) as txn: - utils.sync_ha_chassis_group(self.context, net['id'], self.nb_api, - self.sb_api, txn) + utils.sync_ha_chassis_group( + self.context, port_id, net['id'], self.nb_api, + self.sb_api, txn) ha_chassis = self.nb_api.db_find('HA_Chassis').execute( check_error=True) @@ -109,3 +115,80 @@ class TestSyncHaChassisGroup(base.TestOVNFunctionalBase): ha_chassis_exp = str(ha_chassis[0]['_uuid']) ha_chassis_ret = str(hcg.ha_chassis[0].uuid) self.assertEqual(ha_chassis_exp, ha_chassis_ret) + + def test_sync_ha_chassis_group_extport(self): + # Create a network and an external port + net = self._make_network(self.fmt, 'n1', True)['network'] + port_data = { + 'port': {'network_id': net['id'], + 'tenant_id': self._tenant_id, + portbindings.VNIC_TYPE: portbindings.VNIC_DIRECT}} + port_req = self.new_create_request('ports', port_data, self.fmt) + port_res = port_req.get_response(self.api) + port = self.deserialize(self.fmt, port_res)['port'] + + # Add 3 chassis, two eligible for hosting the external port + chassis1 = self.add_fake_chassis('host1', azs=[], + enable_chassis_as_extport=True) + chassis2 = self.add_fake_chassis('host2', azs=[], + enable_chassis_as_extport=True) + self.add_fake_chassis('host3') + + # Invoke the sync method + with self.nb_api.transaction(check_error=True) as txn: + utils.sync_ha_chassis_group( + self.context, port['id'], net['id'], self.nb_api, + self.sb_api, txn) + + # Assert only the eligible chassis are present in HA Chassis + ha_chassis = self.nb_api.db_find('HA_Chassis').execute( + check_error=True) + ha_chassis_names = [hc['chassis_name'] for hc in ha_chassis] + self.assertEqual(2, len(ha_chassis)) + self.assertEqual(sorted([chassis1, chassis2]), + sorted(ha_chassis_names)) + + # Assert the HA Chassis Group has the correct name and the + # eligible chassis are included in it + hcg_name = utils.ovn_extport_chassis_group_name(port['id']) + hcg = self.nb_api.ha_chassis_group_get(hcg_name).execute( + check_error=True) + self.assertEqual(hcg_name, hcg.name) + ha_chassis_exp = sorted([str(hc['_uuid']) for hc in ha_chassis]) + ha_chassis_ret = sorted([str(hc.uuid) for hc in hcg.ha_chassis]) + self.assertEqual(ha_chassis_exp, ha_chassis_ret) + + # Delete one eligible Chassis and resync the HA chassis group + # associated to the external port. The method should not re-create + # the existing HA Chassis Group but only update the "ha_chassis" list + self.del_fake_chassis(chassis2) + with self.nb_api.transaction(check_error=True) as txn: + utils.sync_ha_chassis_group( + self.context, port['id'], net['id'], self.nb_api, + self.sb_api, txn) + + # Assert the chassis deletion reflects in the HA Chassis and + # HA Chassis Group + ha_chassis = self.nb_api.db_find('HA_Chassis').execute( + check_error=True) + ha_chassis_names = [hc['chassis_name'] for hc in ha_chassis] + self.assertEqual(1, len(ha_chassis)) + self.assertEqual([chassis1], ha_chassis_names) + + hcg = self.nb_api.ha_chassis_group_get(hcg_name).execute( + check_error=True) + self.assertEqual(hcg_name, hcg.name) + ha_chassis_exp = str(ha_chassis[0]['_uuid']) + ha_chassis_ret = str(hcg.ha_chassis[0].uuid) + self.assertEqual(ha_chassis_exp, ha_chassis_ret) + + # Delete the external port, assert that the HA Chassis and HA Chassis + # Group were also deleted + self.plugin.delete_port(self.context, port['id']) + ha_chassis = self.nb_api.db_find('HA_Chassis').execute( + check_error=True) + self.assertEqual(0, len(ha_chassis)) + self.assertRaises( + idlutils.RowNotFound, + self.nb_api.ha_chassis_group_get(hcg_name).execute, + check_error=True) diff --git a/neutron/tests/unit/common/ovn/test_utils.py b/neutron/tests/unit/common/ovn/test_utils.py index dbe8e252ff3..ee9c7c92ba2 100644 --- a/neutron/tests/unit/common/ovn/test_utils.py +++ b/neutron/tests/unit/common/ovn/test_utils.py @@ -60,17 +60,30 @@ class TestUtils(base.BaseTestCase): resolver_file=resolver_file_name) self.assertEqual(expected_dns_resolvers, observed_dns_resolvers) - def test_is_gateway_chassis(self): + def _test_is_chassis(self, is_gateway=False, is_extport=False): + if is_gateway: + cms_option_value = constants.CMS_OPT_CHASSIS_AS_GW + func = utils.is_gateway_chassis + if is_extport: + cms_option_value = constants.CMS_OPT_CHASSIS_AS_EXTPORT_HOST + func = utils.is_extport_host_chassis + chassis = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={ - 'other_config': {'ovn-cms-options': 'enable-chassis-as-gw'}}) - non_gw_chassis_0 = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={ + 'other_config': {'ovn-cms-options': cms_option_value}}) + wrong_chassis_0 = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={ 'other_config': {'ovn-cms-options': ''}}) - non_gw_chassis_1 = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={ + wrong_chassis_1 = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={ 'other_config': {}}) - self.assertTrue(utils.is_gateway_chassis(chassis)) - self.assertFalse(utils.is_gateway_chassis(non_gw_chassis_0)) - self.assertFalse(utils.is_gateway_chassis(non_gw_chassis_1)) + self.assertTrue(func(chassis)) + self.assertFalse(func(wrong_chassis_0)) + self.assertFalse(func(wrong_chassis_1)) + + def test_is_gateway_chassis(self): + self._test_is_chassis(is_gateway=True) + + def test_is_extport_host_chassis(self): + self._test_is_chassis(is_extport=True) def test_get_chassis_availability_zones_no_azs(self): chassis = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={ @@ -178,7 +191,7 @@ class TestUtils(base.BaseTestCase): set(), utils.get_chassis_in_azs(chassis_list, ['az6'])) - def test_get_gateway_chassis_without_azs(self): + def test_get_chassis_without_azs(self): ch0 = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={ 'name': 'ch0', 'other_config': { @@ -199,8 +212,8 @@ class TestUtils(base.BaseTestCase): chassis_list = [ch0, ch1, ch2, ch3] self.assertEqual( - {'ch1'}, - utils.get_gateway_chassis_without_azs(chassis_list)) + {'ch1', 'ch3'}, + utils.get_chassis_without_azs(chassis_list)) def test_is_ovn_metadata_port(self): meta_port = { diff --git a/neutron/tests/unit/fake_resources.py b/neutron/tests/unit/fake_resources.py index 031ab2b632a..d55fd0ae862 100644 --- a/neutron/tests/unit/fake_resources.py +++ b/neutron/tests/unit/fake_resources.py @@ -182,6 +182,7 @@ class FakeOvsdbSbOvnIdl(object): self._get_chassis_physnets.return_value = ['fake-physnet'] self.get_chassis_and_physnets = mock.Mock() self.get_gateway_chassis_from_cms_options = mock.Mock() + self.get_extport_chassis_from_cms_options = mock.Mock(return_value=[]) self.is_col_present = mock.Mock() self.is_col_present.return_value = False self.db_set = mock.Mock() @@ -191,6 +192,7 @@ class FakeOvsdbSbOvnIdl(object): self.is_table_present.return_value = False self.get_chassis_by_card_serial_from_cms_options = mock.Mock() self.get_schema_version = mock.Mock(return_value='3.6.0') + self.get_chassis_host_for_port = mock.Mock(return_value=set()) class FakeOvsdbTransaction(object): @@ -858,7 +860,7 @@ class FakeChassis(object): def create(attrs=None, az_list=None, chassis_as_gw=False, bridge_mappings=None, rp_bandwidths=None, rp_inventory_defaults=None, rp_hypervisors=None, - card_serial_number=None): + card_serial_number=None, chassis_as_extport=False): cms_opts = [] if az_list: cms_opts.append("%s=%s" % (ovn_const.CMS_OPT_AVAILABILITY_ZONES, @@ -866,6 +868,9 @@ class FakeChassis(object): if chassis_as_gw: cms_opts.append(ovn_const.CMS_OPT_CHASSIS_AS_GW) + if chassis_as_extport: + cms_opts.append(ovn_const.CMS_OPT_CHASSIS_AS_EXTPORT_HOST) + if rp_bandwidths: cms_opts.append('%s=%s' % (n_const.RP_BANDWIDTHS, ';'.join(rp_bandwidths))) diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py index f73a66c325a..e66895da336 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py @@ -450,10 +450,10 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, # Assert sync_ha_chassis_group() is called for both networks expected_calls = [ - mock.call(mock.ANY, 'net0', + mock.call(mock.ANY, 'p0', 'net0', self.fake_ovn_client._nb_idl, self.fake_ovn_client._sb_idl, mock.ANY), - mock.call(mock.ANY, 'net1', + mock.call(mock.ANY, 'p1', 'net1', self.fake_ovn_client._nb_idl, self.fake_ovn_client._sb_idl, mock.ANY), ] diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 6d53548a619..0d6375413a2 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -1059,9 +1059,16 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase): port['port']['id'], ovn_const.TYPE_PORTS) - def _test_set_port_status_up(self, is_compute_port=False): + @mock.patch.object(ovn_utils, 'sync_ha_chassis_group') + @mock.patch.object(ovn_utils, 'is_port_external') + def _test_set_port_status_up(self, mock_is_ext, mock_sync, + is_compute_port=False, + is_extport_present=False): port_device_owner = 'compute:nova' if is_compute_port else '' self.mech_driver._plugin.nova_notifier = mock.Mock() + mock_is_ext.return_value = is_extport_present + self.sb_ovn.get_extport_chassis_from_cms_options.return_value = [ + mock.Mock()] with self.network() as net1, \ self.subnet(network=net1) as subnet1, \ self.port(subnet=subnet1, is_admin=True, @@ -1095,9 +1102,19 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase): ulsp.assert_called_once_with(mock.ANY, mock.ANY) + if is_extport_present: + mock_sync.assert_called_once_with( + mock.ANY, port1['port']['id'], port1['port']['network_id'], + self.nb_ovn, self.sb_ovn, mock.ANY) + else: + mock_sync.assert_not_called() + def test_set_port_status_up(self): self._test_set_port_status_up(is_compute_port=False) + def test_set_port_status_up_extport(self): + self._test_set_port_status_up(is_extport_present=True) + def test_set_compute_port_status_up(self): self._test_set_port_status_up(is_compute_port=True) @@ -2582,74 +2599,214 @@ class TestOVNMechanismDriver(TestOVNMechanismDriverBase): expected_candidates = [ch0.name, ch2.name] self.assertEqual(sorted(expected_candidates), sorted(candidates)) - def test_sync_ha_chassis_group(self): - fake_txn = mock.MagicMock() + def test__get_info_for_ha_chassis_group_as_extport(self): net_attrs = {az_def.AZ_HINTS: ['az0', 'az1', 'az2']} fake_net = ( fakes.FakeNetwork.create_one_network(attrs=net_attrs).info()) mock.patch.object(self.mech_driver._plugin, 'get_network', return_value=fake_net).start() + fake_port = fakes.FakePort.create_one_port().info() + fake_port['network_id'] = fake_net['id'] ch0 = fakes.FakeChassis.create(az_list=['az0', 'az1'], - chassis_as_gw=True) - ch1 = fakes.FakeChassis.create(az_list=['az2'], chassis_as_gw=True) - ch2 = fakes.FakeChassis.create(az_list=['az3'], chassis_as_gw=True) - ch3 = fakes.FakeChassis.create(az_list=[], chassis_as_gw=True) - ch4 = fakes.FakeChassis.create(az_list=[], chassis_as_gw=False) - self.sb_ovn.get_gateway_chassis_from_cms_options.return_value = [ - ch0, ch1, ch2, ch3, ch4] + chassis_as_extport=True) + ch1 = fakes.FakeChassis.create(az_list=['az2'], + chassis_as_extport=True) + ch2 = fakes.FakeChassis.create(az_list=['az3'], + chassis_as_extport=True) + ch3 = fakes.FakeChassis.create(az_list=[], chassis_as_extport=True) + ch4 = fakes.FakeChassis.create(az_list=['az0'], + chassis_as_extport=True) + ch5 = fakes.FakeChassis.create(az_list=['az0', 'az1'], + chassis_as_extport=True) + self.sb_ovn.get_extport_chassis_from_cms_options.return_value = [ + ch0, ch1, ch2, ch3, ch4, ch5] - # Invoke the method - hcg_cmd = ovn_utils.sync_ha_chassis_group( - self.context, fake_net['id'], self.nb_ovn, self.sb_ovn, fake_txn) + self.sb_ovn.get_chassis_host_for_port.return_value = { + ch4.name, ch5.name} - # Assert it attempts to add the chassis group for that network - ha_ch_grp_name = ovn_utils.ovn_name(fake_net['id']) - ext_ids = {ovn_const.OVN_AZ_HINTS_EXT_ID_KEY: 'az0,az1,az2'} - self.nb_ovn.ha_chassis_group_add.assert_called_once_with( - ha_ch_grp_name, may_exist=True, external_ids=ext_ids) + hcg_info = ovn_utils._get_info_for_ha_chassis_group( + self.context, fake_port['id'], fake_net['id'], self.sb_ovn) - # Assert that only Chassis belonging to the AZ hints are - # added to the HA Chassis Group for that network - expected_calls = [ - mock.call(hcg_cmd, ch0.name, priority=mock.ANY), - mock.call(hcg_cmd, ch1.name, priority=mock.ANY)] - self.nb_ovn.ha_chassis_group_add_chassis.assert_has_calls( - expected_calls, any_order=True) + expected_group_name = ovn_utils.ovn_extport_chassis_group_name( + fake_port['id']) + expected_ch_list = [ch0, ch1, ch2, ch3, ch4, ch5] + expected_az_hints = ['az0', 'az1', 'az2'] + expected_ignore_chassis = [ch4.name, ch5.name] + self.assertEqual(expected_group_name, hcg_info.group_name) + self.assertEqual(expected_ch_list, + hcg_info.chassis_list) + self.assertEqual(expected_az_hints, hcg_info.az_hints) + self.assertEqual(sorted(expected_ignore_chassis), + sorted(hcg_info.ignore_chassis)) - def test_sync_ha_chassis_group_no_az_hints(self): - fake_txn = mock.MagicMock() - # No AZ hints are specified for that network - net_attrs = {az_def.AZ_HINTS: []} + def test__get_info_for_ha_chassis_group_as_gw(self): + net_attrs = {az_def.AZ_HINTS: ['az0', 'az1', 'az2']} fake_net = ( fakes.FakeNetwork.create_one_network(attrs=net_attrs).info()) mock.patch.object(self.mech_driver._plugin, 'get_network', return_value=fake_net).start() + fake_port = fakes.FakePort.create_one_port().info() + fake_port['network_id'] = fake_net['id'] ch0 = fakes.FakeChassis.create(az_list=['az0', 'az1'], chassis_as_gw=True) - ch1 = fakes.FakeChassis.create(az_list=['az2'], chassis_as_gw=True) - ch2 = fakes.FakeChassis.create(az_list=[], chassis_as_gw=True) + ch1 = fakes.FakeChassis.create(az_list=['az2'], + chassis_as_gw=True) + ch2 = fakes.FakeChassis.create(az_list=['az3'], + chassis_as_gw=True) ch3 = fakes.FakeChassis.create(az_list=[], chassis_as_gw=True) - ch4 = fakes.FakeChassis.create(az_list=[], chassis_as_gw=False) + ch4 = fakes.FakeChassis.create(az_list=['az0'], + chassis_as_gw=True) + ch5 = fakes.FakeChassis.create(az_list=['az0', 'az1'], + chassis_as_gw=True) + self.sb_ovn.get_extport_chassis_from_cms_options.return_value = [] self.sb_ovn.get_gateway_chassis_from_cms_options.return_value = [ - ch0, ch1, ch2, ch3, ch4] + ch0, ch1, ch2, ch3, ch4, ch5] + + hcg_info = ovn_utils._get_info_for_ha_chassis_group( + self.context, fake_port['id'], fake_net['id'], self.sb_ovn) + + expected_group_name = ovn_utils.ovn_name(fake_net['id']) + expected_ch_list = [ch0, ch1, ch2, ch3, ch4, ch5] + expected_az_hints = ['az0', 'az1', 'az2'] + self.assertEqual(expected_group_name, hcg_info.group_name) + self.assertEqual(expected_ch_list, + hcg_info.chassis_list) + self.assertEqual(expected_az_hints, hcg_info.az_hints) + self.assertEqual(set(), hcg_info.ignore_chassis) + + def _build_hcg_info(self, with_az=False, with_ignore_chassis=False): + az_hints = [] + if with_az: + az_hints = ['az0', 'az1'] + ch0 = fakes.FakeChassis.create(attrs={'name': 'ch0'}, + az_list=['az0', 'az1']) + ch1 = fakes.FakeChassis.create(attrs={'name': 'ch1'}, + az_list=['az2']) + ch2 = fakes.FakeChassis.create(attrs={'name': 'ch2'}, + az_list=['az3', 'az0']) + ch3 = fakes.FakeChassis.create(attrs={'name': 'ch3'}, + az_list=['az1']) + else: + ch0 = fakes.FakeChassis.create(attrs={'name': 'ch0'}) + ch1 = fakes.FakeChassis.create(attrs={'name': 'ch1'}) + ch2 = fakes.FakeChassis.create(attrs={'name': 'ch2'}) + ch3 = fakes.FakeChassis.create(attrs={'name': 'ch3'}) + + chassis_list = [ch0, ch1, ch2, ch3] + + ignore_chassis = set() + if with_ignore_chassis: + ignore_chassis = {ch1.name, ch2.name} + + return ovn_utils.HAChassisGroupInfo( + group_name='fake-hcg-name', chassis_list=chassis_list, + az_hints=az_hints, ignore_chassis=ignore_chassis) + + def test__filter_candidates_for_ha_chassis_group(self): + fake_hcg_info = self._build_hcg_info() + candidates = ovn_utils._filter_candidates_for_ha_chassis_group( + fake_hcg_info) + self.assertEqual(['ch0', 'ch1', 'ch2', 'ch3'], sorted(candidates)) + + def test__filter_candidates_for_ha_chassis_group_with_az(self): + fake_hcg_info = self._build_hcg_info(with_az=True) + candidates = ovn_utils._filter_candidates_for_ha_chassis_group( + fake_hcg_info) + self.assertEqual(['ch0', 'ch2', 'ch3'], sorted(candidates)) + + def test__filter_candidates_for_ha_chassis_group_with_ignore_chassis(self): + fake_hcg_info = self._build_hcg_info(with_ignore_chassis=True) + candidates = ovn_utils._filter_candidates_for_ha_chassis_group( + fake_hcg_info) + self.assertEqual(['ch0', 'ch3'], sorted(candidates)) + + def test__filter_candidates_for_ha_chassis_group_az_and_ignore(self): + fake_hcg_info = self._build_hcg_info(with_az=True, + with_ignore_chassis=True) + candidates = ovn_utils._filter_candidates_for_ha_chassis_group( + fake_hcg_info) + self.assertEqual(['ch0', 'ch3'], sorted(candidates)) + + @mock.patch.object(ovn_utils, '_get_info_for_ha_chassis_group') + def test_sync_ha_chassis_group(self, mock_hcg_info): + self.nb_ovn.ha_chassis_group_get.side_effect = idlutils.RowNotFound + fake_txn = mock.Mock() + + hcg_info = self._build_hcg_info() + mock_hcg_info.return_value = hcg_info # Invoke the method - hcg_cmd = ovn_utils.sync_ha_chassis_group( - self.context, fake_net['id'], self.nb_ovn, self.sb_ovn, fake_txn) + ovn_utils.sync_ha_chassis_group( + self.context, 'fake-port-id', 'fake-net-id', + self.nb_ovn, self.sb_ovn, fake_txn) - # Assert it attempts to add the chassis group for that network - ha_ch_grp_name = ovn_utils.ovn_name(fake_net['id']) - ext_ids = {ovn_const.OVN_AZ_HINTS_EXT_ID_KEY: ''} + # Assert it creates the HA Chassis Group + ext_ids = {ovn_const.OVN_AZ_HINTS_EXT_ID_KEY: + ','.join(hcg_info.az_hints)} self.nb_ovn.ha_chassis_group_add.assert_called_once_with( - ha_ch_grp_name, may_exist=True, external_ids=ext_ids) + hcg_info.group_name, may_exist=True, external_ids=ext_ids) - # Assert that only Chassis that are gateways and DOES NOT - # belong to any AZs are added expected_calls = [ - mock.call(hcg_cmd, ch2.name, priority=mock.ANY), - mock.call(hcg_cmd, ch3.name, priority=mock.ANY)] + mock.call(hcg_info.group_name, 'ch0', priority=mock.ANY), + mock.call(hcg_info.group_name, 'ch1', priority=mock.ANY), + mock.call(hcg_info.group_name, 'ch2', priority=mock.ANY), + mock.call(hcg_info.group_name, 'ch3', priority=mock.ANY)] + self.nb_ovn.ha_chassis_group_add_chassis.assert_has_calls( + expected_calls, any_order=True) + + @mock.patch.object(ovn_utils, '_get_info_for_ha_chassis_group') + def test_sync_ha_chassis_group_existing_group(self, mock_hcg_info): + fake_txn = mock.Mock() + hcg_info = self._build_hcg_info() + mock_hcg_info.return_value = hcg_info + + hc0 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'chassis_name': 'ch0', 'priority': 1}) + # hc1 is the chassis with the highest priority in the group + hc1 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'chassis_name': 'ch1', 'priority': 99}) + # hc2 and hc3 are no longer valid candidates and should be removed + hc2 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'chassis_name': 'ch98', 'priority': 2}) + hc3 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'chassis_name': 'ch99', 'priority': 3}) + + hcg_attrs = { + 'name': hcg_info.group_name, + 'ha_chassis': [hc0, hc1, hc2, hc3]} + fake_ha_chassis_group = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs=hcg_attrs) + self.nb_ovn.ha_chassis_group_get().execute.return_value = ( + fake_ha_chassis_group) + + # Invoke the method + ovn_utils.sync_ha_chassis_group( + self.context, 'fake-port-id', 'fake-net-id', + self.nb_ovn, self.sb_ovn, fake_txn) + + # Assert the group was not re-created + self.nb_ovn.ha_chassis_group_add.assert_not_called() + + # Assert the chassis that are no longer part of the candidates list + # are removed from group + expected_calls = [ + mock.call(hcg_info.group_name, 'ch98', if_exists=True), + mock.call(hcg_info.group_name, 'ch99', if_exists=True)] + self.nb_ovn.ha_chassis_group_del_chassis.assert_has_calls( + expected_calls, any_order=True) + self.assertEqual( + 2, self.nb_ovn.ha_chassis_group_del_chassis.call_count) + + # Assert the candidates have been added to the group and ch1 + # was kept as the highest priority one + expected_calls = [ + mock.call(hcg_info.group_name, 'ch0', priority=mock.ANY), + mock.call(hcg_info.group_name, 'ch1', + priority=ovn_const.HA_CHASSIS_GROUP_HIGHEST_PRIORITY), + mock.call(hcg_info.group_name, 'ch2', priority=mock.ANY), + mock.call(hcg_info.group_name, 'ch3', priority=mock.ANY)] self.nb_ovn.ha_chassis_group_add_chassis.assert_has_calls( expected_calls, any_order=True) @@ -3968,9 +4125,8 @@ class TestOVNMechanismDriverSecurityGroup(MechDriverSetupBase, sync_mock.return_value = fake_grp with self.network() as n, self.subnet(n): - net_id = n['network']['id'] self._create_port( - self.fmt, net_id, + self.fmt, n['network']['id'], arg_list=(portbindings.VNIC_TYPE,), **{portbindings.VNIC_TYPE: vnic_type}) @@ -3983,8 +4139,8 @@ class TestOVNMechanismDriverSecurityGroup(MechDriverSetupBase, self.assertEqual(ovn_const.LSP_TYPE_EXTERNAL, kwargs['type']) self.assertEqual(fake_grp, kwargs['ha_chassis_group']) sync_mock.assert_called_once_with( - mock.ANY, net_id, self.mech_driver.nb_ovn, - self.mech_driver.sb_ovn, mock.ANY) + mock.ANY, mock.ANY, n['network']['id'], + self.mech_driver.nb_ovn, self.mech_driver.sb_ovn, mock.ANY) def test_create_port_with_vnic_direct(self): self._test_create_port_with_vnic_type(portbindings.VNIC_DIRECT) diff --git a/releasenotes/notes/external-port-scheduling-a5419ac51d863087.yaml b/releasenotes/notes/external-port-scheduling-a5419ac51d863087.yaml new file mode 100644 index 00000000000..9c94cfa0405 --- /dev/null +++ b/releasenotes/notes/external-port-scheduling-a5419ac51d863087.yaml @@ -0,0 +1,14 @@ +--- +features: + - | + A new ovn-cms-options option called ``enable-chassis-as-extport-host`` + is now recognized by ML2/OVN and is used to identify nodes that are + eligible for scheduling OVN's external ports. This feature is backward + compatible and if no nodes contain this new option the external + ports will continue to be scheduled using the ``enable-chassis-as-gw`` + option as before. + This change also introduces a limit to the number of members for each + HA Chassis Group to 5, matching the limit of gateway router port + replicas. This is because OVN uses BFD to monitor the connectivity + of each member and having an unlimited number of members could + potentially put a lot of stress in OVN.