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.

Conflicts:
  neutron/common/ovn/utils.py
  neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py
  neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_client.py
  neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py

Closes-Bug: 2037294
Change-Id: Ic46d847e3aebfe543d5a7ab49d18d1f1abf1342e
Signed-off-by: Lucas Alvares Gomes <[email protected]>
(cherry picked from commit 770914f)

Author: umago

neutron/common/ovn/constants.py

@@ -69,6 +69,7 @@
 
 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 @@
 # 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'
@@ -420,6 +427,7 @@
 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'
 

neutron/common/ovn/utils.py

@@ -35,7 +35,7 @@
 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 @@
 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-<UUID>
+    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(
@@ -671,6 +680,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, [])
@@ -724,7 +739,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
@@ -733,7 +748,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)}
 
 
@@ -862,77 +877,150 @@ def get_ovn_chassis_other_config(chassis):
         return chassis.external_ids
 
 
-def sync_ha_chassis_group(context, 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.
+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 network_id: The Neutron 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 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))
 
-    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))
+    return HAChassisGroupInfo(
+        group_name=group_name, chassis_list=chassis_list, az_hints=az_hints,
+        ignore_chassis=ignore_chassis)
 
-    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)
+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:
-        az_chassis = get_chassis_in_azs(ch_list, az_hints)
+        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 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
+    """
+    # 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)
+
+    # 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
-
-    # 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)):
+            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)
+
+    # 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_subnets_address_scopes(context, subnets, fixed_ips, ml2_plugin):
@@ -1090,3 +1178,9 @@ def get_requested_chassis(requested_chassis):
     if isinstance(requested_chassis, str):
         return requested_chassis.split(',')
     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')

neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py

@@ -1149,6 +1149,22 @@ def set_port_status_up(self, port_id):
             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

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py

@@ -659,6 +659,20 @@ def get_gateway_chassis_from_cms_options(self, name_only=True):
         :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.
@@ -674,3 +688,11 @@ def get_all_chassis(self, chassis_type=None):
         :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
+        """

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py

@@ -875,9 +875,11 @@ def get_chassis_hostname_and_physnets(self):
     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 = {}
@@ -964,3 +966,19 @@ def get_ports_on_chassis(self, chassis):
     def db_set(self, table, record, *col_values, if_exists=True, **columns):
         return cmd.DbSetCommand(self, table, record, *col_values,
                                 if_exists=if_exists, **columns)
+
+    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

neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py

@@ -606,7 +606,8 @@ def check_for_ha_chassis_group(self):
                     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))