diff --git a/neutron/common/ovn/constants.py b/neutron/common/ovn/constants.py index e1d7c672623..5d3ecdd72a2 100644 --- a/neutron/common/ovn/constants.py +++ b/neutron/common/ovn/constants.py @@ -23,7 +23,7 @@ OVN_NETWORK_MTU_EXT_ID_KEY = 'neutron:mtu' OVN_PORT_NAME_EXT_ID_KEY = 'neutron:port_name' OVN_PORT_FIP_EXT_ID_KEY = 'neutron:port_fip' OVN_ROUTER_NAME_EXT_ID_KEY = 'neutron:router_name' -OVN_ROUTER_AZ_HINTS_EXT_ID_KEY = 'neutron:availability_zone_hints' +OVN_AZ_HINTS_EXT_ID_KEY = 'neutron:availability_zone_hints' OVN_ROUTER_IS_EXT_GW = 'neutron:is_ext_gw' OVN_GW_PORT_EXT_ID_KEY = 'neutron:gw_port_id' OVN_SUBNET_EXT_ID_KEY = 'neutron:subnet_id' @@ -58,6 +58,7 @@ OVN_DROP_PORT_GROUP_NAME = 'neutron_pg_drop' OVN_ROUTER_PORT_GW_MTU_OPTION = 'gateway_mtu' OVN_PROVNET_PORT_NAME_PREFIX = 'provnet-' +OVN_NAME_PREFIX = 'neutron-' # Agent extension constants OVN_AGENT_DESC_KEY = 'neutron:description' diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index 4fab4e6143e..913c3c69be2 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -59,7 +59,7 @@ def ovn_name(id): # is a UUID. If so then there will be no matches. # We prefix the UUID to enable us to use the Neutron UUID when # updating, deleting etc. - return 'neutron-%s' % id + return "%s%s" % (constants.OVN_NAME_PREFIX, id) def ovn_lrouter_port_name(id): @@ -513,7 +513,7 @@ def get_port_id_from_gwc_row(row): def get_chassis_availability_zones(chassis): """Return a list of availability zones from a given OVN Chassis.""" - azs = [] + azs = set() if not chassis: return azs @@ -522,11 +522,42 @@ def get_chassis_availability_zones(chassis): if not opt.startswith(opt_key): continue values = opt.split('=')[1] - azs = [az.strip() for az in values.split(':') if az.strip()] + azs = {az.strip() for az in values.split(':') if az.strip()} break return azs +def get_chassis_in_azs(chassis_list, az_list): + """Return a set of Chassis that belongs to the AZs. + + Given a list of Chassis and a list of availability zones (AZs), + return a set of Chassis that belongs to one or more AZs. + + :param chassis_list: A list of Chassis objects + :param az_list: A list of availability zones + :returns: A set of Chassis names + """ + chassis = set() + for ch in chassis_list: + chassis_azs = get_chassis_availability_zones(ch) + if chassis_azs.intersection(az_list): + chassis.add(ch.name) + return chassis + + +def get_gateway_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 + belong to any availability zones. + + :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 + get_chassis_availability_zones(ch)} + + def parse_ovn_lb_port_forwarding(ovn_rtr_lb_pfs): """Return a dictionary compatible with port forwarding from OVN lb.""" result = {} diff --git a/neutron/common/utils.py b/neutron/common/utils.py index 0346fe5d43e..d484f4e42cb 100644 --- a/neutron/common/utils.py +++ b/neutron/common/utils.py @@ -987,7 +987,7 @@ def get_sql_random_method(sql_dialect_name): def get_az_hints(resource): """Return the availability zone hints from a given resource.""" - return (resource.get(az_def.AZ_HINTS) or + return (sorted(resource.get(az_def.AZ_HINTS, [])) or cfg.CONF.default_availability_zones) 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 8aa9c803e35..e1e1e7c3ca3 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/mech_driver.py @@ -31,6 +31,7 @@ from neutron_lib.callbacks import resources from neutron_lib import constants as const from neutron_lib import context as n_context from neutron_lib import exceptions as n_exc +from neutron_lib.exceptions import availability_zone as az_exc from neutron_lib.plugins import directory from neutron_lib.plugins.ml2 import api from oslo_config import cfg @@ -325,6 +326,8 @@ class OVNMechanismDriver(api.MechanismDriver): # Override availability zone methods self.patch_plugin_merge("get_availability_zones", get_availability_zones) + self.patch_plugin_choose("validate_availability_zones", + validate_availability_zones) # Now IDL connections can be safely used. self._post_fork_event.set() @@ -1254,3 +1257,16 @@ def get_availability_zones(cls, context, _driver, filters=None, fields=None, sorts=None, limit=None, marker=None, page_reverse=False): return list(_driver.list_availability_zones(context, filters).values()) + + +def validate_availability_zones(cls, context, resource_type, + availability_zones, _driver): + if not availability_zones or resource_type != 'network': + return + + azs = {az['name'] for az in + _driver.list_availability_zones(context).values()} + diff = set(availability_zones) - azs + if diff: + raise az_exc.AvailabilityZoneNotFound( + availability_zone=', '.join(diff)) 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 aac70ea76c4..0d16d3f100d 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/api.py @@ -626,7 +626,8 @@ class SbAPI(api.API, metaclass=abc.ABCMeta): value. And hostname and physnets are related to the same host. """ - def get_gateway_chassis_from_cms_options(self): + @abc.abstractmethod + def get_gateway_chassis_from_cms_options(self, name_only=True): """Get chassis eligible for external connectivity from CMS options. When admin wants to enable router gateway on few chassis, @@ -635,7 +636,10 @@ class SbAPI(api.API, metaclass=abc.ABCMeta): ovs-vsctl set open . external_ids:ovn-cms-options="enable-chassis-as-gw" In this function, we parse ovn-cms-options and return these chassis - :returns: List with chassis names. + + :param name_only: Return only the chassis names instead of + objects. Defaults to True. + :returns: List with chassis. """ @abc.abstractmethod 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 27653ccea72..24ef98583c5 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 @@ -848,13 +848,11 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend): chassis_info_dict[ch.hostname] = self._get_chassis_physnets(ch) return chassis_info_dict - def get_gateway_chassis_from_cms_options(self): - gw_chassis = [] - for ch in self.chassis_list().execute(check_error=True): - cms_options = ch.external_ids.get('ovn-cms-options', '') - if 'enable-chassis-as-gw' in cms_options.split(','): - gw_chassis.append(ch.name) - return gw_chassis + 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 + ch.external_ids.get(ovn_const.OVN_CMS_OPTIONS, '').split(',')] def get_chassis_and_physnets(self): chassis_info_dict = {} 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 1712ada69ce..a8cdafd740e 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -571,10 +571,18 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): raise periodics.NeverAgain() + def _delete_default_ha_chassis_group(self, txn): + # TODO(lucasgomes): Remove the deletion of the + # HA_CHASSIS_GROUP_DEFAULT_NAME in the Y cycle. We no longer + # have a default HA Chassis Group. + cmd = [self._nb_idl.ha_chassis_group_del( + ovn_const.HA_CHASSIS_GROUP_DEFAULT_NAME, if_exists=True)] + self._ovn_client._transaction(cmd, txn=txn) + # A static spacing value is used here, but this method will only run # once per lock due to the use of periodics.NeverAgain(). @periodics.periodic(spacing=600, run_immediately=True) - def check_for_ha_chassis_group_address(self): + def check_for_ha_chassis_group(self): # If external ports is not supported stop running # this periodic task if not self._ovn_client.is_external_ports_supported(): @@ -583,44 +591,27 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): if not self.has_lock: return - default_ch_grp = self._nb_idl.ha_chassis_group_add( - ovn_const.HA_CHASSIS_GROUP_DEFAULT_NAME, may_exist=True).execute( - check_error=True) - - # NOTE(lucasagomes): Find the existing chassis with the highest - # priority and keep it as being the highest to avoid moving - # things around - high_prio_ch = max(default_ch_grp.ha_chassis, key=lambda x: x.priority, - default=None) - - all_ch = self._sb_idl.get_all_chassis() - gw_ch = self._sb_idl.get_gateway_chassis_from_cms_options() - ch_to_del = set(all_ch) - set(gw_ch) + external_ports = self._nb_idl.db_find_rows( + 'Logical_Switch_Port', ('type', '=', ovn_const.LSP_TYPE_EXTERNAL) + ).execute(check_error=True) + context = n_context.get_admin_context() with self._nb_idl.transaction(check_error=True) as txn: - for ch in ch_to_del: - txn.add(self._nb_idl.ha_chassis_group_del_chassis( - ovn_const.HA_CHASSIS_GROUP_DEFAULT_NAME, ch, - if_exists=True)) + for port in external_ports: + network_id = port.external_ids[ + ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY].replace( + ovn_const.OVN_NAME_PREFIX, '') + ha_ch_grp = self._ovn_client.sync_ha_chassis_group( + context, network_id, txn) + try: + port_ha_ch_uuid = port.ha_chassis_group[0].uuid + except IndexError: + port_ha_ch_uuid = None + if port_ha_ch_uuid != ha_ch_grp: + txn.add(self._nb_idl.set_lswitch_port( + port.name, ha_chassis_group=ha_ch_grp)) - # NOTE(lucasagomes): If the high priority chassis is in - # the list of chassis to be added/updated. Add it first with - # the highest priority number possible and then add the rest - # (the priority of the rest of the chassis does not matter - # since only the highest one is active) - priority = ovn_const.HA_CHASSIS_GROUP_HIGHEST_PRIORITY - if high_prio_ch and high_prio_ch.chassis_name in gw_ch: - txn.add(self._nb_idl.ha_chassis_group_add_chassis( - ovn_const.HA_CHASSIS_GROUP_DEFAULT_NAME, - high_prio_ch.chassis_name, priority=priority)) - gw_ch.remove(high_prio_ch.chassis_name) - priority -= 1 - - for ch in gw_ch: - txn.add(self._nb_idl.ha_chassis_group_add_chassis( - ovn_const.HA_CHASSIS_GROUP_DEFAULT_NAME, - ch, priority=priority)) - priority -= 1 + self._delete_default_ha_chassis_group(txn) raise periodics.NeverAgain() 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 46b70641b51..4d46afb30a5 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 @@ -15,6 +15,7 @@ import collections import copy +import random import netaddr from neutron_lib.api.definitions import l3 @@ -321,10 +322,74 @@ class OVNClient(object): parent_name, tag, dhcpv4_options, dhcpv6_options, cidrs.strip(), device_owner, sg_ids) - def _get_default_ha_chassis_group(self): - return self._nb_idl.ha_chassis_group_get( - ovn_const.HA_CHASSIS_GROUP_DEFAULT_NAME).execute( - check_error=True).uuid + def sync_ha_chassis_group(self, context, network_id, txn): + """Return the UUID of the HA Chassis Group. + + 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 txn: The ovsdbapp transaction object. + :returns: An HA Chassis Group UUID. + """ + az_hints = common_utils.get_az_hints( + self._plugin.get_network(context, network_id)) + + ha_ch_grp_name = utils.ovn_name(network_id) + # FIXME(lucasagomes): Couldn't find a better way of doing this + # without a sub-transaction. This shouldn't be a problem since + # the HA Chassis Group associated with a network will be deleted + # as part of the network delete method (if present) + with self._nb_idl.create_transaction(check_error=True) as sub_txn: + sub_txn.add(self._nb_idl.ha_chassis_group_add( + ha_ch_grp_name, may_exist=True)) + + ha_ch_grp = self._nb_idl.ha_chassis_group_get( + ha_ch_grp_name).execute(check_error=True) + txn.add(self._nb_idl.db_set( + 'HA_Chassis_Group', ha_ch_grp_name, ('external_ids', + {ovn_const.OVN_AZ_HINTS_EXT_ID_KEY: ','.join(az_hints)}))) + + # Get the chassis belonging to the AZ hints + ch_list = self._sb_idl.get_gateway_chassis_from_cms_options( + name_only=False) + if not az_hints: + az_chassis = utils.get_gateway_chassis_without_azs(ch_list) + else: + az_chassis = utils.get_chassis_in_azs(ch_list, az_hints) + + # Remove any chassis that no longer belongs to the AZ hints + all_ch = {ch.chassis_name for ch in ha_ch_grp.ha_chassis} + ch_to_del = all_ch - az_chassis + for ch in ch_to_del: + txn.add(self._nb_idl.ha_chassis_group_del_chassis( + ha_ch_grp_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. + high_prio_ch = max(ha_ch_grp.ha_chassis, key=lambda x: x.priority, + default=None) + priority = ovn_const.HA_CHASSIS_GROUP_HIGHEST_PRIORITY + if high_prio_ch and high_prio_ch.chassis_name in az_chassis: + txn.add(self._nb_idl.ha_chassis_group_add_chassis( + ha_ch_grp_name, high_prio_ch.chassis_name, + priority=priority)) + az_chassis.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)): + txn.add(self._nb_idl.ha_chassis_group_add_chassis( + ha_ch_grp_name, ch, priority=priority)) + priority -= 1 + + return ha_ch_grp.uuid def create_port(self, context, port): if utils.is_lsp_ignored(port): @@ -390,7 +455,8 @@ class OVNClient(object): if (self.is_external_ports_supported() and port_info.type == ovn_const.LSP_TYPE_EXTERNAL): kwargs['ha_chassis_group'] = ( - self._get_default_ha_chassis_group()) + self.sync_ha_chassis_group( + context, port['network_id'], txn)) # NOTE(mjozefcz): Do not set addresses if the port is not # bound, has no device_owner and it is OVN LB VIP port. @@ -510,7 +576,8 @@ class OVNClient(object): if self.is_external_ports_supported(): if port_info.type == ovn_const.LSP_TYPE_EXTERNAL: columns_dict['ha_chassis_group'] = ( - self._get_default_ha_chassis_group()) + self.sync_ha_chassis_group( + context, port['network_id'], txn)) else: # Clear the ha_chassis_group field columns_dict['ha_chassis_group'] = [] @@ -1147,7 +1214,7 @@ class OVNClient(object): router.get('gw_port_id') or '', ovn_const.OVN_REV_NUM_EXT_ID_KEY: str(utils.get_revision_number( router, ovn_const.TYPE_ROUTERS)), - ovn_const.OVN_ROUTER_AZ_HINTS_EXT_ID_KEY: + ovn_const.OVN_AZ_HINTS_EXT_ID_KEY: ','.join(common_utils.get_az_hints(router))} def create_router(self, context, router, add_external_gateway=True): @@ -1606,7 +1673,9 @@ class OVNClient(object): ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY: network['name'], ovn_const.OVN_NETWORK_MTU_EXT_ID_KEY: str(network['mtu']), ovn_const.OVN_REV_NUM_EXT_ID_KEY: str( - utils.get_revision_number(network, ovn_const.TYPE_NETWORKS))}} + utils.get_revision_number(network, ovn_const.TYPE_NETWORKS)), + ovn_const.OVN_AZ_HINTS_EXT_ID_KEY: + ','.join(common_utils.get_az_hints(network))}} # Enable IGMP snooping if igmp_snooping_enable is enabled in Neutron value = 'true' if ovn_conf.is_igmp_snooping_enabled() else 'false' @@ -1639,13 +1708,13 @@ class OVNClient(object): def delete_network(self, context, network_id): with self._nb_idl.transaction(check_error=True) as txn: - ls, ls_dns_record = self._nb_idl.get_ls_and_dns_record( - utils.ovn_name(network_id)) + ls_name = utils.ovn_name(network_id) + ls, ls_dns_record = self._nb_idl.get_ls_and_dns_record(ls_name) - txn.add(self._nb_idl.ls_del(utils.ovn_name(network_id), - if_exists=True)) + txn.add(self._nb_idl.ls_del(ls_name, if_exists=True)) if ls_dns_record: txn.add(self._nb_idl.dns_del(ls_dns_record.uuid)) + txn.add(self._nb_idl.ha_chassis_group_del(ls_name, if_exists=True)) db_rev.delete_revision( context, network_id, ovn_const.TYPE_NETWORKS) @@ -1714,6 +1783,19 @@ 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_name.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: + self.sync_ha_chassis_group(context, network['id'], txn) + self._qos_driver.update_network(txn, network, original_network) if check_rev_cmd.result == ovn_const.TXN_COMMITTED: 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 b37abd1eeec..d1df24f8e4a 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 @@ -74,6 +74,39 @@ class ChassisEvent(row_event.RowEvent): super(ChassisEvent, self).__init__(events, table, None) self.event_name = 'ChassisEvent' + def _get_ha_chassis_groups_within_azs(self, az_hints): + """Find all HA Chassis groups that are within the given AZs. + + :param az_hints: A list of availability zones hints + :returns: A set with the HA Chassis Groups objects + """ + ha_chassis_list = [] + for hcg in self.driver.nb_ovn.db_list_rows( + 'HA_Chassis_Group').execute(check_error=True): + if not hcg.name.startswith(ovn_const.OVN_NAME_PREFIX): + continue + # The filter() is to get rid of the empty string in + # the list that is returned because of split() + azs = {az for az in + hcg.external_ids.get( + ovn_const.OVN_AZ_HINTS_EXT_ID_KEY, '').split(',') if az} + # Find which Ha Chassis Group that is included in the + # Availability Zone hints + if az_hints.intersection(azs): + ha_chassis_list.append(hcg) + # If the Availability Zone hints is empty return a list + # of HA Chassis Groups that does not belong to any AZ + elif not az_hints and not azs: + ha_chassis_list.append(hcg) + return ha_chassis_list + + def _get_min_priority_in_hcg(self, ha_chassis_group): + """Find the next lowest priority number within a HA Chassis Group.""" + min_priority = min( + [ch.priority for ch in ha_chassis_group.ha_chassis], + default=ovn_const.HA_CHASSIS_GROUP_HIGHEST_PRIORITY) + return min_priority - 1 + def handle_ha_chassis_group_changes(self, event, row, old): """Handle HA Chassis Group changes. @@ -88,9 +121,31 @@ class ChassisEvent(row_event.RowEvent): if not is_gw_chassis and event == self.ROW_CREATE: return + azs = utils.get_chassis_availability_zones(row) + if event == self.ROW_UPDATE: is_old_gw = utils.is_gateway_chassis(old) if is_gw_chassis and is_old_gw: + old_azs = utils.get_chassis_availability_zones(old) + # If there are no differences in the AZs, return + if azs == old_azs: + return + # Find out the HA Chassis Groups that were affected by + # the update (to add and/or remove the updated Chassis) + ha_ch_add = self._get_ha_chassis_groups_within_azs( + azs - old_azs) + ha_ch_del = self._get_ha_chassis_groups_within_azs( + old_azs - azs) + with self.driver.nb_ovn.transaction(check_error=True) as txn: + for hcg in ha_ch_add: + min_priority = self._get_min_priority_in_hcg(hcg) + txn.add( + self.driver.nb_ovn.ha_chassis_group_add_chassis( + hcg.name, row.name, priority=min_priority)) + for hcg in ha_ch_del: + txn.add( + self.driver.nb_ovn.ha_chassis_group_del_chassis( + hcg.name, row.name, if_exists=True)) return elif not is_gw_chassis and is_old_gw: # Chassis is not a gateway anymore, treat it as deletion @@ -100,24 +155,19 @@ class ChassisEvent(row_event.RowEvent): event = self.ROW_CREATE if event == self.ROW_CREATE: - default_group = self.driver.nb_ovn.ha_chassis_group_get( - ovn_const.HA_CHASSIS_GROUP_DEFAULT_NAME).execute( - check_error=True) - - # Find what's the lowest priority number current in the group - # and add the new chassis as the new lowest - min_priority = min( - [ch.priority for ch in default_group.ha_chassis], - default=ovn_const.HA_CHASSIS_GROUP_HIGHEST_PRIORITY) - - self.driver.nb_ovn.ha_chassis_group_add_chassis( - ovn_const.HA_CHASSIS_GROUP_DEFAULT_NAME, row.name, - priority=min_priority - 1).execute(check_error=True) + ha_chassis_list = self._get_ha_chassis_groups_within_azs(azs) + with self.driver.nb_ovn.transaction(check_error=True) as txn: + for hcg in ha_chassis_list: + min_priority = self._get_min_priority_in_hcg(hcg) + txn.add(self.driver.nb_ovn.ha_chassis_group_add_chassis( + hcg.name, row.name, priority=min_priority)) elif event == self.ROW_DELETE: - self.driver.nb_ovn.ha_chassis_group_del_chassis( - ovn_const.HA_CHASSIS_GROUP_DEFAULT_NAME, - row.name, if_exists=True).execute(check_error=True) + ha_chassis_list = self._get_ha_chassis_groups_within_azs(azs) + with self.driver.nb_ovn.transaction(check_error=True) as txn: + for hcg in ha_chassis_list: + txn.add(self.driver.nb_ovn.ha_chassis_group_del_chassis( + hcg.name, row.name, if_exists=True)) def match_fn(self, event, row, old): if event != self.ROW_UPDATE: @@ -125,12 +175,19 @@ class ChassisEvent(row_event.RowEvent): # NOTE(lucasgomes): If the external_ids column wasn't updated # (meaning, Chassis "gateway" status didn't change) just returns if not hasattr(old, 'external_ids') and event == self.ROW_UPDATE: - return + return False if (old.external_ids.get('ovn-bridge-mappings') != row.external_ids.get('ovn-bridge-mappings')): return True - f = utils.is_gateway_chassis - return f(old) != f(row) + # Check if either the Gateway status or Availability Zones has + # changed in the Chassis + is_gw = utils.is_gateway_chassis(row) + is_gw_old = utils.is_gateway_chassis(old) + azs = utils.get_chassis_availability_zones(row) + old_azs = utils.get_chassis_availability_zones(old) + if is_gw != is_gw_old or azs != old_azs: + return True + return False def run(self, event, row, old): host = row.hostname diff --git a/neutron/services/ovn_l3/plugin.py b/neutron/services/ovn_l3/plugin.py index 0399636dbed..d2ef49de353 100644 --- a/neutron/services/ovn_l3/plugin.py +++ b/neutron/services/ovn_l3/plugin.py @@ -485,7 +485,7 @@ class OVNL3RouterPlugin(service_base.ServicePluginBase, return [] return [az.strip() for az in lr.external_ids.get( - ovn_const.OVN_ROUTER_AZ_HINTS_EXT_ID_KEY, '').split(',') + ovn_const.OVN_AZ_HINTS_EXT_ID_KEY, '').split(',') if az.strip()] def validate_availability_zones(self, context, resource_type, diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py index 13919fcc2cf..7d0b7b6850c 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py @@ -470,11 +470,6 @@ class TestExternalPorts(base.TestOVNFunctionalBase): '10.0.0.0/24') self.sub = self.deserialize(self.fmt, res) - # The default group will be created by the maintenance task ( - # which is disabled in the functional jobs). So let's add it - self.default_ch_grp = self.nb_api.ha_chassis_group_add( - ovn_const.HA_CHASSIS_GROUP_DEFAULT_NAME).execute(check_error=True) - def _find_port_row_by_name(self, name): cmd = self.nb_api.db_find_rows( 'Logical_Switch_Port', ('name', '=', name)) @@ -482,8 +477,9 @@ class TestExternalPorts(base.TestOVNFunctionalBase): return rows[0] if rows else None def _test_external_port_create(self, vnic_type): + net_id = self.n1['network']['id'] port_data = { - 'port': {'network_id': self.n1['network']['id'], + 'port': {'network_id': net_id, 'tenant_id': self._tenant_id, portbindings.VNIC_TYPE: vnic_type}} @@ -494,8 +490,8 @@ class TestExternalPorts(base.TestOVNFunctionalBase): ovn_port = self._find_port_row_by_name(port['id']) self.assertEqual(ovn_const.LSP_TYPE_EXTERNAL, ovn_port.type) self.assertEqual(1, len(ovn_port.ha_chassis_group)) - self.assertEqual(str(self.default_ch_grp.uuid), - str(ovn_port.ha_chassis_group[0].uuid)) + self.assertEqual(utils.ovn_name(net_id), + str(ovn_port.ha_chassis_group[0].name)) def test_external_port_create_vnic_direct(self): self._test_external_port_create(portbindings.VNIC_DIRECT) @@ -507,8 +503,9 @@ class TestExternalPorts(base.TestOVNFunctionalBase): self._test_external_port_create(portbindings.VNIC_MACVTAP) def _test_external_port_update(self, vnic_type): + net_id = self.n1['network']['id'] port_data = { - 'port': {'network_id': self.n1['network']['id'], + 'port': {'network_id': net_id, 'tenant_id': self._tenant_id}} port_req = self.new_create_request('ports', port_data, self.fmt) @@ -529,8 +526,8 @@ class TestExternalPorts(base.TestOVNFunctionalBase): ovn_port = self._find_port_row_by_name(port['id']) self.assertEqual(ovn_const.LSP_TYPE_EXTERNAL, ovn_port.type) self.assertEqual(1, len(ovn_port.ha_chassis_group)) - self.assertEqual(str(self.default_ch_grp.uuid), - str(ovn_port.ha_chassis_group[0].uuid)) + self.assertEqual(utils.ovn_name(net_id), + str(ovn_port.ha_chassis_group[0].name)) def test_external_port_update_vnic_direct(self): self._test_external_port_update(portbindings.VNIC_DIRECT) @@ -571,8 +568,9 @@ class TestExternalPorts(base.TestOVNFunctionalBase): self._test_external_port_create_switchdev(portbindings.VNIC_MACVTAP) def _test_external_port_update_switchdev(self, vnic_type): + net_id = self.n1['network']['id'] port_data = { - 'port': {'network_id': self.n1['network']['id'], + 'port': {'network_id': net_id, 'tenant_id': self._tenant_id, portbindings.VNIC_TYPE: vnic_type}} @@ -585,8 +583,8 @@ class TestExternalPorts(base.TestOVNFunctionalBase): ovn_port = self._find_port_row_by_name(port['id']) self.assertEqual(ovn_const.LSP_TYPE_EXTERNAL, ovn_port.type) self.assertEqual(1, len(ovn_port.ha_chassis_group)) - self.assertEqual(str(self.default_ch_grp.uuid), - str(ovn_port.ha_chassis_group[0].uuid)) + self.assertEqual(utils.ovn_name(net_id), + str(ovn_port.ha_chassis_group[0].name)) # Now, update the port to add a "switchdev" capability and make # sure it's not treated as an "external" port anymore nor it's diff --git a/neutron/tests/unit/common/ovn/test_utils.py b/neutron/tests/unit/common/ovn/test_utils.py index b0cc30ab503..c090ce0a613 100644 --- a/neutron/tests/unit/common/ovn/test_utils.py +++ b/neutron/tests/unit/common/ovn/test_utils.py @@ -65,14 +65,14 @@ class TestUtils(base.BaseTestCase): def test_get_chassis_availability_zones_no_azs(self): chassis = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={ 'external_ids': {'ovn-cms-options': 'enable-chassis-as-gw'}}) - self.assertEqual([], utils.get_chassis_availability_zones(chassis)) + self.assertEqual(set(), utils.get_chassis_availability_zones(chassis)) def test_get_chassis_availability_zones_one_az(self): chassis = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={ 'external_ids': {'ovn-cms-options': 'enable-chassis-as-gw,availability-zones=az0'}}) self.assertEqual( - ['az0'], utils.get_chassis_availability_zones(chassis)) + {'az0'}, utils.get_chassis_availability_zones(chassis)) def test_get_chassis_availability_zones_multiple_az(self): chassis = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={ @@ -80,7 +80,7 @@ class TestUtils(base.BaseTestCase): 'ovn-cms-options': 'enable-chassis-as-gw,availability-zones=az0:az1 :az2:: :'}}) self.assertEqual( - ['az0', 'az1', 'az2'], + {'az0', 'az1', 'az2'}, utils.get_chassis_availability_zones(chassis)) def test_get_chassis_availability_zones_malformed(self): @@ -88,7 +88,7 @@ class TestUtils(base.BaseTestCase): 'external_ids': {'ovn-cms-options': 'enable-chassis-as-gw,availability-zones:az0'}}) self.assertEqual( - [], utils.get_chassis_availability_zones(chassis)) + set(), utils.get_chassis_availability_zones(chassis)) def test_is_security_groups_enabled(self): self.assertTrue(utils.is_security_groups_enabled( @@ -141,6 +141,57 @@ class TestUtils(base.BaseTestCase): rc = utils.parse_ovn_lb_port_forwarding(tc_lbs) self.assertEqual(rc, tc.output, tc.description) + def test_get_chassis_in_azs(self): + ch0 = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={ + 'name': 'ch0', + 'external_ids': { + 'ovn-cms-options': + 'enable-chassis-as-gw,availability-zones=az0:az1:az2'}}) + ch1 = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={ + 'name': 'ch1', + 'external_ids': { + 'ovn-cms-options': 'enable-chassis-as-gw'}}) + ch2 = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={ + 'name': 'ch2', + 'external_ids': { + 'ovn-cms-options': + 'enable-chassis-as-gw,availability-zones=az1:az5'}}) + + chassis_list = [ch0, ch1, ch2] + self.assertEqual( + {'ch0', 'ch2'}, + utils.get_chassis_in_azs(chassis_list, ['az1', 'az5'])) + self.assertEqual( + {'ch0'}, + utils.get_chassis_in_azs(chassis_list, ['az2', 'az6'])) + self.assertEqual( + set(), + utils.get_chassis_in_azs(chassis_list, ['az6'])) + + def test_get_gateway_chassis_without_azs(self): + ch0 = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={ + 'name': 'ch0', + 'external_ids': { + 'ovn-cms-options': + 'enable-chassis-as-gw,availability-zones=az0:az1:az2'}}) + ch1 = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={ + 'name': 'ch1', + 'external_ids': { + 'ovn-cms-options': 'enable-chassis-as-gw'}}) + ch2 = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={ + 'name': 'ch2', + 'external_ids': { + 'ovn-cms-options': + 'enable-chassis-as-gw,availability-zones=az1:az5'}}) + ch3 = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={ + 'name': 'ch3', + 'external_ids': {}}) + + chassis_list = [ch0, ch1, ch2, ch3] + self.assertEqual( + {'ch1'}, + utils.get_gateway_chassis_without_azs(chassis_list)) + class TestGateWayChassisValidity(base.BaseTestCase): diff --git a/neutron/tests/unit/fake_resources.py b/neutron/tests/unit/fake_resources.py index 890c53a0b00..bfbe625b394 100644 --- a/neutron/tests/unit/fake_resources.py +++ b/neutron/tests/unit/fake_resources.py @@ -154,6 +154,9 @@ class FakeOvsdbNbOvnIdl(object): self.qos_del_ext_ids = mock.Mock() self.meter_add = mock.Mock() self.meter_del = mock.Mock() + self.ha_chassis_group_add = mock.Mock() + self.ha_chassis_group_add_chassis = mock.Mock() + self.ha_chassis_group_del_chassis = mock.Mock() class FakeOvsdbSbOvnIdl(object): 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 d66af4da5e7..9af35a0461b 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 @@ -352,61 +352,65 @@ class TestDBInconsistenciesPeriodics(testlib_api.SqlTestCaseLight, ] nb_idl.db_set.assert_has_calls(expected_calls) - def test_check_for_ha_chassis_group_address_not_supported(self): + def test_check_for_ha_chassis_group_not_supported(self): self.fake_ovn_client.is_external_ports_supported.return_value = False self.assertRaises(periodics.NeverAgain, - self.periodic.check_for_ha_chassis_group_address) + self.periodic.check_for_ha_chassis_group) self.assertFalse( self.fake_ovn_client._nb_idl.ha_chassis_group_add.called) - def test_check_for_ha_chassis_group_address(self): + def test_check_for_ha_chassis_group_no_external_ports(self): self.fake_ovn_client.is_external_ports_supported.return_value = True nb_idl = self.fake_ovn_client._nb_idl - sb_idl = self.fake_ovn_client._sb_idl + nb_idl.db_find_rows.return_value.execute.return_value = [] + self.assertRaises(periodics.NeverAgain, + self.periodic.check_for_ha_chassis_group) + self.assertFalse( + self.fake_ovn_client.sync_ha_chassis_group.called) - gw_chassis_0 = fakes.FakeOvsdbRow.create_one_ovsdb_row( - attrs={'priority': 1, - 'name': 'gw_chassis_0', - 'chassis_name': 'gw_chassis_0'}) - gw_chassis_1 = fakes.FakeOvsdbRow.create_one_ovsdb_row( - attrs={'priority': 2, - 'name': 'gw_chassis_1', - 'chassis_name': 'gw_chassis_1'}) - non_gw_chassis_0 = fakes.FakeOvsdbRow.create_one_ovsdb_row( - attrs={'name': 'non_gw_chassis_0'}) - default_ha_group = fakes.FakeOvsdbRow.create_one_ovsdb_row( - attrs={'ha_chassis': [gw_chassis_0, gw_chassis_1]}) + def test_check_for_ha_chassis_group(self): + self.fake_ovn_client.is_external_ports_supported.return_value = True + nb_idl = self.fake_ovn_client._nb_idl - nb_idl.ha_chassis_group_add.return_value.execute.return_value = ( - default_ha_group) - sb_idl.get_all_chassis.return_value = [ - non_gw_chassis_0.name, gw_chassis_0.name, gw_chassis_1.name] - sb_idl.get_gateway_chassis_from_cms_options.return_value = [ - gw_chassis_0.name, gw_chassis_1.name] + hcg0 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'uuid': '1f4323db-fb58-48e9-adae-6c6e833c581d', + 'name': 'test-ha-grp'}) + hcg1 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'uuid': 'e95ff98f-7f03-484b-a156-d8c7e366dd3d', + 'name': 'another-test-ha-grp'}) + p0 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'type': constants.LSP_TYPE_EXTERNAL, + 'name': 'p0', + 'ha_chassis_group': [hcg0], + 'external_ids': { + constants.OVN_NETWORK_NAME_EXT_ID_KEY: 'neutron-net0'}}) + p1 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'type': constants.LSP_TYPE_EXTERNAL, + 'name': 'p1', + 'ha_chassis_group': [hcg1], + 'external_ids': { + constants.OVN_NETWORK_NAME_EXT_ID_KEY: 'neutron-net1'}}) + + nb_idl.db_find_rows.return_value.execute.return_value = [p0, p1] + self.fake_ovn_client.sync_ha_chassis_group.return_value = hcg0.uuid # Invoke the periodic method, it meant to run only once at startup # so NeverAgain will be raised at the end self.assertRaises(periodics.NeverAgain, - self.periodic.check_for_ha_chassis_group_address) + self.periodic.check_for_ha_chassis_group) - # Make sure the non GW chassis has been removed from the - # default HA_CHASSIS_GROUP - nb_idl.ha_chassis_group_del_chassis.assert_called_once_with( - constants.HA_CHASSIS_GROUP_DEFAULT_NAME, non_gw_chassis_0.name, - if_exists=True) - - # Assert the GW chassis are being added to the - # default HA_CHASSIS_GROUP + # Assert sync_ha_chassis_group() is called for both networks expected_calls = [ - mock.call(constants.HA_CHASSIS_GROUP_DEFAULT_NAME, - gw_chassis_1.chassis_name, - priority=constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY), - # Note that the second chassis is getting priority -1 - mock.call(constants.HA_CHASSIS_GROUP_DEFAULT_NAME, - gw_chassis_0.chassis_name, - priority=constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY - 1) - ] - nb_idl.ha_chassis_group_add_chassis.assert_has_calls(expected_calls) + mock.call(mock.ANY, 'net0', mock.ANY), + mock.call(mock.ANY, 'net1', mock.ANY)] + self.fake_ovn_client.sync_ha_chassis_group.assert_has_calls( + expected_calls) + + # Assert set_lswitch_port() is only called for p1 because + # the ha_chassis_group is different than what was returned + # by sync_ha_chassis_group() + nb_idl.set_lswitch_port.assert_called_once_with( + 'p1', ha_chassis_group=hcg0.uuid) def test_check_for_mcast_flood_reports(self): nb_idl = self.fake_ovn_client._nb_idl diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py index de8cf6a5c3b..e2da4b2e5ff 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py @@ -610,7 +610,7 @@ class TestChassisEvent(base.BaseTestCase): def setUp(self): super(TestChassisEvent, self).setUp() - self.driver = mock.Mock() + self.driver = mock.MagicMock() self.nb_ovn = self.driver.nb_ovn self.driver._ovn_client.is_external_ports_supported.return_value = True self.event = ovsdb_monitor.ChassisEvent(self.driver) @@ -627,44 +627,76 @@ class TestChassisEvent(base.BaseTestCase): self.assertFalse(self.nb_ovn.ha_chassis_group_del_chassis.called) def _test_handle_ha_chassis_group_changes_create(self, event): + # Chassis + ext_ids = { + 'ovn-cms-options': 'enable-chassis-as-gw,availability-zones=az-0'} row = fakes.FakeOvsdbTable.create_one_ovsdb_table( - attrs={'name': 'SpongeBob'}) + attrs={'name': 'SpongeBob', 'external_ids': ext_ids}) + # HA Chassis ch0 = fakes.FakeOvsdbTable.create_one_ovsdb_table( attrs={'priority': 10}) ch1 = fakes.FakeOvsdbTable.create_one_ovsdb_table( attrs={'priority': 9}) - default_grp = fakes.FakeOvsdbTable.create_one_ovsdb_table( - attrs={'ha_chassis': [ch0, ch1]}) - self.nb_ovn.ha_chassis_group_get.return_value.execute.return_value = ( - default_grp) + ch2 = fakes.FakeOvsdbTable.create_one_ovsdb_table( + attrs={'priority': 10}) + # HA Chassis Groups + ha_ch_grp0 = fakes.FakeOvsdbTable.create_one_ovsdb_table( + attrs={'ha_chassis': [ch0, ch1], 'name': 'neutron-ha-ch-grp0', + 'external_ids': { + ovn_const.OVN_AZ_HINTS_EXT_ID_KEY: 'az-0,az-1'}}) + ha_ch_grp1 = fakes.FakeOvsdbTable.create_one_ovsdb_table( + attrs={'ha_chassis': [ch2], 'name': 'neutron-ha-ch-grp1', + 'external_ids': { + ovn_const.OVN_AZ_HINTS_EXT_ID_KEY: 'az-2'}}) + + self.nb_ovn.db_list_rows.return_value.execute.return_value = [ + ha_ch_grp0, ha_ch_grp1] self.event.handle_ha_chassis_group_changes(event, row, mock.Mock()) - # Assert the new chassis has been added to the default - # group with the lowest priority + # Assert the new chassis has been added to "neutron-ha-ch-grp0" + # HA Chassis Group with the lowest priority self.nb_ovn.ha_chassis_group_add_chassis.assert_called_once_with( - ovn_const.HA_CHASSIS_GROUP_DEFAULT_NAME, 'SpongeBob', priority=8) + 'neutron-ha-ch-grp0', 'SpongeBob', priority=8) def test_handle_ha_chassis_group_changes_create(self): self._test_handle_ha_chassis_group_changes_create( self.event.ROW_CREATE) def _test_handle_ha_chassis_group_changes_delete(self, event): + # Chassis + ext_ids = { + 'ovn-cms-options': 'enable-chassis-as-gw,availability-zones=az-0'} row = fakes.FakeOvsdbTable.create_one_ovsdb_table( - attrs={'name': 'SpongeBob'}) + attrs={'name': 'SpongeBob', 'external_ids': ext_ids}) + # HA Chassis + ha_ch = fakes.FakeOvsdbTable.create_one_ovsdb_table( + attrs={'priority': 10}) + # HA Chassis Group + ha_ch_grp = fakes.FakeOvsdbTable.create_one_ovsdb_table( + attrs={'ha_chassis': [ha_ch], 'name': 'neutron-ha-ch-grp', + 'external_ids': { + ovn_const.OVN_AZ_HINTS_EXT_ID_KEY: 'az-0'}}) + self.nb_ovn.db_list_rows.return_value.execute.return_value = [ + ha_ch_grp] + self.event.handle_ha_chassis_group_changes(event, row, mock.Mock()) # Assert chassis was removed from the default group self.nb_ovn.ha_chassis_group_del_chassis.assert_called_once_with( - ovn_const.HA_CHASSIS_GROUP_DEFAULT_NAME, 'SpongeBob', - if_exists=True) + 'neutron-ha-ch-grp', 'SpongeBob', if_exists=True) def test_handle_ha_chassis_group_changes_delete(self): self._test_handle_ha_chassis_group_changes_delete( self.event.ROW_DELETE) - def test_handle_ha_chassis_group_changes_update_still_gw(self): + def test_handle_ha_chassis_group_changes_update_no_changes(self): # Assert nothing was done because the update didn't - # change the gateway chassis status + # change the gateway chassis status or the availability zones + ext_ids = { + 'ovn-cms-options': 'enable-chassis-as-gw,availability-zones=az-0'} + new = fakes.FakeOvsdbTable.create_one_ovsdb_table( + attrs={'name': 'SpongeBob', 'external_ids': ext_ids}) + old = new self.assertIsNone(self.event.handle_ha_chassis_group_changes( - self.event.ROW_UPDATE, mock.Mock(), mock.Mock())) + self.event.ROW_UPDATE, new, old)) self.assertFalse(self.nb_ovn.ha_chassis_group_add_chassis.called) self.assertFalse(self.nb_ovn.ha_chassis_group_del_chassis.called) 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 e7f58d72e6c..6b959c21f87 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 @@ -18,6 +18,7 @@ from unittest import mock import uuid import netaddr +from neutron_lib.api.definitions import availability_zone as az_def from neutron_lib.api.definitions import external_net from neutron_lib.api.definitions import extra_dhcp_opt as edo_ext from neutron_lib.api.definitions import portbindings @@ -1997,6 +1998,108 @@ 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() + 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() + + 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] + + fake_ha_ch = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'chassis_name': ch2.name, 'priority': 1}) + fake_ch_grp_uuid = 'fake-ha-ch-grp-uuid' + fake_ch_grp = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'uuid': fake_ch_grp_uuid, 'ha_chassis': [fake_ha_ch]}) + + self.nb_ovn.ha_chassis_group_get.return_value.execute.return_value = ( + fake_ch_grp) + + # Invoke the method + ret = self.mech_driver._ovn_client.sync_ha_chassis_group( + self.context, fake_net['id'], fake_txn) + + # Assert the UUID of the HA Chassis Group is returned + self.assertEqual(fake_ch_grp_uuid, ret) + + # Assert it attempts to add the chassis group for that network + ha_ch_grp_name = ovn_utils.ovn_name(fake_net['id']) + self.nb_ovn.ha_chassis_group_add.assert_called_once_with( + ha_ch_grp_name, may_exist=True) + + # Assert existing members that no longer belong to those + # AZs are removed + self.nb_ovn.ha_chassis_group_del_chassis.assert_called_once_with( + ha_ch_grp_name, ch2.name, if_exists=True) + + # Assert that only Chassis belonging to the AZ hints are + # added to the HA Chassis Group for that network + expected_calls = [ + mock.call(ha_ch_grp_name, ch0.name, priority=mock.ANY), + mock.call(ha_ch_grp_name, ch1.name, priority=mock.ANY)] + self.nb_ovn.ha_chassis_group_add_chassis.assert_has_calls( + expected_calls, any_order=True) + + 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: []} + 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() + + 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) + 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] + + fake_ha_ch = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'chassis_name': ch1.name, 'priority': 1}) + fake_ch_grp_uuid = 'fake-ha-ch-grp-uuid' + fake_ch_grp = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'uuid': fake_ch_grp_uuid, 'ha_chassis': [fake_ha_ch]}) + + self.nb_ovn.ha_chassis_group_get.return_value.execute.return_value = ( + fake_ch_grp) + + # Invoke the method + ret = self.mech_driver._ovn_client.sync_ha_chassis_group( + self.context, fake_net['id'], fake_txn) + + # Assert the UUID of the HA Chassis Group is returned + self.assertEqual(fake_ch_grp_uuid, ret) + + # Assert it attempts to add the chassis group for that network + ha_ch_grp_name = ovn_utils.ovn_name(fake_net['id']) + self.nb_ovn.ha_chassis_group_add.assert_called_once_with( + ha_ch_grp_name, may_exist=True) + + # Assert existing members that does belong to any AZ are removed + self.nb_ovn.ha_chassis_group_del_chassis.assert_called_once_with( + ha_ch_grp_name, ch1.name, if_exists=True) + + # Assert that only Chassis that are gateways and DOES NOT + # belong to any AZs are added + expected_calls = [ + mock.call(ha_ch_grp_name, ch2.name, priority=mock.ANY), + mock.call(ha_ch_grp_name, ch3.name, priority=mock.ANY)] + self.nb_ovn.ha_chassis_group_add_chassis.assert_has_calls( + expected_calls, any_order=True) + class OVNMechanismDriverTestCase(MechDriverSetupBase, test_plugin.Ml2PluginV2TestCase): @@ -3082,15 +3185,16 @@ class TestOVNMechanismDriverSecurityGroup(MechDriverSetupBase, @mock.patch('neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.' 'ovn_client.OVNClient.is_external_ports_supported', lambda *_: True) - def _test_create_port_with_vnic_type(self, vnic_type): + @mock.patch('neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.' + 'ovn_client.OVNClient.sync_ha_chassis_group') + def _test_create_port_with_vnic_type(self, vnic_type, sync_mock): fake_grp = 'fake-default-ha-group-uuid' - row = fakes.FakeOvsdbRow.create_one_ovsdb_row(attrs={'uuid': fake_grp}) - self.mech_driver.nb_ovn.ha_chassis_group_get.return_value.\ - execute.return_value = row + sync_mock.return_value = fake_grp with self.network() as n, self.subnet(n): + net_id = n['network']['id'] self._create_port( - self.fmt, n['network']['id'], + self.fmt, net_id, arg_list=(portbindings.VNIC_TYPE,), **{portbindings.VNIC_TYPE: vnic_type}) @@ -3101,6 +3205,7 @@ class TestOVNMechanismDriverSecurityGroup(MechDriverSetupBase, 1, self.mech_driver.nb_ovn.create_lswitch_port.call_count) 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, mock.ANY) def test_create_port_with_vnic_direct(self): self._test_create_port_with_vnic_type(portbindings.VNIC_DIRECT) diff --git a/neutron/tests/unit/services/ovn_l3/test_plugin.py b/neutron/tests/unit/services/ovn_l3/test_plugin.py index 66d8fee9aa2..ee26c9f759d 100644 --- a/neutron/tests/unit/services/ovn_l3/test_plugin.py +++ b/neutron/tests/unit/services/ovn_l3/test_plugin.py @@ -424,7 +424,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): ovn_const.OVN_GW_PORT_EXT_ID_KEY: '', ovn_const.OVN_REV_NUM_EXT_ID_KEY: '1', ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'router', - ovn_const.OVN_ROUTER_AZ_HINTS_EXT_ID_KEY: ''}) + ovn_const.OVN_AZ_HINTS_EXT_ID_KEY: ''}) @mock.patch('neutron.db.extraroute_db.ExtraRoute_dbonly_mixin.' 'update_router') @@ -443,7 +443,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): external_ids={ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'test', ovn_const.OVN_REV_NUM_EXT_ID_KEY: '1', ovn_const.OVN_GW_PORT_EXT_ID_KEY: '', - ovn_const.OVN_ROUTER_AZ_HINTS_EXT_ID_KEY: ''}) + ovn_const.OVN_AZ_HINTS_EXT_ID_KEY: ''}) @mock.patch.object(utils, 'get_lrouter_non_gw_routes') @mock.patch('neutron.db.l3_db.L3_NAT_dbonly_mixin.update_router') @@ -537,7 +537,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): external_ids = {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'router', ovn_const.OVN_REV_NUM_EXT_ID_KEY: '1', ovn_const.OVN_GW_PORT_EXT_ID_KEY: 'gw-port-id', - ovn_const.OVN_ROUTER_AZ_HINTS_EXT_ID_KEY: ''} + ovn_const.OVN_AZ_HINTS_EXT_ID_KEY: ''} self.l3_inst._ovn.create_lrouter.assert_called_once_with( 'neutron-router-id', external_ids=external_ids, enabled=True, options={}) @@ -1567,7 +1567,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): def _test_get_router_availability_zones(self, azs, expected): lr = fake_resources.FakeOvsdbRow.create_one_ovsdb_row( attrs={'id': 'fake-router', 'external_ids': { - ovn_const.OVN_ROUTER_AZ_HINTS_EXT_ID_KEY: azs}}) + ovn_const.OVN_AZ_HINTS_EXT_ID_KEY: azs}}) self.l3_inst._ovn.get_lrouter.return_value = lr azs_list = self.l3_inst.get_router_availability_zones(lr) self.assertEqual(sorted(expected), sorted(azs_list))