From 31e7167e7a95ca9dcf744124206c90c45be8b60d Mon Sep 17 00:00:00 2001 From: Martin Kalcok Date: Wed, 27 Aug 2025 19:17:04 +0200 Subject: [PATCH 01/23] ovn_db_sync: Improve coexistence support. neutron-ovn-db-sync-util synchronizes content between neutron's database and OVN NB/SB databases. As a side-effect, it can sometimes remove resources from OVN database that were not meant for neutron to manage. Coexistence support [0] aims to avoid these scenarios. The ovn_db_sync script already tries to avoid these unwanted removals by checking for presence of well known neutron external_ids for resources like "Logical Switch" and "Logical Switch Port" [1]. This change adds similar checks for: * Logical Router Port * Static Route * Port Group NAT rules are still missing the check because they don't have "neutron:" external_ids to check. In addition to the ovn_db_sync script, there is a 'maintenance' process that periodically updates OVN resources. This change also update its methods to not alter resources not owned by the neutron. [0] https://specs.openstack.org/openstack/neutron-specs/specs/2024.1/ml2ovn-coexistence-support-ovn-ext-resources.html [1] https://opendev.org/openstack/neutron/src/commit/f9067a719084710ee4f46fa31edb6a938e0dbbb0/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/impl_idl_ovn.py#L308-L316 Closes-bug: #2027742 Change-Id: I1434700928779577073d1369c0a2983a4076cc0e Signed-off-by: Martin Kalcok --- .../ovn/mech_driver/ovsdb/impl_idl_ovn.py | 16 ++- .../ovn/mech_driver/ovsdb/maintenance.py | 11 +- .../ovn/mech_driver/ovsdb/ovn_db_sync.py | 6 +- .../ovn/mech_driver/ovsdb/test_ovn_db_sync.py | 124 ++++++++++++++++-- .../mech_driver/ovsdb/test_impl_idl_ovn.py | 34 ++++- .../ovn/mech_driver/ovsdb/test_maintenance.py | 34 +++-- .../ovn/mech_driver/ovsdb/test_ovn_db_sync.py | 11 +- 7 files changed, 196 insertions(+), 40 deletions(-) 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 7c25b69049e..7282d0fa215 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 @@ -332,11 +332,17 @@ def get_all_logical_routers_with_rports(self): if ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY not in ( lrouter.external_ids): continue - lrports = {lrport.name.replace('lrp-', ''): lrport.networks - for lrport in getattr(lrouter, 'ports', [])} - sroutes = [{'destination': sroute.ip_prefix, - 'nexthop': sroute.nexthop} - for sroute in getattr(lrouter, 'static_routes', [])] + lrports = { + lrport.name.replace('lrp-', ''): lrport.networks + for lrport in getattr(lrouter, 'ports', []) + if ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY in lrport.external_ids + } + sroutes = [ + {'destination': route.ip_prefix, 'nexthop': route.nexthop} + for route in getattr(lrouter, 'static_routes', []) + if any(eid.startswith(constants.DEVICE_OWNER_NEUTRON_PREFIX) + for eid in route.external_ids) + ] dnat_and_snats = [] snat = [] 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 2783ec84102..95ee4c464eb 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -533,6 +533,8 @@ def check_for_igmp_snoop_support(self): cmds = [] for ls in self._nb_idl.ls_list().execute(check_error=True): + if ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY not in ls.external_ids: + continue snooping = ls.other_config.get(ovn_const.MCAST_SNOOP) flood = ls.other_config.get(ovn_const.MCAST_FLOOD_UNREGISTERED) @@ -572,9 +574,11 @@ def check_for_ha_chassis_group(self): context = n_context.get_admin_context() with self._nb_idl.transaction(check_error=True) as txn: for port in external_ports: - network_id = port.external_ids[ - ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY].replace( + network_id = port.external_ids.get( + ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY, '').replace( ovn_const.OVN_NAME_PREFIX, '') + if not network_id: + continue ha_ch_grp, high_prio_ch = utils.sync_ha_chassis_group_network( context, self._nb_idl, self._sb_idl, port.name, network_id, txn) @@ -1005,6 +1009,9 @@ def set_network_type(self): for seg in net_segments} cmds = [] for ls in self._nb_idl.ls_list().execute(check_error=True): + if ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY not in ls.external_ids: + continue + if ovn_const.OVN_NETTYPE_EXT_ID_KEY not in ls.external_ids: net_id = utils.get_neutron_name(ls.name) external_ids = { diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py index 9745ed183bb..29aa0f4a63e 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/ovn_db_sync.py @@ -196,7 +196,11 @@ def sync_port_groups(self, ctx): ovn_pgs = set() port_groups = self.ovn_api.db_list_rows('Port_Group').execute() or [] for pg in port_groups: - ovn_pgs.add(pg.name) + # Default neutron "drop pg" does NOT have any external IDs, but + # we still want to manage it, so we match it on its name. + if (ovn_const.OVN_SG_EXT_ID_KEY in pg.external_ids or + pg.name == ovn_const.OVN_DROP_PORT_GROUP_NAME): + ovn_pgs.add(pg.name) add_pgs = neutron_pgs.difference(ovn_pgs) remove_pgs = ovn_pgs.difference(neutron_pgs) diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py index 5140361b099..45604c41f6e 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py @@ -98,6 +98,12 @@ def setUp(self, *args): self.expected_dns_records = [] self.expected_ports_with_unknown_addr = [] self.expected_qos_records = [] + # Set of externally managed resources that should not + # be cleaned up by the sync_db + self.create_ext_port_groups = [] + self.create_ext_lrouter_ports = [] + self.create_ext_lrouter_routes = [] + self.ctx = context.get_admin_context() ovn_config.cfg.CONF.set_override('ovn_metadata_enabled', True, group='ovn') @@ -515,6 +521,9 @@ def _create_resources(self, restart_ovsdb_processes=False): self.create_lrouter_routes.append(('neutron-' + r1['id'], '10.13.0.0/24', '20.0.0.13')) + self.create_ext_lrouter_routes.append(('neutron-' + r1['id'], + '10.14.0.0/24', + '20.0.0.14')) self.delete_lrouter_routes.append(('neutron-' + r1['id'], '10.10.0.0/24', '20.0.0.10')) @@ -662,10 +671,18 @@ def _create_resources(self, restart_ovsdb_processes=False): 'neutron-' + r1['id'])) self.create_lrouter_ports.append(('lrp-' + uuidutils.generate_uuid(), 'neutron-' + r1['id'])) + self.create_ext_lrouter_ports.append( + ('ext-lrp-' + uuidutils.generate_uuid(), 'neutron-' + r1['id']) + ) + self.create_ext_lrouter_ports.append( + ('ext-lrp-' + uuidutils.generate_uuid(), 'neutron-' + r1['id']) + ) self.delete_lrouters.append('neutron-' + r2['id']) self.create_port_groups.extend([{'name': 'pg1', 'acls': []}, {'name': 'pg2', 'acls': []}]) + self.create_ext_port_groups.extend([{'name': 'ext-pg1', 'acls': []}, + {'name': 'ext-pg2', 'acls': []}]) self.delete_port_groups.append( utils.ovn_port_group_name(n1_prtr['port']['security_groups'][0])) # Create a network and subnet with orphaned OVN resources. @@ -790,7 +807,14 @@ def _modify_resources_in_nb_db(self): txn.add(self.nb_api.lr_del(lrouter_name, if_exists=True)) for lrport, lrouter_name in self.create_lrouter_ports: - txn.add(self.nb_api.add_lrouter_port(lrport, lrouter_name)) + external_ids = {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: + lrouter_name} + txn.add(self.nb_api.add_lrouter_port( + lrport, lrouter_name, True, external_ids=external_ids)) + + for lrport, lrouter_name in self.create_ext_lrouter_ports: + txn.add(self.nb_api.add_lrouter_port( + lrport, lrouter_name, True)) for lrport, lrouter_name, networks in self.update_lrouter_ports: txn.add(self.nb_api.update_lrouter_port( @@ -807,6 +831,10 @@ def _modify_resources_in_nb_db(self): ip_prefix=ip_prefix, nexthop=nexthop, **columns)) + for lr_name, ip_prefix, nexthop in self.create_ext_lrouter_routes: + txn.add(self.nb_api.add_static_route(lr_name, + ip_prefix=ip_prefix, + nexthop=nexthop)) routers = defaultdict(list) for lrouter_name, ip_prefix, nexthop in self.delete_lrouter_routes: routers[lrouter_name].append((ip_prefix, nexthop)) @@ -839,7 +867,12 @@ def _modify_resources_in_nb_db(self): txn.add(self.nb_api.delete_acl(lswitch_name, lport_name, True)) + columns = { + 'external_ids': {ovn_const.OVN_SG_EXT_ID_KEY: 'sg_uuid'}, + } for pg in self.create_port_groups: + txn.add(self.nb_api.pg_add(**pg, **columns)) + for pg in self.create_ext_port_groups: txn.add(self.nb_api.pg_add(**pg)) for pg in self.delete_port_groups: txn.add(self.nb_api.pg_del(pg)) @@ -1297,6 +1330,7 @@ def _get_ipv6_ra_configs_for_router_port(port): self.ctx, port)) return ipv6_ra_configs + neutron_prefix = constants.DEVICE_OWNER_NEUTRON_PREFIX for router_id in db_router_ids: r_ports = self._list('ports', query_params='device_id=%s' % (router_id)) @@ -1315,18 +1349,27 @@ def _get_ipv6_ra_configs_for_router_port(port): lrouter = idlutils.row_by_value( self.mech_driver.nb_ovn.idl, 'Logical_Router', 'name', 'neutron-' + str(router_id), None) - lports = getattr(lrouter, 'ports', []) + all_lports = getattr(lrouter, 'ports', []) + managed_lports = [ + lport for lport in all_lports + if (ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY in + lport.external_ids) + ] + plugin_lrouter_port_ids = [lport.name.replace('lrp-', '') - for lport in lports] + for lport in managed_lports] plugin_lport_networks = { lport.name.replace('lrp-', ''): lport.networks - for lport in lports} + for lport in managed_lports} plugin_lport_ra_configs = { lport.name.replace('lrp-', ''): lport.ipv6_ra_configs - for lport in lports} + for lport in managed_lports} sroutes = getattr(lrouter, 'static_routes', []) - plugin_routes = [sroute.ip_prefix + sroute.nexthop - for sroute in sroutes] + plugin_routes = [] + for sroute in sroutes: + if any(e_id.startswith(neutron_prefix) + for e_id in sroute.external_ids): + plugin_routes.append(sroute.ip_prefix + sroute.nexthop) nats = getattr(lrouter, 'nat', []) plugin_nats = [ nat.external_ip + nat.logical_ip + nat.type + @@ -1342,18 +1385,29 @@ def _get_ipv6_ra_configs_for_router_port(port): lrouter = idlutils.row_by_value( self.nb_api.idl, 'Logical_Router', 'name', 'neutron-' + router_id, None) - lports = getattr(lrouter, 'ports', []) + all_lports = getattr(lrouter, 'ports', []) + managed_lports = [ + lport for lport in all_lports + if (ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY in + lport.external_ids) + ] monitor_lrouter_port_ids = [lport.name.replace('lrp-', '') - for lport in lports] + for lport in managed_lports] monitor_lport_networks = { lport.name.replace('lrp-', ''): lport.networks - for lport in lports} + for lport in managed_lports} monitor_lport_ra_configs = { lport.name.replace('lrp-', ''): lport.ipv6_ra_configs - for lport in lports} + for lport in managed_lports} sroutes = getattr(lrouter, 'static_routes', []) - monitor_routes = [sroute.ip_prefix + sroute.nexthop - for sroute in sroutes] + monitor_routes = [] + for sroute in sroutes: + if any(e_id.startswith(neutron_prefix) + for e_id in sroute.external_ids): + monitor_routes.append( + sroute.ip_prefix + sroute.nexthop + ) + nats = getattr(lrouter, 'nat', []) monitor_nats = [ nat.external_ip + nat.logical_ip + nat.type + @@ -1511,7 +1565,9 @@ def _validate_port_groups(self, should_match=True): mn_pgs = [] for row in self.nb_api.tables['Port_Group'].rows.values(): - mn_pgs.append(getattr(row, 'name', '')) + if (ovn_const.OVN_SG_EXT_ID_KEY in row.external_ids or + row.name == ovn_const.OVN_DROP_PORT_GROUP_NAME): + mn_pgs.append(getattr(row, 'name', '')) if should_match: self.assertCountEqual(nb_pgs, db_pgs) @@ -1617,6 +1673,46 @@ def _test_ovn_nb_sync_helper(self, mode, modify_resources=True, self._sync_resources(mode) self._validate_resources(should_match=should_match_after_sync) + if not restart_ovsdb_processes: + # Restarting ovsdb-server removes all its previous content. + # We can not expect to find external resources in the DB + # if it was wiped out. + self._validate_external_resources() + + def _validate_external_resources(self): + """Ensure that resources not owned by Neutron are in the OVN DB. + + This function is useful to validate that external resources survived + ovn_db_sync. + """ + db_routers = self._list('routers') + db_router_ids = [router['id'] for router in db_routers['routers']] + + pgs = [] + for pg in self.nb_api.tables['Port_Group'].rows.values(): + pgs.append(pg.name) + + lrports = [] + sroutes = [] + for router_id in db_router_ids: + lrouter = idlutils.row_by_value( + self.mech_driver.nb_ovn.idl, 'Logical_Router', 'name', + 'neutron-' + str(router_id), None) + + for lrport in getattr(lrouter, 'ports', []): + lrports.append(lrport.name) + + for route in getattr(lrouter, 'static_routes', []): + sroutes.append(route.ip_prefix + route.nexthop) + + for port_name, _ in self.create_ext_lrouter_ports: + self.assertIn(port_name, lrports) + + for _, prefix, next_hop in self.create_ext_lrouter_routes: + self.assertIn(prefix + next_hop, sroutes) + + for ext_pg in self.create_ext_port_groups: + self.assertIn(ext_pg['name'], pgs) def test_ovn_nb_sync_repair(self): self._test_ovn_nb_sync_helper(ovn_const.OVN_DB_SYNC_MODE_REPAIR) diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py index 130f1ffc453..1ad0f923a6b 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl_ovn.py @@ -183,25 +183,47 @@ class TestNBImplIdlOvn(TestDBImplIdlOvn): 'networks': ['10.0.3.0/24'], 'options': {ovn_const.OVN_GATEWAY_CHASSIS_KEY: None}}, {'name': 'xrp-id-b1', - 'external_ids': {}, 'networks': ['20.0.1.0/24']}, + 'external_ids': { + ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: + utils.ovn_name('lr-id-b'), + }, 'networks': ['20.0.1.0/24']}, {'name': utils.ovn_lrouter_port_name('orp-id-b2'), - 'external_ids': {}, 'networks': ['20.0.2.0/24'], + 'external_ids': { + ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: + utils.ovn_name('lr-id-b'), + }, 'networks': ['20.0.2.0/24'], 'options': {ovn_const.OVN_GATEWAY_CHASSIS_KEY: 'host-2'}}, {'name': utils.ovn_lrouter_port_name('orp-id-b3'), - 'external_ids': {}, 'networks': ['20.0.3.0/24'], + 'external_ids': { + ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: + utils.ovn_name('lr-id-b'), + }, 'networks': ['20.0.3.0/24'], 'options': {}}, {'name': utils.ovn_lrouter_port_name('gwc'), 'external_ids': {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'lr-id-f', ovn_const.OVN_ROUTER_IS_EXT_GW: str(True)}, 'networks': ['10.0.4.0/24'], + 'options': {}}, + {'name': utils.ovn_lrouter_port_name('not-managed'), + 'external_ids': { + 'owner': 'not-owned-by-neutron', + }, + 'networks': ['10.0.5.0/24'], 'options': {}}], 'gateway_chassis': [ {'chassis_name': 'fake-chassis', 'name': utils.ovn_lrouter_port_name('gwc') + '_fake-chassis'}], 'static_routes': [{'ip_prefix': '20.0.0.0/16', - 'nexthop': '10.0.3.253'}, + 'nexthop': '10.0.3.253', + 'external_ids': { + ovn_const.OVN_SUBNET_EXT_ID_KEY: 'uuid_1'}}, {'ip_prefix': '10.0.0.0/16', - 'nexthop': '20.0.2.253'}], + 'nexthop': '20.0.2.253', + 'external_ids': { + ovn_const.OVN_SUBNET_EXT_ID_KEY: 'uuid_2'}}, + {'ip_prefix': '30.0.0.0/16', + 'nexthop': '30.0.4.253', + 'external_ids': {'owner': 'not-owned-by-neutron'}}], 'nats': [{'external_ip': '10.0.3.1', 'logical_ip': '20.0.0.0/16', 'type': 'snat'}, {'external_ip': '20.0.2.1', 'logical_ip': '10.0.0.0/24', @@ -481,7 +503,7 @@ def test_get_all_logical_switches_with_ports(self): def _test_get_all_logical_routers_with_rports(self, is_gw_port): # Test empty - mapping = self.nb_ovn_idl.get_all_logical_switches_with_ports() + mapping = self.nb_ovn_idl.get_all_logical_routers_with_rports() self.assertCountEqual(mapping, {}) # Test loaded values self._load_nb_db() 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 613c6563a2d..4acd893c8d6 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 @@ -401,30 +401,39 @@ def test_check_for_igmp_snoop_support(self): attrs={'name': 'ls0', 'other_config': { constants.MCAST_SNOOP: 'false', - constants.MCAST_FLOOD_UNREGISTERED: 'false'}}) + constants.MCAST_FLOOD_UNREGISTERED: 'false'}, + 'external_ids': {constants.OVN_NETWORK_NAME_EXT_ID_KEY: 'port0'}}) ls1 = fakes.FakeOvsdbRow.create_one_ovsdb_row( attrs={'name': 'ls1', - 'other_config': {}}) + 'other_config': {}, + 'external_ids': {constants.OVN_NETWORK_NAME_EXT_ID_KEY: 'port1'}}) ls2 = fakes.FakeOvsdbRow.create_one_ovsdb_row( attrs={'name': 'ls2', 'other_config': { constants.MCAST_SNOOP: 'true', - constants.MCAST_FLOOD_UNREGISTERED: 'false'}}) + constants.MCAST_FLOOD_UNREGISTERED: 'false'}, + 'external_ids': {constants.OVN_NETWORK_NAME_EXT_ID_KEY: 'port2'}}) ls3 = fakes.FakeOvsdbRow.create_one_ovsdb_row( attrs={'name': '', - 'other_config': {}}) + 'other_config': {}, + 'external_ids': {constants.OVN_NETWORK_NAME_EXT_ID_KEY: 'port3'}}) ls4 = fakes.FakeOvsdbRow.create_one_ovsdb_row( attrs={'name': '', - 'other_config': {constants.MCAST_SNOOP: 'false'}}) - + 'other_config': {constants.MCAST_SNOOP: 'false'}, + 'external_ids': {constants.OVN_NETWORK_NAME_EXT_ID_KEY: 'port4'}}) + ls5 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'name': 'ls5', + 'other_config': {}, + 'external_ids': {}}) nb_idl.ls_list.return_value.execute.return_value = [ls0, ls1, ls2, ls3, - ls4] + ls4, ls5] self.assertRaises(periodics.NeverAgain, self.periodic.check_for_igmp_snoop_support) # "ls2" is not part of the transaction because it already - # have the right value set; "ls3" and "ls4" do not have a name set. + # have the right value set; "ls3" and "ls4" do not have a name set; + # "ls5" is not managed by neutron. expected_calls = [ mock.call('Logical_Switch', 'ls0', ('other_config', { @@ -479,7 +488,14 @@ def test_check_for_ha_chassis_group(self, 'external_ids': { constants.OVN_NETWORK_NAME_EXT_ID_KEY: 'neutron-net1'}}) - nb_idl.db_find_rows.return_value.execute.return_value = [p0, p1] + # Port p2 is not owned by Neutron and should not be affected. + p2 = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'type': constants.LSP_TYPE_EXTERNAL, + 'name': 'p2', + 'ha_chassis_group': [hcg1], + 'external_ids': {}}) + + nb_idl.db_find_rows.return_value.execute.return_value = [p0, p1, p2] mock_sync_ha_chassis_group_network.return_value = hcg0.uuid, mock.ANY # Invoke the periodic method, it meant to run only once at startup diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py index cf9cd85eca6..74729509d86 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py @@ -147,7 +147,7 @@ def setUp(self): 'security_group_id': 'sg2'}], 'name': 'all-tcpe'}] - self.sg_port_groups_ovn = [mock.Mock(), mock.Mock(), mock.Mock()] + self.sg_port_groups_ovn = [mock.Mock(), mock.Mock(), mock.Mock(), mock.Mock()] self.sg_port_groups_ovn[0].configure_mock( name='pg_sg1', external_ids={ovn_const.OVN_SG_EXT_ID_KEY: 'sg1'}, @@ -159,8 +159,13 @@ def setUp(self): ports=[], acls=[]) self.sg_port_groups_ovn[2].configure_mock( - name='neutron_pg_drop', - external_ids=[], + name=ovn_const.OVN_DROP_PORT_GROUP_NAME, + external_ids={}, + ports=[], + acls=[]) + self.sg_port_groups_ovn[3].configure_mock( + name='external_pg', + external_ids={'owner': 'not-owned-by-neutron'}, ports=[], acls=[]) From 84468bb6a2c4169eae897de666d41300c627468f Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Thu, 25 Sep 2025 15:05:09 +0000 Subject: [PATCH 02/23] [OVN] Simplify the logic to create a "HA_Chassis_Group" This patch simplifies the logic for the "HA_Chassis_Group" creation. The method creates 3 list/sets: * A list/set of chassis to be deleted. * A list/set of chassis to be added. * A list/set of chassis to be kept. If the "HA_Chassis_Group" is already present and the highest priority chassis is not deleted, it must keep the highest priority to avoid rebinding the external ports into another chassis. Related-Bug: #2125553 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: Ibf3b4f108058387d4a9689d6411139e7727b1315 --- neutron/common/ovn/utils.py | 65 ++++---- .../ovn/mech_driver/test_mech_driver.py | 141 ++++++++++++++++++ 2 files changed, 172 insertions(+), 34 deletions(-) diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index e88a3d89149..2e186741f0e 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -1128,42 +1128,39 @@ def _sync_ha_chassis_group(nb_idl, hcg_info, txn): high_prio_ch_name = None # Check if the HA Chassis Group existed before. If so, re-calculate - # the canditates in case something changed and keep the highest priority + # the candidates 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 - candidates - for ch in ch_to_del: - txn.add(nb_idl.ha_chassis_group_del_chassis( - hcg_info.group_name, ch, if_exists=True)) - - # 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) - if (high_prio_ch and - high_prio_ch.chassis_name in candidates): - # If found, keep it as the highest priority chassis in the group - high_prio_ch_name = high_prio_ch.chassis_name - txn.add(nb_idl.ha_chassis_group_add_chassis( - hcg_info.group_name, high_prio_ch.chassis_name, - priority=priority)) - 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) - - # 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): + ch_existing_dict = { + ha_chassis.chassis_name: ha_chassis.priority for + ha_chassis in ha_ch_grp.ha_chassis} if ha_ch_grp else {} + ch_delete = set(ch_existing_dict) - candidates + ch_keep = set(ch_existing_dict) & candidates + # The number of chassis to add will depend on the chassis to keep and the + # maximum chassis number. + ch_add_list = list(candidates - set(ch_existing_dict)) + if ch_add_list: + num_to_add = min(max_chassis_number - len(ch_keep), len(ch_add_list)) + ch_add_list = random.sample(ch_add_list, num_to_add) + + # Delete chassis. + for ch in ch_delete: + txn.add(nb_idl.ha_chassis_group_del_chassis( + hcg_info.group_name, ch, if_exists=True)) + + # Create an ordered list (by priority) of chassis names. This list will + # contain: + # * First the current chassis to be kept and this list will be ordered + # with the current order; if the highest priority chassis is present, + # it will keep the highest priority again. + # * Second, the new chassis to be added. Because "ch_add" has been randomly + # generated, this order will be used. + for _delete in ch_delete: + ch_existing_dict.pop(_delete) + ch_ordered_list = sorted(list(ch_existing_dict.items()), + key=lambda x: x[1], reverse=True) + ch_ordered_list = [ch[0] for ch in ch_ordered_list] + ch_add_list + for ch in ch_ordered_list: txn.add(nb_idl.ha_chassis_group_add_chassis( hcg_info.group_name, ch, priority=priority)) priority -= 1 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 7bd6e1bc386..23cd5027914 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 @@ -3053,6 +3053,147 @@ def test_sync_ha_chassis_group_network_existing_group( self.nb_ovn.ha_chassis_group_add_chassis.assert_has_calls( expected_calls, any_order=True) + @staticmethod + def _create_fake_hcg(name, chassis_prio): + ha_chassis = [] + for chassis_name, prio in chassis_prio.items(): + hc = fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs={'chassis_name': chassis_name, 'priority': prio}) + ha_chassis.append(hc) + + hcg_attrs = {'name': name, 'ha_chassis': ha_chassis} + return fakes.FakeOvsdbRow.create_one_ovsdb_row( + attrs=hcg_attrs) + + def test__sync_ha_chassis_group_no_hcg(self): + network_id = uuidutils.generate_uuid() + hcg_info = self._build_hcg_info(network_id=network_id) + self.nb_ovn.lookup.return_value = None + ovn_utils._sync_ha_chassis_group(self.nb_ovn, hcg_info, mock.Mock()) + self.nb_ovn.ha_chassis_group_add.assert_called_once_with( + hcg_info.group_name, may_exist=True, + external_ids=hcg_info.external_ids) + self.nb_ovn.ha_chassis_group_del_chassis.assert_not_called() + add_calls = [ + 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( + add_calls, any_order=True) + + def test__sync_ha_chassis_group_hcg_no_delete(self): + network_id = uuidutils.generate_uuid() + hcg_info = self._build_hcg_info(network_id=network_id) + max_prio = ovn_const.HA_CHASSIS_GROUP_HIGHEST_PRIORITY + chassis_prio = { + 'ch0': max_prio, 'ch1': max_prio - 1, + 'ch2': max_prio - 2, 'ch3': max_prio - 3, + } + hcg = self._create_fake_hcg(hcg_info.group_name, chassis_prio) + self.nb_ovn.lookup.return_value = hcg + hcg_uuid, prio_chassis = ovn_utils._sync_ha_chassis_group( + self.nb_ovn, hcg_info, mock.Mock()) + + self.assertEqual(hcg.uuid, hcg_uuid) + self.assertEqual(prio_chassis, 'ch0') + self.nb_ovn.ha_chassis_group_del_chassis.assert_not_called() + add_calls = [ + mock.call(hcg_info.group_name, 'ch0', priority=max_prio), + mock.call(hcg_info.group_name, 'ch1', priority=max_prio - 1), + mock.call(hcg_info.group_name, 'ch2', priority=max_prio - 2), + mock.call(hcg_info.group_name, 'ch3', priority=max_prio - 3), + ] + self.nb_ovn.ha_chassis_group_add_chassis.assert_has_calls( + add_calls, any_order=True) + + def test__sync_ha_chassis_group_hcg_delete(self): + network_id = uuidutils.generate_uuid() + hcg_info = self._build_hcg_info(network_id=network_id, + with_ignore_chassis=True) + max_prio = ovn_const.HA_CHASSIS_GROUP_HIGHEST_PRIORITY + chassis_prio = { + 'ch0': max_prio, 'ch1': max_prio - 1, + 'ch2': max_prio - 2, 'ch3': max_prio - 3, + } + hcg = self._create_fake_hcg(hcg_info.group_name, chassis_prio,) + self.nb_ovn.lookup.return_value = hcg + hcg_uuid, prio_chassis = ovn_utils._sync_ha_chassis_group( + self.nb_ovn, hcg_info, mock.Mock()) + + self.assertEqual(hcg.uuid, hcg_uuid) + self.assertEqual(prio_chassis, 'ch0') + del_calls = [ + mock.call(hcg_info.group_name, 'ch1', if_exists=True), + mock.call(hcg_info.group_name, 'ch2', if_exists=True), + ] + self.nb_ovn.ha_chassis_group_del_chassis.assert_has_calls( + del_calls, any_order=True) + add_calls = [ + mock.call(hcg_info.group_name, 'ch0', priority=max_prio), + mock.call(hcg_info.group_name, 'ch3', priority=max_prio - 1), + ] + self.nb_ovn.ha_chassis_group_add_chassis.assert_has_calls( + add_calls, any_order=True) + + def test__sync_ha_chassis_group_hcg_new_chassis(self): + network_id = uuidutils.generate_uuid() + hcg_info = self._build_hcg_info(network_id=network_id) + max_prio = ovn_const.HA_CHASSIS_GROUP_HIGHEST_PRIORITY + chassis_prio = { + 'ch0': max_prio, 'ch1': max_prio - 1, + } + hcg = self._create_fake_hcg(hcg_info.group_name, chassis_prio) + self.nb_ovn.lookup.return_value = hcg + hcg_uuid, prio_chassis = ovn_utils._sync_ha_chassis_group( + self.nb_ovn, hcg_info, mock.Mock()) + + self.assertEqual(hcg.uuid, hcg_uuid) + self.assertEqual(prio_chassis, 'ch0') + self.nb_ovn.ha_chassis_group_del_chassis.assert_not_called() + add_calls = [ + mock.call(hcg_info.group_name, 'ch0', priority=max_prio), + mock.call(hcg_info.group_name, 'ch1', priority=max_prio - 1), + 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( + add_calls, any_order=True) + + def test__sync_ha_chassis_group_hcg_new_chassis_remove_highest(self): + network_id = uuidutils.generate_uuid() + hcg_info = self._build_hcg_info(network_id=network_id, + with_ignore_chassis=True) + max_prio = ovn_const.HA_CHASSIS_GROUP_HIGHEST_PRIORITY + # NOTE: the highest priority chassis (ch1) is removed. A new highest + # priority chassis is assigned. + chassis_prio = { + 'ch0': max_prio - 1, 'ch1': max_prio, + 'ch2': max_prio - 2, + } + hcg = self._create_fake_hcg(hcg_info.group_name, chassis_prio,) + self.nb_ovn.lookup.return_value = hcg + hcg_uuid, prio_chassis = ovn_utils._sync_ha_chassis_group( + self.nb_ovn, hcg_info, mock.Mock()) + + self.assertEqual(hcg.uuid, hcg_uuid) + self.assertEqual(prio_chassis, 'ch0') + del_calls = [ + mock.call(hcg_info.group_name, 'ch1', if_exists=True), + mock.call(hcg_info.group_name, 'ch2', if_exists=True), + ] + self.nb_ovn.ha_chassis_group_del_chassis.assert_has_calls( + del_calls, any_order=True) + # NOTE: because the chassis list already present in the HCG keeps the + # same order, ch0 will receive the highest priority. + add_calls = [ + mock.call(hcg_info.group_name, 'ch0', priority=max_prio), + mock.call(hcg_info.group_name, 'ch3', priority=max_prio - 1), + ] + self.nb_ovn.ha_chassis_group_add_chassis.assert_has_calls( + add_calls, any_order=True) + @mock.patch.object(mech_driver, 'LOG') def test_responsible_for_ports_allocation(self, mock_log): rp1 = str(place_utils.device_resource_provider_uuid( From 1271e85b2b57be58f54b131b808cf103cae48fcd Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Fri, 26 Sep 2025 20:35:26 +0000 Subject: [PATCH 03/23] [OVN] Remove maintenance method "check_for_ha_chassis_group" This method ensures that there is a "HA_Chassis_Group" for any external port created. This is already enforced by the API when an external port is created. This method is no longer needed. Related-Bug: #2125553 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: I931813395d790f0220a6a56a7224602325c791d2 --- .../ovn/mech_driver/ovsdb/maintenance.py | 32 -------- .../ovn/mech_driver/ovsdb/test_maintenance.py | 20 ----- .../ovn/mech_driver/ovsdb/test_maintenance.py | 73 ------------------- 3 files changed, 125 deletions(-) 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 95ee4c464eb..3a562762546 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -555,38 +555,6 @@ def check_for_igmp_snoop_support(self): raise periodics.NeverAgain() - # A static spacing value is used here, but this method will only run - # once per lock due to the use of periodics.NeverAgain(). - @has_lock_periodic( - periodic_run_limit=ovn_const.MAINTENANCE_TASK_RETRY_LIMIT, - spacing=ovn_const.MAINTENANCE_ONE_RUN_TASK_SPACING, - run_immediately=True) - 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(): - raise periodics.NeverAgain() - - 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 port in external_ports: - network_id = port.external_ids.get( - ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY, '').replace( - ovn_const.OVN_NAME_PREFIX, '') - if not network_id: - continue - ha_ch_grp, high_prio_ch = utils.sync_ha_chassis_group_network( - context, self._nb_idl, self._sb_idl, port.name, - network_id, txn) - txn.add(self._nb_idl.set_lswitch_port( - port.name, ha_chassis_group=ha_ch_grp)) - - raise periodics.NeverAgain() - # A static spacing value is used here, but this method will only run # once per lock due to the use of periodics.NeverAgain(). @has_lock_periodic( diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py index 5a7ea7a6158..704f248854d 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py @@ -1058,26 +1058,6 @@ def _verify_lb(test, protocol, vip_ext_port, vip_int_port): # Assert load balancer for port forwarding is gone self.assertFalse(self._find_pf_lb(router_id, fip_id)) - def test_check_for_ha_chassis_group(self): - net1 = self._create_network('network1test', external=False) - self._create_subnet('subnet1test', net1['id']) - p1 = self._create_port('testp1', net1['id'], vnic_type='direct') - - # Remove the HA Chassis Group register, created during the port - # creation. - self.nb_api.set_lswitch_port(p1['id'], ha_chassis_group=[]).execute( - check_error=True) - hcg_uuid = next(iter(self.nb_api._tables['HA_Chassis_Group'].rows)) - self.nb_api.ha_chassis_group_del(hcg_uuid).execute(check_error=True) - lsp = self.nb_api.lookup('Logical_Switch_Port', p1['id']) - self.assertEqual([], lsp.ha_chassis_group) - - self.assertRaises(periodics.NeverAgain, - self.maint.check_for_ha_chassis_group) - hcg_uuid = next(iter(self.nb_api._tables['HA_Chassis_Group'].rows)) - lsp = self.nb_api.lookup('Logical_Switch_Port', p1['id']) - self.assertEqual(hcg_uuid, lsp.ha_chassis_group[0].uuid) - def _test_check_provider_distributed_ports( self, is_distributed_fip, net_type, expected_value=None): cfg.CONF.set_override( 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 4acd893c8d6..11adbfc53e0 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 @@ -446,79 +446,6 @@ def test_check_for_igmp_snoop_support(self): ] nb_idl.db_set.assert_has_calls(expected_calls) - 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) - self.assertFalse( - self.fake_ovn_client._nb_idl.ha_chassis_group_add.called) - - @mock.patch.object(utils, 'sync_ha_chassis_group_network') - def test_check_for_ha_chassis_group_no_external_ports( - self, mock_sync_ha_chassis_group_network): - self.fake_ovn_client.is_external_ports_supported.return_value = True - nb_idl = self.fake_ovn_client._nb_idl - nb_idl.db_find_rows.return_value.execute.return_value = [] - self.assertRaises(periodics.NeverAgain, - self.periodic.check_for_ha_chassis_group) - self.assertFalse(mock_sync_ha_chassis_group_network.called) - - @mock.patch.object(utils, 'sync_ha_chassis_group_network') - def test_check_for_ha_chassis_group(self, - mock_sync_ha_chassis_group_network): - self.fake_ovn_client.is_external_ports_supported.return_value = True - nb_idl = self.fake_ovn_client._nb_idl - - 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'}}) - - # Port p2 is not owned by Neutron and should not be affected. - p2 = fakes.FakeOvsdbRow.create_one_ovsdb_row( - attrs={'type': constants.LSP_TYPE_EXTERNAL, - 'name': 'p2', - 'ha_chassis_group': [hcg1], - 'external_ids': {}}) - - nb_idl.db_find_rows.return_value.execute.return_value = [p0, p1, p2] - mock_sync_ha_chassis_group_network.return_value = hcg0.uuid, mock.ANY - - # 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) - - # Assert sync_ha_chassis_group_network() is called for both networks - expected_calls = [ - mock.call(mock.ANY, self.fake_ovn_client._nb_idl, - self.fake_ovn_client._sb_idl, 'p0', 'net0', mock.ANY), - mock.call(mock.ANY, self.fake_ovn_client._nb_idl, - self.fake_ovn_client._sb_idl, 'p1', 'net1', mock.ANY), - ] - mock_sync_ha_chassis_group_network.assert_has_calls(expected_calls, - any_order=True) - - expected_calls = [ - mock.call('p0', ha_chassis_group=hcg0.uuid), - mock.call('p1', ha_chassis_group=hcg0.uuid)] - nb_idl.set_lswitch_port.assert_has_calls(expected_calls, - any_order=True) - def test_check_localnet_port_has_learn_fdb(self): cfg.CONF.set_override('localnet_learn_fdb', 'True', group='ovn') From c5fe2f27ef792596b0a90de1b736638919354db3 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Fri, 26 Sep 2025 21:09:07 +0000 Subject: [PATCH 04/23] [OVN] Remove check for external ports support The field "ha_chassis_group" was added to the "Logical_Switch_Port" table in [1], that was released in OVN v20.03.0. Neutron enforces newer versions of OVN to work properly. [1]https://github.com/ovn-org/ovn/commit/b31c76000bef314b68e776d318d1ce4cf152450b Related-Bug: #2125553 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: Iff2ebe33d5ca454e06b914313ca124c105c4dedb --- .../plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py | 4 +++- .../notes/ovn-external-ports-supported-5895ea9c22d0aa0c.yaml | 5 +++++ 2 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 releasenotes/notes/ovn-external-ports-supported-5895ea9c22d0aa0c.yaml 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 23cd5027914..f1b4aec4ee0 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 @@ -1150,6 +1150,7 @@ def _test_set_port_status_up(self, mock_is_ext, mock_sync, is_extport_present=False): port_device_owner = 'compute:nova' if is_compute_port else '' self.mech_driver._plugin.nova_notifier = mock.Mock() + mock_sync.return_value = mock.Mock(), mock.Mock() mock_is_ext.return_value = is_extport_present self.sb_ovn.get_extport_chassis_from_cms_options.return_value = [ mock.Mock()] @@ -1187,9 +1188,10 @@ def _test_set_port_status_up(self, mock_is_ext, mock_sync, ulsp.assert_called_once_with(mock.ANY, mock.ANY) if is_extport_present: - mock_sync.assert_called_once_with( + sync_call = mock.call( mock.ANY, self.nb_ovn, self.sb_ovn, port1['port']['id'], port1['port']['network_id'], mock.ANY) + mock_sync.assert_has_calls([sync_call, sync_call]) else: mock_sync.assert_not_called() diff --git a/releasenotes/notes/ovn-external-ports-supported-5895ea9c22d0aa0c.yaml b/releasenotes/notes/ovn-external-ports-supported-5895ea9c22d0aa0c.yaml new file mode 100644 index 00000000000..a38ed389d2a --- /dev/null +++ b/releasenotes/notes/ovn-external-ports-supported-5895ea9c22d0aa0c.yaml @@ -0,0 +1,5 @@ +--- +other: + - | + It is assumed now that OVN supports external ports. This functionality + was added in OVN v20.03.0. From e601feaeaecd8f3ef532d06219a1cca524ae121b Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Mon, 29 Sep 2025 14:25:32 +0000 Subject: [PATCH 05/23] [OVN] Add the physical network to the ``Logical_Switch`` register Now the ``Logical_Switch`` register (that represents an OVN network), stored the physical in the "external_ids" field, if the network has it. If not, this field won't be present. Related-Bug: #2125553 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: I4e7c78da3bc0367f7208477a4d84dd970c9d366c --- .../ovn/mech_driver/ovsdb/maintenance.py | 22 +++++---- .../ovn/mech_driver/ovsdb/ovn_client.py | 2 + .../ovn/mech_driver/ovsdb/test_maintenance.py | 48 ++++++++++++++----- 3 files changed, 51 insertions(+), 21 deletions(-) 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 3a562762546..1f3bffcb4c8 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -964,26 +964,32 @@ def remove_invalid_gateway_chassis_from_unbound_lrp(self): raise periodics.NeverAgain() - # TODO(ralonsoh): Remove this method in the E+2 cycle (SLURP release) + # TODO(ralonsoh): Remove this method in the G+4 cycle (SLURP release) @has_lock_periodic( periodic_run_limit=ovn_const.MAINTENANCE_TASK_RETRY_LIMIT, spacing=ovn_const.MAINTENANCE_ONE_RUN_TASK_SPACING, run_immediately=True) - def set_network_type(self): - """Add the network type to the Logical_Switch registers""" + def set_network_type_and_physnet(self): + """Add the network type and physnet to the Logical_Switch registers""" context = n_context.get_admin_context() net_segments = network_obj.NetworkSegment.get_objects(context) - net_segments = {seg.network_id: seg.network_type - for seg in net_segments} + net_type = {seg.network_id: seg.network_type for seg in net_segments} + net_physnet = {seg.network_id: seg.physical_network + for seg in net_segments} cmds = [] for ls in self._nb_idl.ls_list().execute(check_error=True): if ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY not in ls.external_ids: continue - if ovn_const.OVN_NETTYPE_EXT_ID_KEY not in ls.external_ids: - net_id = utils.get_neutron_name(ls.name) + net_id = utils.get_neutron_name(ls.name) + physnet = net_physnet[net_id] + if (ovn_const.OVN_NETTYPE_EXT_ID_KEY not in ls.external_ids or + (ovn_const.OVN_PHYSNET_EXT_ID_KEY not in ls.external_ids + and physnet)): external_ids = { - ovn_const.OVN_NETTYPE_EXT_ID_KEY: net_segments[net_id]} + ovn_const.OVN_NETTYPE_EXT_ID_KEY: net_type[net_id]} + if physnet: + external_ids[ovn_const.OVN_PHYSNET_EXT_ID_KEY] = physnet cmds.append(self._nb_idl.db_set( 'Logical_Switch', ls.uuid, ('external_ids', external_ids))) 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 0265c9768d1..53a2f410633 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 @@ -2110,6 +2110,8 @@ def _gen_network_parameters(self, # NOTE(twilson): in the case of multiple segments, or when all # segments are removed, NETWORK_TYPE=None, which is invalid ovsdb ovn_const.OVN_NETTYPE_EXT_ID_KEY: network.get(pnet.NETWORK_TYPE), + ovn_const.OVN_PHYSNET_EXT_ID_KEY: + network.get(pnet.PHYSICAL_NETWORK), } # Enable IGMP snooping if igmp_snooping_enable is enabled in Neutron diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py index 704f248854d..aa9aa8949ba 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_maintenance.py @@ -1238,20 +1238,42 @@ def test_remove_invalid_gateway_chassis_from_unbound_lrp(self): lr = self.nb_api.lookup('Logical_Router', utils.ovn_name(router['id'])) self.assertEqual([], lr.ports[0].gateway_chassis) - def test_set_network_type(self): + def test_set_network_type_and_physnet(self): net1 = self._create_network(uuidutils.generate_uuid()) - ls_name = utils.ovn_name(net1['id']) - self.nb_api.db_remove( - 'Logical_Switch', ls_name, 'external_ids', - ovn_const.OVN_NETTYPE_EXT_ID_KEY).execute(check_error=True) - ls = self.nb_api.lookup('Logical_Switch', ls_name) - self.assertIsNone(ls.external_ids.get( - ovn_const.OVN_NETTYPE_EXT_ID_KEY)) - - self.assertRaises(periodics.NeverAgain, self.maint.set_network_type) - ls = self.nb_api.lookup('Logical_Switch', ls_name) - self.assertEqual(net1[provnet_apidef.NETWORK_TYPE], - ls.external_ids.get(ovn_const.OVN_NETTYPE_EXT_ID_KEY)) + net2 = self._create_network(uuidutils.generate_uuid(), + provider='physnet1', net_type='vlan') + ls1_name = utils.ovn_name(net1['id']) + ls2_name = utils.ovn_name(net2['id']) + for _ls_name in (ls1_name, ls2_name): + self.nb_api.db_remove( + 'Logical_Switch', _ls_name, 'external_ids', + ovn_const.OVN_NETTYPE_EXT_ID_KEY).execute(check_error=True) + self.nb_api.db_remove( + 'Logical_Switch', _ls_name, 'external_ids', + ovn_const.OVN_PHYSNET_EXT_ID_KEY).execute(check_error=True) + ls = self.nb_api.lookup('Logical_Switch', _ls_name) + self.assertIsNone(ls.external_ids.get( + ovn_const.OVN_NETTYPE_EXT_ID_KEY)) + self.assertIsNone(ls.external_ids.get( + ovn_const.OVN_PHYSNET_EXT_ID_KEY)) + + self.assertRaises(periodics.NeverAgain, + self.maint.set_network_type_and_physnet) + ls1 = self.nb_api.lookup('Logical_Switch', ls1_name) + self.assertEqual( + net1[provnet_apidef.NETWORK_TYPE], + ls1.external_ids.get(ovn_const.OVN_NETTYPE_EXT_ID_KEY)) + self.assertNotIn( + ovn_const.OVN_PHYSNET_EXT_ID_KEY, + ls1.external_ids.get) + + ls2 = self.nb_api.lookup('Logical_Switch', ls2_name) + self.assertEqual( + net2[provnet_apidef.NETWORK_TYPE], + ls2.external_ids.get(ovn_const.OVN_NETTYPE_EXT_ID_KEY)) + self.assertEqual( + net2[provnet_apidef.PHYSICAL_NETWORK], + ls2.external_ids.get(ovn_const.OVN_PHYSNET_EXT_ID_KEY)) def test_check_network_broadcast_arps_to_all_routers(self): net = self._create_network('net', external=True) From 833a6ee17501df436b64dc361171d9078a50dbc7 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Mon, 29 Sep 2025 10:34:04 +0000 Subject: [PATCH 06/23] [OVN] Add method ``sync_ha_chassis_group_network_unified`` This new method is meant to be used when a network with external ports is connected to a router. In this case, it is needed to create a single scheduler for all the ports connected to the router and matching the same chassis as the gateway router port. This method uses the network ID to assign the group name. It also uses a defined chassis priority (that will match the gateway router port assignation). The created ``HA_Chassis_Group`` must have a set of ``HA_Chassis`` registers that should have the same chassis name and priority as the gateway router port ``Gateway_Chassis``. NOTE: in future developments, the router port ``Gateway_Chassis`` will be replaced with a ``HA_Chassis_Group`` register. Related-Bug: #2125553 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: Ia4f685077a8d72bf28f66daf21225d96f57ddef6 --- neutron/common/ovn/constants.py | 1 + neutron/common/ovn/utils.py | 92 +++++++++++++++++-- .../tests/functional/common/ovn/test_utils.py | 49 ++++++++++ .../ovn/mech_driver/test_mech_driver.py | 6 +- 4 files changed, 140 insertions(+), 8 deletions(-) diff --git a/neutron/common/ovn/constants.py b/neutron/common/ovn/constants.py index 9cf83222d88..17ff55d20d1 100644 --- a/neutron/common/ovn/constants.py +++ b/neutron/common/ovn/constants.py @@ -22,6 +22,7 @@ OVN_SG_RULE_EXT_ID_KEY = 'neutron:security_group_rule_id' OVN_ML2_MECH_DRIVER_NAME = 'ovn' OVN_NETWORK_NAME_EXT_ID_KEY = 'neutron:network_name' +OVN_NETWORK_ID_EXT_ID_KEY = 'neutron:network_id' OVN_NETWORK_MTU_EXT_ID_KEY = 'neutron:mtu' OVN_PORT_NAME_EXT_ID_KEY = 'neutron:port_name' OVN_PORT_EXT_ID_KEY = 'neutron:port_id' diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index 2e186741f0e..34d27bd263c 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -16,6 +16,7 @@ import inspect import os import random +import typing import netaddr from neutron_lib.api.definitions import external_net @@ -64,9 +65,32 @@ BPInfo = collections.namedtuple( 'BPInfo', ['bp_param', 'vnic_type', 'capabilities']) -HAChassisGroupInfo = collections.namedtuple( - 'HAChassisGroupInfo', ['group_name', 'chassis_list', 'az_hints', - 'ignore_chassis', 'external_ids']) +_OVS_PERSIST_UUID = _SENTINEL = object() + + +class HAChassisGroupInfo: + def __init__(self, + group_name: str, + chassis_list: list[typing.Any], + az_hints: list[str], + ignore_chassis: set[str], + external_ids: dict[str, typing.Any], + priority: dict[str, typing.Any] | None = None): + if priority: + # If present, the "priority" dictionary must contain all the + # chassis names present in "chassis_list". + ch_name_list = [ch.name for ch in chassis_list] + if sorted(ch_name_list) != sorted(list(priority.keys())): + raise RuntimeError(_( + 'In a "HAChassisGroupInfo", the "chassis_list" must have ' + 'the same chassis as the "priority" dictionary')) + + self.group_name = group_name + self.chassis_list = chassis_list + self.az_hints = az_hints + self.ignore_chassis = ignore_chassis + self.external_ids = external_ids + self.priority = priority class OvsdbClientCommand: @@ -1110,6 +1134,16 @@ def _sync_ha_chassis_group(nb_idl, hcg_info, txn): :returns: The HA Chassis Group UUID or the HA Chassis Group command object, The name of the Chassis with the highest priority (could be None) """ + def get_priority(ch_name): + nonlocal priority + nonlocal hcg_info + if hcg_info.priority: + return hcg_info.priority[ch_name] + + _priority = int(priority) + priority -= 1 + return _priority + # If there are Chassis marked for hosting external ports create a HA # Chassis Group per external port, otherwise do it at the network level candidates = _filter_candidates_for_ha_chassis_group(hcg_info) @@ -1162,8 +1196,7 @@ def _sync_ha_chassis_group(nb_idl, hcg_info, txn): ch_ordered_list = [ch[0] for ch in ch_ordered_list] + ch_add_list for ch in ch_ordered_list: txn.add(nb_idl.ha_chassis_group_add_chassis( - hcg_info.group_name, ch, priority=priority)) - priority -= 1 + hcg_info.group_name, ch, priority=get_priority(ch))) if not high_prio_ch_name: high_prio_ch_name = ch @@ -1221,13 +1254,60 @@ def sync_ha_chassis_group_network(context, nb_idl, sb_idl, port_id, plugin = directory.get_plugin() resource = plugin.get_network(context, network_id) az_hints = common_utils.get_az_hints(resource) - external_ids = {constants.OVN_AZ_HINTS_EXT_ID_KEY: ','.join(az_hints)} + external_ids = {constants.OVN_AZ_HINTS_EXT_ID_KEY: ','.join(az_hints), + constants.OVN_NETWORK_ID_EXT_ID_KEY: network_id, + } hcg_info = HAChassisGroupInfo( group_name=group_name, chassis_list=chassis_list, az_hints=az_hints, ignore_chassis=ignore_chassis, external_ids=external_ids) return _sync_ha_chassis_group(nb_idl, hcg_info, txn) +@ovn_context(idl_var_name='nb_idl') +def sync_ha_chassis_group_network_unified(context, nb_idl, sb_idl, network_id, + router_id, chassis_prio, txn): + """Creates a single HA_Chassis_Group for a given network + + This method creates a single HA_Chassis_Group for a network. This method + is called when a network with external ports is connected to a router; + in order to provide N/S connectivity all external ports need to be bound + to the same chassis as the gateway Logical_Router_Port. + + The chassis list and the priority is already provided. This method checks + if all gateway chassis provided have external connectivity to this network. + """ + chassis_physnets = sb_idl.get_chassis_and_physnets() + group_name = ovn_name(network_id) + ls = nb_idl.get_lswitch(group_name) + + # It is expected to be called for a non-tunnelled network with a physical + # network assigned. + physnet = ls.external_ids.get(constants.OVN_PHYSNET_EXT_ID_KEY) + if physnet: + missing_mappings = set() + for ch_name in chassis_prio: + if physnet not in chassis_physnets[ch_name]: + missing_mappings.add(ch_name) + + if missing_mappings: + LOG.warning('The following chassis do not have mapped the ' + f'physical network {physnet}: {missing_mappings}') + + chassis_list = [sb_idl.lookup('Chassis', ch_name, None) + for ch_name in chassis_prio.keys()] + plugin = directory.get_plugin() + resource = plugin.get_network(context, network_id) + az_hints = common_utils.get_az_hints(resource) + external_ids = {constants.OVN_AZ_HINTS_EXT_ID_KEY: ','.join(az_hints), + constants.OVN_NETWORK_ID_EXT_ID_KEY: network_id, + constants.OVN_ROUTER_ID_EXT_ID_KEY: router_id, + } + hcg_info = HAChassisGroupInfo( + group_name=group_name, chassis_list=chassis_list, az_hints=az_hints, + ignore_chassis=set(), external_ids=external_ids, priority=chassis_prio) + return _sync_ha_chassis_group(nb_idl, hcg_info, txn) + + def get_port_type_virtual_and_parents(subnets_by_id, fixed_ips, network_id, port_id, nb_idl): """Returns if a port is type virtual and its corresponding parents. diff --git a/neutron/tests/functional/common/ovn/test_utils.py b/neutron/tests/functional/common/ovn/test_utils.py index ffdd7816e3d..df98413b3f9 100644 --- a/neutron/tests/functional/common/ovn/test_utils.py +++ b/neutron/tests/functional/common/ovn/test_utils.py @@ -14,6 +14,7 @@ import ddt from neutron_lib.api.definitions import portbindings +from neutron_lib.api.definitions import provider_net from oslo_utils import uuidutils from ovsdbapp.backend.ovs_idl import event from ovsdbapp.backend.ovs_idl import idlutils @@ -211,6 +212,54 @@ def test_sync_ha_chassis_group_network_extport(self): self.nb_api.ha_chassis_group_get(hcg_name).execute, check_error=True) + def _test_sync_unify_ha_chassis_group_network(self, create_hcg=False): + physnet = 'physnet1' + net_ext_args = {provider_net.NETWORK_TYPE: 'vlan', + provider_net.PHYSICAL_NETWORK: physnet, + external_net.EXTERNAL: True} + net_ext = self._make_network(self.fmt, 'test-ext-net', True, + as_admin=True, + arg_list=tuple(net_ext_args.keys()), + **net_ext_args)['network'] + other_config = {'ovn-bridge-mappings': physnet + ':br-ex'} + ch1 = self.add_fake_chassis('host1', azs=[], enable_chassis_as_gw=True, + other_config=other_config) + ch2 = self.add_fake_chassis('host2', azs=[], enable_chassis_as_gw=True, + other_config=other_config) + ch3 = self.add_fake_chassis('host3', azs=[], enable_chassis_as_gw=True) + group_name = utils.ovn_name(net_ext['id']) + + # Create a pre-existing HCG. + if create_hcg: + chassis_list = [self.sb_api.lookup('Chassis', ch2)] + hcg_info = utils.HAChassisGroupInfo( + group_name=group_name, chassis_list=chassis_list, + az_hints=[], ignore_chassis=set(), external_ids={}) + with self.nb_api.transaction(check_error=True) as txn: + utils._sync_ha_chassis_group(self.nb_api, hcg_info, txn) + hcg = self.nb_api.lookup('HA_Chassis_Group', group_name) + self.assertEqual(1, len(hcg.ha_chassis)) + self.assertEqual(ovn_const.HA_CHASSIS_GROUP_HIGHEST_PRIORITY, + hcg.ha_chassis[0].priority) + + # Invoke the sync method + chassis_prio = {ch1: 10, ch2: 20, ch3: 30} + with self.nb_api.transaction(check_error=True) as txn: + utils.sync_ha_chassis_group_network_unified( + self.context, self.nb_api, self.sb_api, net_ext['id'], + 'router-id', chassis_prio, txn) + + hcg = self.nb_api.lookup('HA_Chassis_Group', group_name) + self.assertEqual(3, len(hcg.ha_chassis)) + for hc in hcg.ha_chassis: + self.assertEqual(chassis_prio[hc.chassis_name], hc.priority) + + def test_sync_unify_ha_chassis_group_network_no_hcg(self): + self._test_sync_unify_ha_chassis_group_network() + + def test_sync_unify_ha_chassis_group_network_existing_hcg(self): + self._test_sync_unify_ha_chassis_group_network(create_hcg=True) + @utils.ovn_context() def method_with_idl_and_default_txn(ls_name, idl, txn=None): 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 f1b4aec4ee0..22c4a9dbbe8 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 @@ -2987,8 +2987,10 @@ def test_sync_ha_chassis_group_network(self, mock_candidates, *args): 'fake-net-id', fake_txn) # Assert it creates the HA Chassis Group - ext_ids = {ovn_const.OVN_AZ_HINTS_EXT_ID_KEY: - ','.join(hcg_info.az_hints)} + ext_ids = { + ovn_const.OVN_AZ_HINTS_EXT_ID_KEY: ','.join(hcg_info.az_hints), + ovn_const.OVN_NETWORK_ID_EXT_ID_KEY: 'fake-net-id', + } self.nb_ovn.ha_chassis_group_add.assert_called_once_with( hcg_info.group_name, may_exist=True, external_ids=ext_ids) From b4890b958184592f8a1b8a0c833d9fb7168cc20a Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Wed, 1 Oct 2025 10:27:55 +0000 Subject: [PATCH 07/23] [OVN] ``DelLSwitchPortCommand`` also deletes external ports HCG The ``DelLSwitchPortCommand`` now removes the external ports ``HA_Chassis_Group`` associated registers. It is not needed to handle it outside the deletion command. Related-Bug: #2125553 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: I1e7b8a1df23c6dab2a8de604f18063fbcf6aeef7 --- .../drivers/ovn/mech_driver/ovsdb/commands.py | 8 +++ .../ovn/mech_driver/ovsdb/ovn_client.py | 9 ---- .../tests/functional/common/ovn/test_utils.py | 6 ++- .../ovn/mech_driver/test_mech_driver.py | 51 +++++++++++++++---- .../ovn/mech_driver/ovsdb/test_commands.py | 5 +- 5 files changed, 57 insertions(+), 22 deletions(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py index 687db6bd4c1..ffad2ac0f96 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py @@ -296,6 +296,14 @@ def run_idl(self, txn): for uuid in cur_port_dhcp_opts: self.api._tables['DHCP_Options'].rows[uuid].delete() + # Delete the HA_Chassis_Group associated to an external port. + if (lport.type == ovn_const.LSP_TYPE_EXTERNAL and + lport.ha_chassis_group): + hcg = lport.ha_chassis_group[0] + lport.delvalue('ha_chassis_group', hcg) + if hcg.name == utils.ovn_extport_chassis_group_name(lport.name): + hcg.delete() + _delvalue_from_list(lswitch, 'ports', lport) self.api._tables['Logical_Switch_Port'].rows[lport.uuid].delete() 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 53a2f410633..9357e42029c 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 @@ -855,15 +855,6 @@ def _delete_port(self, port_id, port_object=None): 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): diff --git a/neutron/tests/functional/common/ovn/test_utils.py b/neutron/tests/functional/common/ovn/test_utils.py index df98413b3f9..59f4cbddde4 100644 --- a/neutron/tests/functional/common/ovn/test_utils.py +++ b/neutron/tests/functional/common/ovn/test_utils.py @@ -155,9 +155,13 @@ def test_sync_ha_chassis_group_network_extport(self): # Invoke the sync method with self.nb_api.transaction(check_error=True) as txn: - utils.sync_ha_chassis_group_network( + hcg, _ = utils.sync_ha_chassis_group_network( self.context, self.nb_api, self.sb_api, port['id'], net['id'], txn) + # It is needed to assign the HCG to the LSP. When the port is + # deleted, the external port HCG associated will be deleted too. + txn.add( + self.nb_api.set_lswitch_port(port['id'], ha_chassis_group=hcg)) # Assert only the eligible chassis are present in HA Chassis ha_chassis = self.nb_api.db_find('HA_Chassis').execute( 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 d1d5b7fcef6..82fcbb2fcda 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 @@ -19,6 +19,7 @@ import time from unittest import mock +import ddt import netaddr from neutron_lib.api.definitions import external_net from neutron_lib.api.definitions import portbindings @@ -679,6 +680,7 @@ def test_virtual_port_not_set_similiar_address(self): ovn_vport.options) +@ddt.ddt class TestExternalPorts(base.TestOVNFunctionalBase): def setUp(self): @@ -695,7 +697,12 @@ def _find_port_row_by_name(self, name): rows = cmd.execute(check_error=True) return rows[0] if rows else None - def _test_external_port_create(self, vnic_type): + def _test_external_port_create_and_delete( + self, vnic_type, enable_as_gw): + for host in ('host1', 'host2', 'host3'): + self.add_fake_chassis( + host, enable_chassis_as_gw=enable_as_gw, + enable_chassis_as_extport=not enable_as_gw) net_id = self.n1['network']['id'] port_data = { 'port': {'network_id': net_id, @@ -707,10 +714,25 @@ def _test_external_port_create(self, vnic_type): port = self.deserialize(self.fmt, port_res)['port'] ovn_port = self._find_port_row_by_name(port['id']) + hcg_name = str(ovn_port.ha_chassis_group[0].name) self.assertEqual(ovn_const.LSP_TYPE_EXTERNAL, ovn_port.type) self.assertEqual(1, len(ovn_port.ha_chassis_group)) - self.assertEqual(utils.ovn_name(net_id), - str(ovn_port.ha_chassis_group[0].name)) + group_name = (utils.ovn_name(net_id) if enable_as_gw else + utils.ovn_extport_chassis_group_name(port['id'])) + self.assertEqual(group_name, hcg_name) + hcg = self.nb_api.lookup('HA_Chassis_Group', hcg_name) + self.assertEqual(hcg_name, hcg.name) + + port_req = self.new_delete_request('ports', port['id']) + port_req.get_response(self.api) + hcg = self.nb_api.lookup('HA_Chassis_Group', hcg_name, None) + if enable_as_gw: + self.assertEqual(hcg_name, hcg.name) + else: + # If the HCG has been created only for this port (that happens + # when there are chassis for external ports), it should be deleted + # along with the port. + self.assertIsNone(hcg) def _create_router_port(self, vnic_type): net_id = self.n1['network']['id'] @@ -783,14 +805,21 @@ def get_count(self): n_utils.wait_until_true(lambda: test_up_event.get_count() == 1, timeout=10) - def test_external_port_create_vnic_direct(self): - self._test_external_port_create(portbindings.VNIC_DIRECT) - - def test_external_port_create_vnic_direct_physical(self): - self._test_external_port_create(portbindings.VNIC_DIRECT_PHYSICAL) - - def test_external_port_create_vnic_macvtap(self): - self._test_external_port_create(portbindings.VNIC_MACVTAP) + @ddt.data(True, False) + def test_external_port_create_and_delete_vnic_direct(self, enable_as_gw): + self._test_external_port_create_and_delete( + portbindings.VNIC_DIRECT, enable_as_gw) + + @ddt.data(True, False) + def test_external_port_create_and_delete_direct_physical( + self, enable_as_gw): + self._test_external_port_create_and_delete( + portbindings.VNIC_DIRECT_PHYSICAL, enable_as_gw) + + @ddt.data(True, False) + def test_external_port_create_and_delete_vnic_macvtap(self, enable_as_gw): + self._test_external_port_create_and_delete( + portbindings.VNIC_MACVTAP, enable_as_gw) def _test_external_port_update(self, vnic_type): net_id = self.n1['network']['id'] diff --git a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py index 824600aa092..f9f72d7aa99 100644 --- a/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py +++ b/neutron/tests/unit/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_commands.py @@ -402,7 +402,10 @@ def _test_lswitch_port_del_delete_dhcp_opt(self, dhcpv4_opt_ext_ids, attrs={'name': 'lsp', 'external_ids': ext_ids, 'dhcpv4_options': [fake_dhcpv4_options], - 'dhcpv6_options': [fake_dhcpv6_options]}) + 'dhcpv6_options': [fake_dhcpv6_options], + 'type': '', + }, + ) self.ovn_api._tables['Logical_Switch_Port'].rows[fake_lsp.uuid] = \ fake_lsp fake_lswitch = fakes.FakeOvsdbRow.create_one_ovsdb_row( From ac4db0df470b7ea94daf9c757c7d982e4e20d747 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Wed, 24 Sep 2025 11:49:21 +0000 Subject: [PATCH 08/23] [OVN] Always exclude the local chassis creating ``HA_Chassis_Group`` When a ``HA_Chassis_Group`` for an external port is created, the method always discards the chassis bound to the port. A logical port can have several chassis stored in the ``Port_Binding`` register, in the ``name`` field (the main one) and in the ``additional_chassis`` list (see method ``get_chassis_host_for_port``). Closes-Bug: #2125569 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: I3346c2a16f69c3b16d1693db42335b133a721bf9 --- neutron/common/ovn/utils.py | 9 ++++----- .../ml2/drivers/ovn/mech_driver/test_mech_driver.py | 7 +++++-- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index 34d27bd263c..add38184808 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -1237,20 +1237,19 @@ def sync_ha_chassis_group_network(context, nb_idl, sb_idl, port_id, 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) - ignore_chassis = set() LOG.debug('HA Chassis Group %s is based on network %s', group_name, network_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) plugin = directory.get_plugin() resource = plugin.get_network(context, network_id) az_hints = common_utils.get_az_hints(resource) 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 22c4a9dbbe8..5a72961d9f7 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 @@ -2880,7 +2880,9 @@ def test_sync_ha_chassis_group_network_as_gw(self, mock_sync_hcg): 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, ch5] - + expected_ignore_chassis = [ch4.name, ch5.name] + self.sb_ovn.get_chassis_host_for_port.return_value = set( + expected_ignore_chassis) ovn_utils.sync_ha_chassis_group_network( self.context, self.nb_ovn, self.sb_ovn, fake_port['id'], fake_net['id'], None) @@ -2894,7 +2896,8 @@ def test_sync_ha_chassis_group_network_as_gw(self, mock_sync_hcg): 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) + self.assertEqual(sorted(expected_ignore_chassis), + sorted(hcg_info.ignore_chassis)) @mock.patch.object(ovn_utils, '_sync_ha_chassis_group') def test_sync_ha_chassis_group_router(self, mock_sync_hcg): From ca714692970f7a9d912d10152d15cfb776ed2015 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Date: Wed, 8 Oct 2025 13:27:33 +0000 Subject: [PATCH 09/23] Revert "[OVN] Always exclude the local chassis creating ``HA_Chassis_Group``" This reverts commit 4238ccb45069eb75c1af960d367080e731414a8d. Reason for revert: this patch is breaking the Ironic CI. The external can be bound to a gateway chassis and yet this chassis can be used to build the ``HA_Chassis_Group`` register. This limitation imposed by the reverted patch doesn't work if the environment has one single gateway chassis, usually happening that in CI jobs. Change-Id: I201d7681c1a1b0c6e3f54c46ba5d4be586c709ff Signed-off-by: Rodolfo Alonso Hernandez --- neutron/common/ovn/utils.py | 9 +++++---- .../ml2/drivers/ovn/mech_driver/test_mech_driver.py | 7 ++----- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index add38184808..34d27bd263c 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -1237,19 +1237,20 @@ def sync_ha_chassis_group_network(context, nb_idl, sb_idl, port_id, 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) + ignore_chassis = set() LOG.debug('HA Chassis Group %s is based on network %s', group_name, network_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) plugin = directory.get_plugin() resource = plugin.get_network(context, network_id) az_hints = common_utils.get_az_hints(resource) 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 5a72961d9f7..22c4a9dbbe8 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 @@ -2880,9 +2880,7 @@ def test_sync_ha_chassis_group_network_as_gw(self, mock_sync_hcg): 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, ch5] - expected_ignore_chassis = [ch4.name, ch5.name] - self.sb_ovn.get_chassis_host_for_port.return_value = set( - expected_ignore_chassis) + ovn_utils.sync_ha_chassis_group_network( self.context, self.nb_ovn, self.sb_ovn, fake_port['id'], fake_net['id'], None) @@ -2896,8 +2894,7 @@ def test_sync_ha_chassis_group_network_as_gw(self, mock_sync_hcg): 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)) + self.assertEqual(set(), hcg_info.ignore_chassis) @mock.patch.object(ovn_utils, '_sync_ha_chassis_group') def test_sync_ha_chassis_group_router(self, mock_sync_hcg): From 9a686e26213de4cdefc9d39d85a110403e295bf0 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Thu, 16 Oct 2025 07:08:44 +0000 Subject: [PATCH 10/23] [OVN] Add a post fork initialization method in OVN L3 This is the base implementation of a post fork initialization, happening after the initialization of the "process" resource. This event happens after the WSGI workers are already setup. Related-Bug: #2125553 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: If708b969d86940dbec9956c58a4b22271aa9a198 --- neutron/services/ovn_l3/exceptions.py | 5 +++++ neutron/services/ovn_l3/plugin.py | 14 ++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/neutron/services/ovn_l3/exceptions.py b/neutron/services/ovn_l3/exceptions.py index 72196785fb8..24cfc2122b6 100644 --- a/neutron/services/ovn_l3/exceptions.py +++ b/neutron/services/ovn_l3/exceptions.py @@ -19,3 +19,8 @@ class MechanismDriverNotFound(n_exc.NotFound): message = _("None of the supported mechanism drivers found: " "%(mechanism_drivers)s. Check your configuration.") + + +class MechanismDriverOVNNotReady(n_exc.ServiceUnavailable): + message = _('Mechanism driver OVN connection not ready. This service ' + 'plugin must be initialized after the mechanism driver.') diff --git a/neutron/services/ovn_l3/plugin.py b/neutron/services/ovn_l3/plugin.py index c1fd1382598..56eba404707 100644 --- a/neutron/services/ovn_l3/plugin.py +++ b/neutron/services/ovn_l3/plugin.py @@ -19,6 +19,7 @@ from neutron_lib.api.definitions import qos_fip as qos_fip_apidef from neutron_lib.api.definitions import qos_gateway_ip as qos_gateway_ip_apidef from neutron_lib.callbacks import events +from neutron_lib.callbacks import priority_group from neutron_lib.callbacks import registry from neutron_lib.callbacks import resources from neutron_lib import constants as n_const @@ -94,6 +95,7 @@ def __init__(self): self.scheduler = l3_ovn_scheduler.get_scheduler() self.port_forwarding = port_forwarding.OVNPortForwarding(self) self.l3_driver_controller = driver_controller.DriverController(self) + self.subscribe() @staticmethod def _disable_qos_extensions_by_extension_drivers(aliases): @@ -161,6 +163,18 @@ def get_plugin_description(self): return ("L3 Router Service Plugin for basic L3 forwarding" " using OVN") + def subscribe(self): + # By default, the post fork initialization must be done first in the + # ML2 plugin (the lower the priority number, the sooner is attended). + registry.subscribe(self._post_fork_initialize, + resources.PROCESS, events.AFTER_INIT, + priority=priority_group.PRIORITY_DEFAULT + 1, + cancellable=True) + + def _post_fork_initialize(self, resource, event, trigger, payload=None): + if not self._nb_ovn or not self._sb_ovn: + raise ovn_l3_exc.MechanismDriverOVNNotReady() + def _add_neutron_router_interface(self, context, router_id, interface_info): try: From ee72ae7462467022d1d2503725e6ea814488e8f4 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Wed, 24 Sep 2025 07:02:01 +0000 Subject: [PATCH 11/23] [OVN] The external networks GW chassis must the same as the GW LRP The external networks use ``HA_Chassis_Group`` to bind the port to a chassis. The gateway ``Logical_Router_Port`` registers use ``Gateway_Chassis``. In order to route the external ports traffic through a router gateway port, it is needed that both the external port and the gateway port are collocated in the same chassis. This patch detects when a network (private interface in a router) is connected to a router and if this network has external ports. In this case, the Neutron API changes the external ports ``HA_Chassis_Group`` to match the set of ``Gateway_Chassis`` used in the gateway port. The mechanism used to detect any port addition or deletion to a router is the OVN event ``LogicalRouterPortEvent``, loaded by the OVN L3 plugin. NOTE: along with this patch it is needed a mechanism to permanently sync any change in the ``Logical_Router`` ``Gateway_Chassis`` set and the associated ``HA_Chassis_Group`` of the networks connected to this router. This will be done in a follow up patch in order to submit smaller and better reviewable patches. Related-Bug: #2125553 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: I125d87ffbb33541ff3a4bb2a90915d20d0a171ad --- neutron/common/ovn/utils.py | 30 ++++- .../ovn/mech_driver/ovsdb/ovn_client.py | 98 +++++++++++++++++ neutron/services/ovn_l3/ovsdb_monitor.py | 77 +++++++++++++ neutron/services/ovn_l3/plugin.py | 6 + .../ovn/mech_driver/ovsdb/test_ovn_client.py | 37 +++++++ .../services/ovn_l3/test_ovsdb_monitor.py | 103 ++++++++++++++++++ .../ovn/mech_driver/test_mech_driver.py | 2 + 7 files changed, 351 insertions(+), 2 deletions(-) create mode 100644 neutron/services/ovn_l3/ovsdb_monitor.py create mode 100644 neutron/tests/functional/services/ovn_l3/test_ovsdb_monitor.py diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index 34d27bd263c..f42619b9322 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -1093,6 +1093,18 @@ def get_subnets_address_scopes(context, subnets_by_id, fixed_ips, ml2_plugin): return address4_scope_id, address6_scope_id +def get_high_prio_chassis_in_ha_chassis_group(ha_chassis_group): + """Returns (name, priority) of the highest priority HA_Chassis""" + hc_list = [] + for ha_chassis in ha_chassis_group.ha_chassis: + hc_list.append((ha_chassis.chassis_name, ha_chassis.priority)) + hc_list = sorted(hc_list, key=lambda x: x[1], reverse=True) + try: + return hc_list[0] + except IndexError: + return None, None + + def _filter_candidates_for_ha_chassis_group(hcg_info): """Filter a list of chassis candidates for a given HA Chassis Group. @@ -1156,6 +1168,10 @@ def get_priority(ch_name): ha_ch_grp_cmd = txn.add(nb_idl.ha_chassis_group_add( hcg_info.group_name, may_exist=True, external_ids=hcg_info.external_ids)) + else: + # Update the external_ids. + txn.add(nb_idl.db_set('HA_Chassis_Group', hcg_info.group_name, + ('external_ids', hcg_info.external_ids))) max_chassis_number = constants.MAX_CHASSIS_IN_HA_GROUP priority = constants.HA_CHASSIS_GROUP_HIGHEST_PRIORITY @@ -1230,7 +1246,18 @@ def sync_ha_chassis_group_router(context, nb_idl, sb_idl, router_id, txn): @ovn_context(idl_var_name='nb_idl') def sync_ha_chassis_group_network(context, nb_idl, sb_idl, port_id, network_id, txn): - """Syncs the HA Chassis Group for a given network""" + """Syncs the HA_Chassis_Group for a given network""" + # If there is a network associated HA_Chassis_Group, the port will use it + # instead of creating a new one or updating it. + group_name = ovn_name(network_id) + hcg = nb_idl.lookup('HA_Chassis_Group', group_name, default=None) + if hcg: + router_id = hcg.external_ids.get(constants.OVN_ROUTER_ID_EXT_ID_KEY) + if router_id: + # If the HA_Chassis_Group is linked to a router, do not modify it. + ch_name, _ = get_high_prio_chassis_in_ha_chassis_group(hcg) + return hcg.uuid, ch_name + # If there are Chassis marked for hosting external ports create a HA # Chassis Group per external port, otherwise do it at the network # level @@ -1246,7 +1273,6 @@ def sync_ha_chassis_group_network(context, nb_idl, sb_idl, port_id, else: chassis_list = sb_idl.get_gateway_chassis_from_cms_options( name_only=False) - group_name = ovn_name(network_id) ignore_chassis = set() LOG.debug('HA Chassis Group %s is based on network %s', group_name, network_id) 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 9357e42029c..ec36e2c0ea2 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 @@ -1596,6 +1596,29 @@ def update_router(self, context, new_router, router_object=None): 'Error: %(error)s', {'router': router_id, 'error': e}) + def update_router_ha_chassis_group(self, context, router_id): + """If the router has GW, bind all external ports to the same GW chassis + + If a router receives or removes the gateway, this method checks all + the connected internal ports and collects its networks. Then it updates + each network, depending on the presence or not of the router gateway. + See LP#2125553. + """ + # Retrieve all internal networks (aka: ext_ids=neutron:is_ext_gw=False) + # connected to this router. + lr_name = utils.ovn_name(router_id) + lr = self._nb_idl.lr_get(lr_name).execute(check_error=True) + network_ids = set() + for lrp in lr.ports: + ext_gw = lrp.external_ids.get(ovn_const.OVN_ROUTER_IS_EXT_GW) + if not strutils.bool_from_string(ext_gw): + net_name = lrp.external_ids[ + ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY] + network_ids.add(utils.get_neutron_name(net_name)) + + for network_id in network_ids: + self.link_network_ha_chassis_group(context, network_id, router_id) + def delete_router(self, context, router_id): """Delete a logical router.""" lrouter_name = utils.ovn_name(router_id) @@ -1808,6 +1831,21 @@ def _create_lrouter_port(self, context, router, port, txn=None): lsp_address=lsp_address)) self._transaction(commands, txn=txn) + def get_router_port(self, port_id): + try: + return self._nb_idl.lrp_get( + utils.ovn_lrouter_port_name(port_id)).execute( + check_errors=True) + except idlutils.RowNotFound: + return + + def get_router_gateway_ports(self, router_id): + lrps = self._nb_idl.lrp_list(utils.ovn_name(router_id)).execute( + check_errors=True) + return [lrp for lrp in lrps if + strutils.bool_from_string( + lrp.external_ids.get(ovn_const.OVN_ROUTER_IS_EXT_GW))] + def create_router_port(self, context, router_id, router_interface): port = self._plugin.get_port(context, router_interface['port_id']) router = self._l3_plugin.get_router(context, router_id) @@ -2292,6 +2330,66 @@ def update_network(self, context, network, original_network=None): if check_rev_cmd.result == ovn_const.TXN_COMMITTED: db_rev.bump_revision(context, network, ovn_const.TYPE_NETWORKS) + def unlink_network_ha_chassis_group(self, network_id): + """Unlink the network HCG to the router + + If the network (including all subnets) has been detached from the + router, the "HA_Chassis_Group" in unlinked from the router by removing + the router_id tag from the external_ids dictionary. + """ + name = utils.ovn_name(network_id) + hcg = self._nb_idl.lookup('HA_Chassis_Group', name, default=None) + if hcg: + self._nb_idl.db_remove( + 'HA_Chassis_Group', name, 'external_ids', + ovn_const.OVN_ROUTER_ID_EXT_ID_KEY).execute( + check_error=True) + + def link_network_ha_chassis_group(self, context, network_id, router_id): + """Link a unified HCG for all network ext. ports if connected to router + + If a network is connected to a router, this method checks if the router + has a gateway port and the corresponding "Gateway_Chassis" registers. + In that case, it creates a unified "HA_Chassis_Group" for this network + and assign it to all external ports. That will collocate the external + ports in the same gateway chassis as the router gateway port, allowing + N/S communication. See LP#2125553 + """ + gw_lrps = self.get_router_gateway_ports(router_id) + if not gw_lrps: + # The router has no GW ports. Remove the "neutron:router_id" tag + # from the "HA_Chassis_Group" associated, if any. + self.unlink_network_ha_chassis_group(network_id) + return + + # Retrieve all "Gateway_Chassis" and build the "chassis_prio" + # dictionary. + chassis_prio = {} + for gc in gw_lrps[0].gateway_chassis: + chassis_prio[gc.chassis_name] = gc.priority + + with self._nb_idl.transaction(check_error=True) as txn: + # Create the "HA_Chassis_Group" associated to this network. + hcg, _ = utils.sync_ha_chassis_group_network_unified( + context, self._nb_idl, self._sb_idl, network_id, router_id, + chassis_prio, txn) + + # Retrieve all LSPs from external ports in this network. + ls = self._nb_idl.lookup('Logical_Switch', + utils.ovn_name(network_id)) + for lsp in (lsp for lsp in ls.ports if + lsp.type == ovn_const.LSP_TYPE_EXTERNAL): + # NOTE(ralonsoh): this is a protection check but all external + # ports must have "HA_Chassis_Group". If the "HA_Chassis_Group" + # register is for this port only, remove it. + group_name = utils.ovn_extport_chassis_group_name(lsp.name) + if (lsp.ha_chassis_group and + lsp.ha_chassis_group[0].name == group_name): + txn.add(self._nb_idl.ha_chassis_group_del( + lsp.ha_chassis_group[0].name, if_exists=True)) + txn.add(self._nb_idl.db_set('Logical_Switch_Port', lsp.uuid, + ('ha_chassis_group', hcg))) + def _add_subnet_dhcp_options(self, subnet, network, ovn_dhcp_options=None): if utils.is_dhcp_options_ignored(subnet): diff --git a/neutron/services/ovn_l3/ovsdb_monitor.py b/neutron/services/ovn_l3/ovsdb_monitor.py new file mode 100644 index 00000000000..69a5cfe2f7e --- /dev/null +++ b/neutron/services/ovn_l3/ovsdb_monitor.py @@ -0,0 +1,77 @@ +# Copyright 2025 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from neutron_lib import context as neutron_context +from neutron_lib.plugins import constants +from neutron_lib.plugins import directory +from oslo_utils import strutils +from ovsdbapp.backend.ovs_idl import event as row_event + +from neutron.common.ovn import constants as ovn_const +from neutron.common.ovn import utils + + +class LogicalRouterPortEvent(row_event.RowEvent): + """Logical_Router_Port create/delete event. + + If a Logical_Router_Port is deleted or added, first check if this LRP is a + gateway port or not. Then update the corresponding network (or networks) + HA_Chassis_Group, matching the Logical_Router Gateway_Chassis. + See LP#2125553. + """ + def __init__(self, driver): + self.driver = driver + self.l3_plugin = directory.get_plugin(constants.L3) + self.admin_context = neutron_context.get_admin_context() + table = 'Logical_Router_Port' + events = (self.ROW_CREATE, self.ROW_DELETE) + super().__init__(events, table, None) + + def match_fn(self, event, row, old): + if event == self.ROW_DELETE: + # Check if the LR has another port in the same network. If that is + # the case, do nothing. + ls_name = row.external_ids[ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY] + lr_name = row.external_ids[ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY] + lr = self.driver._nb_ovn.lookup('Logical_Router', lr_name) + for lrp in (lrp for lrp in lr.ports if lrp.name != row.name): + if (ls_name == lrp.external_ids[ + ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY]): + return False + return True + + # event == self.ROW_CREATE + return True + + def run(self, event, row, old=None): + ext_gw = row.external_ids.get(ovn_const.OVN_ROUTER_IS_EXT_GW) + router_id = utils.get_neutron_name( + row.external_ids[ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY]) + net_id = utils.get_neutron_name( + row.external_ids[ovn_const.OVN_NETWORK_NAME_EXT_ID_KEY]) + if event == self.ROW_DELETE: + if not strutils.bool_from_string(ext_gw): # LRP internal port. + self.l3_plugin._ovn_client.unlink_network_ha_chassis_group( + net_id) + else: # LRP gateway port. + self.l3_plugin._ovn_client.update_router_ha_chassis_group( + self.admin_context, router_id) + + else: # event == self.ROW_CREATE + if not strutils.bool_from_string(ext_gw): # LRP internal port. + self.l3_plugin._ovn_client.link_network_ha_chassis_group( + self.admin_context, net_id, router_id) + else: # LRP gateway port. + self.l3_plugin._ovn_client.update_router_ha_chassis_group( + self.admin_context, router_id) diff --git a/neutron/services/ovn_l3/plugin.py b/neutron/services/ovn_l3/plugin.py index 56eba404707..67249aaa276 100644 --- a/neutron/services/ovn_l3/plugin.py +++ b/neutron/services/ovn_l3/plugin.py @@ -51,6 +51,7 @@ from neutron.quota import resource_registry from neutron.scheduler import l3_ovn_scheduler from neutron.services.ovn_l3 import exceptions as ovn_l3_exc +from neutron.services.ovn_l3 import ovsdb_monitor from neutron.services.ovn_l3.service_providers import driver_controller from neutron.services.portforwarding.drivers.ovn import driver \ as port_forwarding @@ -175,6 +176,11 @@ def _post_fork_initialize(self, resource, event, trigger, payload=None): if not self._nb_ovn or not self._sb_ovn: raise ovn_l3_exc.MechanismDriverOVNNotReady() + # Register needed events. + self._nb_ovn.idl.notify_handler.watch_events([ + ovsdb_monitor.LogicalRouterPortEvent(self), + ]) + def _add_neutron_router_interface(self, context, router_id, interface_info): try: diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py index 5fe956f6490..a753da56f41 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_client.py @@ -12,6 +12,8 @@ # License for the specific language governing permissions and limitations # under the License. +from unittest import mock + from neutron_lib.api.definitions import external_net from neutron_lib.api.definitions import network_mtu as mtu_def from neutron_lib.api.definitions import provider_net @@ -465,3 +467,38 @@ def test_create_subnet_with_dhcp_options(self): dhcp_options.options['ntp_server']) self.assertEqual('1.2.3.6', dhcp_options.options['wpad']) + + def test_update_ha_chassis_group_linked_to_router(self): + # Create a router with multiple networks (internal, external). The + # method `link_network_ha_chassis_group` must be called for all + # internal networks. + num_private_subnets = 5 + ovn_client = self.mech_driver._ovn_client + net_arg = {provider_net.NETWORK_TYPE: 'geneve', + external_net.EXTERNAL: True} + with self.network('external', as_admin=True, + arg_list=tuple(net_arg.keys()), **net_arg) as net: + with self.subnet(net, cidr='10.100.0.0/24'): + ext_gw = {'network_id': net['network']['id']} + with self.router(external_gateway_info=ext_gw) as router: + router_id = router['router']['id'] + + net_ids = [] + for idx in range(num_private_subnets): + with self.network('internal' + str(idx)) as net: + net_ids.append(net['network']['id']) + with self.subnet(net, cidr=f'10.{idx}.0.0/24') as subnet: + subnet_id = subnet['subnet']['id'] + self._router_interface_action( + 'add', router_id, subnet_id, None) + + lr_name = ovn_utils.ovn_name(router_id) + lr = self.nb_api.lookup('Logical_Router', lr_name) + self.assertEqual(num_private_subnets + 1, len(lr.ports)) + with mock.patch.object(ovn_client, 'link_network_ha_chassis_group') as\ + mock_link: + ovn_client.update_router_ha_chassis_group(self.context, router_id) + calls = [mock.call(self.context, net_id, router_id) + for net_id in net_ids] + self.assertEqual(num_private_subnets, len(mock_link.mock_calls)) + mock_link.assert_has_calls(calls, any_order=True) diff --git a/neutron/tests/functional/services/ovn_l3/test_ovsdb_monitor.py b/neutron/tests/functional/services/ovn_l3/test_ovsdb_monitor.py new file mode 100644 index 00000000000..0a0338f642d --- /dev/null +++ b/neutron/tests/functional/services/ovn_l3/test_ovsdb_monitor.py @@ -0,0 +1,103 @@ +# Copyright 2025 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +from unittest import mock + +from neutron_lib.api.definitions import external_net +from neutron_lib.plugins import constants as plugin_constants +from neutron_lib.plugins import directory + +from neutron.common import utils as n_utils +from neutron.tests.functional import base +from neutron.tests.unit.api import test_extensions +from neutron.tests.unit.extensions import test_l3 + + +class TestLogicalRouterPortEvent( + base.TestOVNFunctionalBase, + test_l3.L3NatTestCaseMixin): + + def setUp(self, **kwargs): + super().setUp(**kwargs) + self.chassis = self.add_fake_chassis('ovs-host1') + self.l3_plugin = directory.get_plugin(plugin_constants.L3) + self.l3_plugin._post_fork_initialize(mock.ANY, mock.ANY, mock.ANY) + self.ext_api = test_extensions.setup_extensions_middleware( + test_l3.L3TestExtensionManager()) + kwargs = {'arg_list': (external_net.EXTERNAL,), + external_net.EXTERNAL: True} + self.net_ext = self._make_network( + self.fmt, 'net_ext', True, as_admin=True, **kwargs) + self.subnet = self._make_subnet(self.fmt, self.net_ext, '20.0.10.1', + '20.0.10.0/24') + self.router = self._make_router(self.fmt, self._tenant_id) + self.router_id = self.router['router']['id'] + self.net_ext_id = self.net_ext['network']['id'] + self.subnet_id = self.subnet['subnet']['id'] + + def test_add_and_delete_gw_network(self): + def is_called(): + try: + mock_update_router.assert_called_once_with( + mock.ANY, self.router_id) + return True + except AssertionError: + return False + + with mock.patch.object( + self.l3_plugin._ovn_client, + 'update_router_ha_chassis_group') as mock_update_router: + self._add_external_gateway_to_router(self.router_id, + self.net_ext_id) + n_utils.wait_until_true(is_called, timeout=10) + mock_update_router.reset_mock() + self._remove_external_gateway_from_router( + self.router_id, self.net_ext_id, external_gw_info={}) + n_utils.wait_until_true(is_called, timeout=10) + + def test_add_private_network(self): + def is_called(): + try: + mock_link.assert_called_once_with( + mock.ANY, self.net_ext_id, self.router_id) + return True + except AssertionError: + return False + + with mock.patch.object( + self.l3_plugin._ovn_client, + 'link_network_ha_chassis_group') as mock_link: + self._router_interface_action( + 'add', self.router_id, self.subnet_id, None) + n_utils.wait_until_true(is_called, timeout=10) + + def test_delete_private_network(self): + def is_called(): + try: + mock_unlink.assert_called_once_with(self.net_ext_id) + return True + except AssertionError: + return False + + with mock.patch.object( + self.l3_plugin._ovn_client, + 'link_network_ha_chassis_group'), \ + mock.patch.object( + self.l3_plugin._ovn_client, + 'unlink_network_ha_chassis_group') as mock_unlink: + self._router_interface_action( + 'add', self.router_id, self.subnet_id, None) + self._router_interface_action( + 'remove', self.router_id, self.subnet_id, None) + n_utils.wait_until_true(is_called, timeout=10) 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 22c4a9dbbe8..a6c95b383fe 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 @@ -2837,6 +2837,7 @@ def test_sync_ha_chassis_group_network_as_extport(self, mock_sync_hcg): self.sb_ovn.get_chassis_host_for_port.return_value = { ch4.name, ch5.name} + self.nb_ovn.lookup.return_value = None ovn_utils.sync_ha_chassis_group_network( self.context, self.nb_ovn, self.sb_ovn, fake_port['id'], @@ -2880,6 +2881,7 @@ def test_sync_ha_chassis_group_network_as_gw(self, mock_sync_hcg): 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, ch5] + self.nb_ovn.lookup.return_value = None ovn_utils.sync_ha_chassis_group_network( self.context, self.nb_ovn, self.sb_ovn, fake_port['id'], From ecacce7069d05db2c296f20faf376b7fd160dc06 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Fri, 17 Oct 2025 10:49:46 +0000 Subject: [PATCH 12/23] [OVN] Sync the LRP Gateway_Chassis with the network HCG If a network with external ports is connected to a router (as an internal network), Neutron will create a single ``HA_Chassis_Group`` register for this network, synced with the gateway ``Logical_Router_Port`` ``Gateway_Chassis`` registers. That will schedule the external ports in the same chassis as the gateway port. This patch attends any ``Gateway_Chassis`` change in order to sync the attached networks ``HA_Chassis_Group`` registers, mimicking what is defined the gateway port ``Gateway_Chassis`` list. NOTE: This patch closes the list of patches that implement the functionality defined in LP#2125553. Closes-Bug: #2125553 Signed-off-by: Rodolfo Alonso Hernandez Change-Id: Id75fcf72715f9b61aaa0487cdea192e8f9634fd1 --- .../ovn/mech_driver/ovsdb/ovsdb_monitor.py | 9 +++ neutron/services/ovn_l3/ovsdb_monitor.py | 29 +++++++ neutron/services/ovn_l3/plugin.py | 1 + .../services/ovn_l3/test_ovsdb_monitor.py | 78 +++++++++++++++++++ 4 files changed, 117 insertions(+) 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 2283306e6f5..039a63a5d0c 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 @@ -89,6 +89,15 @@ def _get_ha_chassis_groups_within_azs(self, az_hints): 'HA_Chassis_Group').execute(check_error=True): if not hcg.name.startswith(ovn_const.OVN_NAME_PREFIX): continue + + net_id = hcg.external_ids.get(ovn_const.OVN_NETWORK_ID_EXT_ID_KEY) + router_id = hcg.external_ids.get( + ovn_const.OVN_ROUTER_ID_EXT_ID_KEY) + if net_id and router_id: + # This HA_Chassis_Group is linked to a router, it will be + # updated matching the router Gateway_Chassis registers. + 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 diff --git a/neutron/services/ovn_l3/ovsdb_monitor.py b/neutron/services/ovn_l3/ovsdb_monitor.py index 69a5cfe2f7e..31d01c13457 100644 --- a/neutron/services/ovn_l3/ovsdb_monitor.py +++ b/neutron/services/ovn_l3/ovsdb_monitor.py @@ -75,3 +75,32 @@ def run(self, event, row, old=None): else: # LRP gateway port. self.l3_plugin._ovn_client.update_router_ha_chassis_group( self.admin_context, router_id) + + +class LogicalRouterPortGatewayChassisEvent(row_event.RowEvent): + """Logical_Router_Port Gateway_Chassis change event. + + When the Gateway_Chassis list of a Logical_Router_Port changes, it is + needed to update the linked HA_Chassis_Group registers. + """ + def __init__(self, driver): + self.driver = driver + self.l3_plugin = directory.get_plugin(constants.L3) + self.admin_context = neutron_context.get_admin_context() + table = 'Logical_Router_Port' + events = (self.ROW_UPDATE, ) + super().__init__(events, table, None) + + def match_fn(self, event, row, old): + if hasattr(old, 'gateway_chassis'): + # NOTE: when a Gateway_Chassis register is deleted, is no longer + # present in the old.gateway_chassis list. + return True + + return False + + def run(self, event, row, old=None): + lr_name = row.external_ids.get(ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY) + router_id = utils.get_neutron_name(lr_name) + self.l3_plugin._ovn_client.update_router_ha_chassis_group( + self.admin_context, router_id) diff --git a/neutron/services/ovn_l3/plugin.py b/neutron/services/ovn_l3/plugin.py index 67249aaa276..b116140a516 100644 --- a/neutron/services/ovn_l3/plugin.py +++ b/neutron/services/ovn_l3/plugin.py @@ -179,6 +179,7 @@ def _post_fork_initialize(self, resource, event, trigger, payload=None): # Register needed events. self._nb_ovn.idl.notify_handler.watch_events([ ovsdb_monitor.LogicalRouterPortEvent(self), + ovsdb_monitor.LogicalRouterPortGatewayChassisEvent(self), ]) def _add_neutron_router_interface(self, context, router_id, diff --git a/neutron/tests/functional/services/ovn_l3/test_ovsdb_monitor.py b/neutron/tests/functional/services/ovn_l3/test_ovsdb_monitor.py index 0a0338f642d..b636fe1fa81 100644 --- a/neutron/tests/functional/services/ovn_l3/test_ovsdb_monitor.py +++ b/neutron/tests/functional/services/ovn_l3/test_ovsdb_monitor.py @@ -18,6 +18,7 @@ from neutron_lib.plugins import constants as plugin_constants from neutron_lib.plugins import directory +from neutron.common.ovn import utils as ovn_utils from neutron.common import utils as n_utils from neutron.tests.functional import base from neutron.tests.unit.api import test_extensions @@ -101,3 +102,80 @@ def is_called(): self._router_interface_action( 'remove', self.router_id, self.subnet_id, None) n_utils.wait_until_true(is_called, timeout=10) + + def test_delete_router(self): + # The ``Logical_Router`` deletion triggers the + # ``LogicalRouterPortEvent`` event, but nothing is executed/called. + def is_called(): + try: + mock_update_router.assert_called_once_with( + mock.ANY, self.router_id) + return True + except AssertionError: + return False + + with mock.patch.object( + self.l3_plugin._ovn_client, + 'update_router_ha_chassis_group') as mock_update_router: + self._add_external_gateway_to_router(self.router_id, + self.net_ext_id) + n_utils.wait_until_true(is_called, timeout=10) + mock_update_router.reset_mock() + req = self.new_delete_request('routers', self.router_id) + req.get_response(self.api) + self.assertRaises(n_utils.WaitTimeout, n_utils.wait_until_true, + is_called, timeout=5) + + +class TestLogicalRouterPortGatewayChassisEvent( + base.TestOVNFunctionalBase, + test_l3.L3NatTestCaseMixin): + + def setUp(self, **kwargs): + super().setUp(**kwargs) + self.chassis = self.add_fake_chassis('ovs-host1') + self.l3_plugin = directory.get_plugin(plugin_constants.L3) + self.l3_plugin._post_fork_initialize(mock.ANY, mock.ANY, mock.ANY) + self.ext_api = test_extensions.setup_extensions_middleware( + test_l3.L3TestExtensionManager()) + kwargs = {'arg_list': (external_net.EXTERNAL,), + external_net.EXTERNAL: True} + self.net_ext = self._make_network( + self.fmt, 'net_ext', True, as_admin=True, **kwargs) + self.subnet = self._make_subnet(self.fmt, self.net_ext, '20.0.10.1', + '20.0.10.0/24') + self.router = self._make_router(self.fmt, self._tenant_id) + self.router_id = self.router['router']['id'] + self.net_ext_id = self.net_ext['network']['id'] + self.subnet_id = self.subnet['subnet']['id'] + + def test_add_and_remove_gateway_chassis(self): + def is_called(): + try: + mock_update_router.assert_called_once_with( + mock.ANY, self.router_id) + return True + except AssertionError: + return False + + ch_list = [] + for idx in range(5): + ch_list.append(self.add_fake_chassis(f'host-{idx}')) + self._add_external_gateway_to_router(self.router_id, self.net_ext_id) + lr = self.l3_plugin._nb_ovn.lookup('Logical_Router', + ovn_utils.ovn_name(self.router_id)) + lrp_gw = lr.ports[0] + with mock.patch.object( + self.l3_plugin._ovn_client, + 'update_router_ha_chassis_group') as mock_update_router: + for ch_name in ch_list: + self.l3_plugin._nb_ovn.lrp_set_gateway_chassis( + lrp_gw.uuid, ch_name).execute(check_error=True) + n_utils.wait_until_true(is_called, timeout=10) + mock_update_router.reset_mock() + + for ch_name in ch_list: + self.l3_plugin._nb_ovn.lrp_del_gateway_chassis( + lrp_gw.uuid, ch_name).execute(check_error=True) + n_utils.wait_until_true(is_called, timeout=10) + mock_update_router.reset_mock() From edba2c343bde1fc3c4947270474fdbbbb19b1eb5 Mon Sep 17 00:00:00 2001 From: Doug Szumski Date: Fri, 28 Nov 2025 14:16:17 +0000 Subject: [PATCH 13/23] Import external_net this comes from https://bugs.launchpad.net/neutron/+bug/2092271 and is referenced by the patches we are pulling in - we may need to pull in patches from that bug as well. --- neutron/tests/functional/common/ovn/test_utils.py | 1 + 1 file changed, 1 insertion(+) diff --git a/neutron/tests/functional/common/ovn/test_utils.py b/neutron/tests/functional/common/ovn/test_utils.py index 59f4cbddde4..f6fca678579 100644 --- a/neutron/tests/functional/common/ovn/test_utils.py +++ b/neutron/tests/functional/common/ovn/test_utils.py @@ -13,6 +13,7 @@ # under the License. import ddt +from neutron_lib.api.definitions import external_net from neutron_lib.api.definitions import portbindings from neutron_lib.api.definitions import provider_net from oslo_utils import uuidutils From 7872cbf402c31e7ebf4ccbccadbbff27c5d5553b Mon Sep 17 00:00:00 2001 From: Doug Szumski Date: Fri, 28 Nov 2025 14:18:46 +0000 Subject: [PATCH 14/23] Non functional change: Make pep8 happy We don't care about fixing these style nits --- .../ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py | 6 ++---- .../functional/services/ovn_l3/test_ovsdb_monitor.py | 8 ++++---- tox.ini | 4 ++-- 3 files changed, 8 insertions(+), 10 deletions(-) diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py index 45604c41f6e..12df910bbbc 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovn_db_sync.py @@ -1353,8 +1353,7 @@ def _get_ipv6_ra_configs_for_router_port(port): managed_lports = [ lport for lport in all_lports if (ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY in - lport.external_ids) - ] + lport.external_ids)] plugin_lrouter_port_ids = [lport.name.replace('lrp-', '') for lport in managed_lports] @@ -1389,8 +1388,7 @@ def _get_ipv6_ra_configs_for_router_port(port): managed_lports = [ lport for lport in all_lports if (ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY in - lport.external_ids) - ] + lport.external_ids)] monitor_lrouter_port_ids = [lport.name.replace('lrp-', '') for lport in managed_lports] monitor_lport_networks = { diff --git a/neutron/tests/functional/services/ovn_l3/test_ovsdb_monitor.py b/neutron/tests/functional/services/ovn_l3/test_ovsdb_monitor.py index b636fe1fa81..bb717f3be22 100644 --- a/neutron/tests/functional/services/ovn_l3/test_ovsdb_monitor.py +++ b/neutron/tests/functional/services/ovn_l3/test_ovsdb_monitor.py @@ -26,8 +26,8 @@ class TestLogicalRouterPortEvent( - base.TestOVNFunctionalBase, - test_l3.L3NatTestCaseMixin): + base.TestOVNFunctionalBase, + test_l3.L3NatTestCaseMixin): def setUp(self, **kwargs): super().setUp(**kwargs) @@ -128,8 +128,8 @@ def is_called(): class TestLogicalRouterPortGatewayChassisEvent( - base.TestOVNFunctionalBase, - test_l3.L3NatTestCaseMixin): + base.TestOVNFunctionalBase, + test_l3.L3NatTestCaseMixin): def setUp(self, **kwargs): super().setUp(**kwargs) diff --git a/tox.ini b/tox.ini index c2b9fcd0772..4f4c26b0c6c 100644 --- a/tox.ini +++ b/tox.ini @@ -7,7 +7,7 @@ ignore_basepython_conflict = True [testenv] description = Run unit tests. -basepython = {env:TOX_PYTHON:python3} +basepython = {env:TOX_PYTHON:python313} setenv = VIRTUAL_ENV={envdir} OS_LOG_CAPTURE={env:OS_LOG_CAPTURE:true} OS_STDOUT_CAPTURE={env:OS_STDOUT_CAPTURE:true} @@ -234,7 +234,7 @@ commands = sphinx-build -W -b linkcheck doc/source doc/build/linkcheck # TODO(amotoki) check the following new rules should be fixed or ignored # E731 do not assign a lambda expression, use a def # W504 line break after binary operator -ignore = E126,E128,E231,E275,E731,I202,H405,N530,W504 +ignore = E126,E128,E231,E275,E731,I202,H405,N530,W504,W503,E501,E123,E121 # H106: Don't put vim configuration in source files # H203: Use assertIs(Not)None to check for None # H204: Use assert(Not)Equal to check for equality From 8dba748f4058d8c5a667f7f428122aef57193281 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Tue, 25 Mar 2025 18:32:43 +0000 Subject: [PATCH 15/23] HA_Chassis_Group create command with HA_Chassis assignation This new command allows to create a "HA_Chassis_Group" register and the corresponding "HA_Chassis" child registers. If the parent "HA_Chassis_Group" register already exists, it allows to update the priority of the "HA_Chassis" registers or deleting the discarded ones. Partial-Bug: #2092271 Change-Id: I73ed014ae6a36e171c1eace8bea45010cf8415fa --- .../drivers/ovn/mech_driver/ovsdb/commands.py | 64 ++++++++++++++ .../ovn/mech_driver/ovsdb/impl_idl_ovn.py | 6 ++ .../ovn/mech_driver/ovsdb/test_impl_idl.py | 84 +++++++++++++++++++ 3 files changed, 154 insertions(+) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py index ffad2ac0f96..2c00e610487 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py @@ -13,6 +13,8 @@ # under the License. import abc +import copy +import uuid from oslo_utils import timeutils from ovsdbapp.backend.ovs_idl import command @@ -100,6 +102,49 @@ def _add_gateway_chassis(api, txn, lrp_name, val): return 'gateway_chassis', uuid_list +def _sync_ha_chassis_group(txn, nb_api, name, chassis_priority, + may_exist=False, table_name='HA_Chassis_Group', + **columns): + result = None + hcg = nb_api.lookup(table_name, name, default=None) + if hcg: + if not may_exist: + raise RuntimeError(_('HA_Chassis_Group %s exists' % name)) + else: + hcg = txn.insert(nb_api._tables[table_name]) + hcg.name = name + command.BaseCommand.set_columns(hcg, **columns) + result = hcg.uuid + + # HA_Chassis registers handling. + # Remove the non-existing chassis in ``self.chassis_priority`` + hc_to_remove = [] + for hc in getattr(hcg, 'ha_chassis', []): + if hc.chassis_name not in chassis_priority: + hc_to_remove.append(hc) + + for hc in hc_to_remove: + hcg.delvalue('ha_chassis', hc) + hc.delete() + + # Update the priority of the existing chassis. + for hc in getattr(hcg, 'ha_chassis', []): + hc_priority = chassis_priority.pop(hc.chassis_name) + hc.priority = hc_priority + + # Add the non-existing HA_Chassis registers. + for hc_name, priority in chassis_priority.items(): + hc = txn.insert(nb_api.tables['HA_Chassis']) + hc.chassis_name = hc_name + hc.priority = priority + hcg.addvalue('ha_chassis', hc) + + if not result: + result = rowview.RowView(hcg) + + return result + + class CheckLivenessCommand(command.BaseCommand): def run_idl(self, txn): # txn.pre_commit responsible for updating nb_global.nb_cfg, but @@ -1097,3 +1142,22 @@ def run_idl(self, txn): virtual_parents) setattr(lsp, 'options', options) + + +class HAChassisGroupWithHCAddCommand(command.AddCommand): + table_name = 'HA_Chassis_Group' + + def __init__(self, api, name, chassis_priority, may_exist=False, + **columns): + super().__init__(api) + self.name = name + self.chassis_priority = copy.deepcopy(chassis_priority) + self.may_exist = may_exist + self.columns = columns + + def run_idl(self, txn): + # HA_Chassis_Group register creation. + self.result = _sync_ha_chassis_group( + txn, self.api, self.name, self.chassis_priority, + may_exist=self.may_exist, table_name=self.table_name, + **self.columns) 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 7282d0fa215..2b081293568 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 @@ -912,6 +912,12 @@ def set_router_mac_age_limit(self, router=None): return cmd.SetLRouterMacAgeLimitCommand( self, router, cfg.get_ovn_mac_binding_age_threshold()) + def ha_chassis_group_with_hc_add(self, name, chassis_priority, + may_exist=False, **columns): + return cmd.HAChassisGroupWithHCAddCommand( + self, name, chassis_priority, may_exist=may_exist, + **columns) + class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend): @n_utils.classproperty diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py index f5adcbea58a..d4d2c20a74c 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py @@ -12,6 +12,7 @@ # under the License. # +from collections import abc import copy from unittest import mock import uuid @@ -695,6 +696,89 @@ def test_modify_static_route_external_ids(self): self.assertEqual(external_ids, lr.static_routes[0].external_ids) + def _cleanup_delete_hcg(self, hcg_name): + if isinstance(hcg_name, str): + self.nbapi.db_destroy('HA_Chassis_Group', hcg_name).execute( + check_error=True) + elif isinstance(hcg_name, abc.Iterable): + for _hcg_name in hcg_name: + self.nbapi.db_destroy('HA_Chassis_Group', _hcg_name).execute( + check_error=True) + + def _check_hcg(self, hcg, hcg_name, chassis_priority, + chassis_priority_deleted=None): + self.assertEqual(hcg_name, hcg.name) + self.assertEqual(len(chassis_priority), len(hcg.ha_chassis)) + for hc in hcg.ha_chassis: + self.assertEqual(chassis_priority[hc.chassis_name], hc.priority) + + if chassis_priority_deleted: + for hc_name in chassis_priority_deleted: + self.assertIsNone( + self.nbapi.lookup('HA_Chassis', hc_name, default=None)) + + def test_ha_chassis_group_with_hc_add_no_existing_hcg(self): + chassis_priority = {'ch1': 1, 'ch2': 2, 'ch3': 3, 'ch4': 4} + hcg_name = uuidutils.generate_uuid() + self.addCleanup(self._cleanup_delete_hcg, hcg_name) + hcg = self.nbapi.ha_chassis_group_with_hc_add( + hcg_name, chassis_priority).execute(check_error=True) + self._check_hcg(hcg, hcg_name, chassis_priority) + + def test_ha_chassis_group_with_hc_add_existing_hcg(self): + chassis_priority = {'ch1': 1, 'ch2': 2, 'ch3': 3, 'ch4': 4} + hcg_name = uuidutils.generate_uuid() + self.addCleanup(self._cleanup_delete_hcg, hcg_name) + self.nbapi.ha_chassis_group_with_hc_add( + hcg_name, chassis_priority).execute(check_error=True) + cmd = self.nbapi.ha_chassis_group_with_hc_add( + hcg_name, chassis_priority) + self.assertRaises(RuntimeError, cmd.execute, check_error=True) + + def test_ha_chassis_group_with_hc_add_existing_hcg_may_exist(self): + chassis_priority = {'ch1': 1, 'ch2': 2, 'ch3': 3, 'ch4': 4} + hcg_name = uuidutils.generate_uuid() + self.addCleanup(self._cleanup_delete_hcg, hcg_name) + hcg = None + for _ in range(2): + hcg = self.nbapi.ha_chassis_group_with_hc_add( + hcg_name, chassis_priority, may_exist=True).execute( + check_error=True) + self._check_hcg(hcg, hcg_name, chassis_priority) + + def test_ha_chassis_group_with_hc_add_existing_hcg_update_chassis(self): + # This test: + # - adds new chassis: ch5, ch6 + # - removes others: ch3, ch4 + # - changes the priority of the existing ones ch1, ch2 + chassis_priority = {'ch1': 1, 'ch2': 2, 'ch3': 3, 'ch4': 4} + hcg_name = uuidutils.generate_uuid() + self.addCleanup(self._cleanup_delete_hcg, hcg_name) + self.nbapi.ha_chassis_group_with_hc_add( + hcg_name, chassis_priority).execute(check_error=True) + + chassis_priority = {'ch1': 2, 'ch2': 1, 'ch5': 3, 'ch6': 4} + hcg = self.nbapi.ha_chassis_group_with_hc_add( + hcg_name, chassis_priority, may_exist=True).execute( + check_error=True) + self._check_hcg(hcg, hcg_name, chassis_priority, + chassis_priority_deleted=['ch3', 'ch4']) + + def test_ha_chassis_group_with_hc_add_two_hcg(self): + # Both HCG will have the same chassis priority (the same chassis + # names, that is something very common. + chassis_priority1 = {'ch1': 1, 'ch2': 2, 'ch3': 3, 'ch4': 4} + chassis_priority2 = {'ch1': 11, 'ch2': 12, 'ch3': 13, 'ch4': 14} + hcg_name1 = uuidutils.generate_uuid() + hcg_name2 = uuidutils.generate_uuid() + self.addCleanup(self._cleanup_delete_hcg, [hcg_name1, hcg_name2]) + hcg1 = self.nbapi.ha_chassis_group_with_hc_add( + hcg_name1, chassis_priority1).execute(check_error=True) + hcg2 = self.nbapi.ha_chassis_group_with_hc_add( + hcg_name2, chassis_priority2).execute(check_error=True) + self._check_hcg(hcg1, hcg_name1, chassis_priority1) + self._check_hcg(hcg2, hcg_name2, chassis_priority2) + class TestIgnoreConnectionTimeout(BaseOvnIdlTest): @classmethod From becc7ecbc412d4cf70e3a834808517aedb8ebe5d Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Thu, 27 Mar 2025 03:01:35 +0000 Subject: [PATCH 16/23] [OVN] Add a Logical_Router_Port HA_Chassis retrieval method This method returns the name of the chassis assigned in the "HA_Chassis_Group" associated with the "Logical_Router_Port", ordered by the priority. Partial-Bug: #2092271 Change-Id: Ifbecf0f2699297a85e67299ce6d8ec98fd41fae4 --- neutron/common/ovn/utils.py | 24 ++++++ .../tests/functional/common/ovn/test_utils.py | 73 +++++++++++++++++++ 2 files changed, 97 insertions(+) diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index f42619b9322..eb76114641d 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -1480,3 +1480,27 @@ def validate_port_forwarding_configuration(): if any(net_type in provider_network_types for net_type in cfg.CONF.ml2.tenant_network_types): raise ovn_exc.InvalidPortForwardingConfiguration() + + +def get_logical_router_port_ha_chassis(nb_idl, lrp, priorities=None): + """Get the list of chassis hosting this Logical_Router_Port. + + :param nb_idl: (``OvsdbNbOvnIdl``) OVN Northbound IDL + :param lrp: Logical_Router_Port + :param priorities: (list of int) a list of HA_Chassis chassis priorities + to search for + :return: List of tuples (chassis_name, priority) sorted by priority. If + ``priorities`` is set then only chassis matching of these + priorities are returned. + """ + chassis = [] + lrp = nb_idl.lookup('Logical_Router_Port', lrp.name, default=None) + if not lrp or not lrp.ha_chassis_group: + return chassis + + for hc in lrp.ha_chassis_group[0].ha_chassis: + if priorities and hc.priority not in priorities: + continue + chassis.append((hc.chassis_name, hc.priority)) + + return chassis diff --git a/neutron/tests/functional/common/ovn/test_utils.py b/neutron/tests/functional/common/ovn/test_utils.py index f6fca678579..0f98026f28e 100644 --- a/neutron/tests/functional/common/ovn/test_utils.py +++ b/neutron/tests/functional/common/ovn/test_utils.py @@ -375,3 +375,76 @@ def test_without_transaction(self, method, _args, _kwargs): def test_needed_parameters(self, method): self.assertRaises(RuntimeError, method, uuidutils.generate_uuid(), None, None) + + +class TestGetLogicalRouterPortHAChassis(base.TestOVNFunctionalBase): + def _create_network_and_port(self): + kwargs = {external_net.EXTERNAL: True, 'as_admin': True} + net = self._make_network(self.fmt, 'n1', True, **kwargs)['network'] + port_data = {'port': {'network_id': net['id'], + 'tenant_id': self._tenant_id,}} + port_req = self.new_create_request('ports', port_data, self.fmt) + port_res = port_req.get_response(self.api) + return self.deserialize(self.fmt, port_res)['port'] + + def _create_gw_chassis(self, num_chassis): + chassis = [] + for _ in range(num_chassis): + chassis.append(self.add_fake_chassis( + uuidutils.generate_uuid(), azs=[], + enable_chassis_as_gw=True)) + return chassis + + def _create_router(self, network_id): + gw_info = {'network_id': network_id} + router = {'router': {'name': uuidutils.generate_uuid(), + 'admin_state_up': True, + 'tenant_id': self._tenant_id, + 'external_gateway_info': gw_info}} + return self.l3_plugin.create_router(self.context, router) + + def _set_lrp_hcg(self, gw_port_id, hcg): + lrp_name = utils.ovn_lrouter_port_name(gw_port_id) + self.nb_api.db_set( + 'Logical_Router_Port', lrp_name, + ('ha_chassis_group', hcg.uuid)).execute() + return self.nb_api.lookup('Logical_Router_Port', lrp_name) + + def _get_router_hcg(self, router_id): + hcg_name = utils.ovn_name(router_id) + return self.nb_api.lookup('HA_Chassis_Group', hcg_name) + + def _check_chassis(self, ha_chassis, expected_chassis, priorities=None): + length = len(priorities) if priorities else len(expected_chassis) + self.assertEqual(length, len(ha_chassis)) + ch_priorities = set([]) + for hc in ha_chassis: + self.assertIn(hc[0], expected_chassis) + ch_priorities.add(hc[1]) + self.assertEqual(length, len(ch_priorities)) + if priorities: + for ch_priority in ch_priorities: + self.assertIn(ch_priority, priorities) + + def test_get_ha_chassis(self): + port = self._create_network_and_port() + ch_list = self._create_gw_chassis(5) + router = self._create_router(port['network_id']) + hcg = self._get_router_hcg(router['id']) + lrp = self._set_lrp_hcg(router['gw_port_id'], hcg) + + ha_chassis = utils.get_logical_router_port_ha_chassis(self.nb_api, lrp) + self._check_chassis(ha_chassis, ch_list) + + def test_get_ha_chassis_priorities(self): + port = self._create_network_and_port() + ch_list = self._create_gw_chassis(5) + router = self._create_router(port['network_id']) + hcg = self._get_router_hcg(router['id']) + lrp = self._set_lrp_hcg(router['gw_port_id'], hcg) + + prio = [ovn_const.HA_CHASSIS_GROUP_HIGHEST_PRIORITY, + ovn_const.HA_CHASSIS_GROUP_HIGHEST_PRIORITY - 1] + ha_chassis = utils.get_logical_router_port_ha_chassis( + self.nb_api, lrp, priorities=prio) + self._check_chassis(ha_chassis, ch_list, priorities=prio) From 7f17153a32b6f8e105ff0f62950f853797b93e3f Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Wed, 16 Apr 2025 07:21:37 +0000 Subject: [PATCH 17/23] Update ``filter_existing_chassis`` signature and make it static Partial-Bug: #2092271 Change-Id: Iafd1edcda72720bcd185d9d232de8e8e03a7cbff --- .../ml2/drivers/ovn/mech_driver/ovsdb/commands.py | 3 +-- neutron/scheduler/l3_ovn_scheduler.py | 4 ++-- .../tests/unit/scheduler/test_l3_ovn_scheduler.py | 13 +++++-------- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py index 2c00e610487..bc6c9aca374 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py @@ -400,8 +400,7 @@ def run_idl(self, txn): az_hints = self.api.get_gateway_chassis_az_hints(self.g_name) filtered_existing_chassis = ( self.scheduler.filter_existing_chassis( - nb_idl=self.api, gw_chassis=self.all_gw_chassis, - physnet=physnet, + gw_chassis=self.all_gw_chassis, physnet=physnet, chassis_physnets=self.chassis_with_physnets, existing_chassis=existing_chassis, az_hints=az_hints, chassis_with_azs=self.chassis_with_azs)) diff --git a/neutron/scheduler/l3_ovn_scheduler.py b/neutron/scheduler/l3_ovn_scheduler.py index f18a627980d..1c21ed9e18a 100644 --- a/neutron/scheduler/l3_ovn_scheduler.py +++ b/neutron/scheduler/l3_ovn_scheduler.py @@ -40,8 +40,8 @@ def select(self, nb_idl, sb_idl, gateway_name, candidates=None, scheduled. """ - def filter_existing_chassis(self, nb_idl, gw_chassis, - physnet, chassis_physnets, + @staticmethod + def filter_existing_chassis(gw_chassis, physnet, chassis_physnets, existing_chassis, az_hints, chassis_with_azs): chassis_list = copy.copy(existing_chassis) for chassis_name in existing_chassis or []: diff --git a/neutron/tests/unit/scheduler/test_l3_ovn_scheduler.py b/neutron/tests/unit/scheduler/test_l3_ovn_scheduler.py index a7aff4086d8..e4d64df7567 100644 --- a/neutron/tests/unit/scheduler/test_l3_ovn_scheduler.py +++ b/neutron/tests/unit/scheduler/test_l3_ovn_scheduler.py @@ -118,8 +118,7 @@ def select(self, chassis_gateway_mapping, gateway_name, def filter_existing_chassis(self, *args, **kwargs): return self.l3_scheduler.filter_existing_chassis( - nb_idl=kwargs.pop('nb_idl'), gw_chassis=kwargs.pop('gw_chassis'), - physnet=kwargs.pop('physnet'), + gw_chassis=kwargs.pop('gw_chassis'), physnet=kwargs.pop('physnet'), chassis_physnets=kwargs.pop('chassis_physnets'), existing_chassis=kwargs.pop('existing_chassis'), az_hints=kwargs.pop('az_hints', []), @@ -171,26 +170,24 @@ def test_filter_existing_chassis(self): # it from Base class didnt seem right. Also, there is no need to have # another test in LeastLoadedScheduler. chassis_physnets = {'temp': ['phys-network-0', 'phys-network-1']} - nb_idl = FakeOVNGatewaySchedulerNbOvnIdl( - self.fake_chassis_gateway_mappings['None'], 'g1') # Check if invalid chassis is removed self.assertEqual( ['temp'], self.filter_existing_chassis( - nb_idl=nb_idl, gw_chassis=["temp"], + gw_chassis=["temp"], physnet='phys-network-1', chassis_physnets=chassis_physnets, existing_chassis=['temp', None])) # Check if invalid is removed -II self.assertFalse( self.filter_existing_chassis( - nb_idl=nb_idl, gw_chassis=["temp"], + gw_chassis=["temp"], physnet='phys-network-1', chassis_physnets=chassis_physnets, existing_chassis=None)) # Check if chassis removed when physnet doesnt exist self.assertFalse( self.filter_existing_chassis( - nb_idl=nb_idl, gw_chassis=["temp"], + gw_chassis=["temp"], physnet='phys-network-2', chassis_physnets=chassis_physnets, existing_chassis=['temp'])) @@ -198,7 +195,7 @@ def test_filter_existing_chassis(self): # or in chassis_physnets self.assertFalse( self.filter_existing_chassis( - nb_idl=nb_idl, gw_chassis=["temp1"], + gw_chassis=["temp1"], physnet='phys-network-2', chassis_physnets=chassis_physnets, existing_chassis=['temp'])) From a790e151d80ab97ec1742b5907e7d102f4261335 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Mon, 21 Apr 2025 14:42:52 +0000 Subject: [PATCH 18/23] [OVN] Method to retrieve the LRP chassis list Added a new method in ``OvsdbNbOvnIdl`` to retrieve the gateway ``Logical_Router_Port`` chassis list with priorities, stored in the ``HA_Chassis_Group`` register associated to the router. Partial-Bug: #2092271 Change-Id: I6b2bc7a3e80a906f146da3645c530d175f31a8dd --- .../ovn/mech_driver/ovsdb/impl_idl_ovn.py | 24 +++++++++++ .../ovn/mech_driver/ovsdb/test_impl_idl.py | 40 +++++++++++++++++++ 2 files changed, 64 insertions(+) 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 2b081293568..35994109b0d 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 @@ -526,6 +526,30 @@ def _get_logical_router_port_gateway_chassis(self, lrp, priorities=None): # make sure that chassis are sorted by priority return sorted(chassis, reverse=True, key=lambda x: x[1]) + @staticmethod + def _get_logical_router_port_ha_chassis_group(lrp, priorities=None): + """Get the list of chassis hosting this gateway port. + + @param lrp: logical router port + @type lrp: Logical_Router_Port row + @param priorities: a list of gateway chassis priorities to search for + @type priorities: list of int + @return: List of tuples (chassis_name, priority) sorted by priority. If + ``priorities`` is set then only chassis matching of these + priorities are returned. + """ + chassis = [] + hcg = getattr(lrp, 'ha_chassis_group', None) + if not hcg: + return chassis + + for hc in hcg[0].ha_chassis: + if priorities is not None and hc.priority not in priorities: + continue + chassis.append((hc.chassis_name, hc.priority)) + # Make sure that chassis are sorted by priority (highest prio first) + return sorted(chassis, reverse=True, key=lambda x: x[1]) + def get_all_chassis_gateway_bindings(self, chassis_candidate_list=None, priorities=None): diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py index d4d2c20a74c..3b441f2b123 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py @@ -19,6 +19,7 @@ import netaddr from neutron_lib import constants +from neutron_lib.utils import net as net_utils from oslo_utils import netutils from oslo_utils import uuidutils from ovsdbapp.backend.ovs_idl import connection @@ -779,6 +780,45 @@ def test_ha_chassis_group_with_hc_add_two_hcg(self): self._check_hcg(hcg1, hcg_name1, chassis_priority1) self._check_hcg(hcg2, hcg_name2, chassis_priority2) + def _add_lrp_with_gw(self, chassis_priority=None, is_gw=True): + if is_gw: + hcg_name = uuidutils.generate_uuid() + hcg = self.nbapi.ha_chassis_group_with_hc_add( + hcg_name, chassis_priority).execute(check_error=True) + kwargs = {'ha_chassis_group': hcg.uuid} + else: + hcg = None + kwargs = {} + + mac = next(net_utils.random_mac_generator(['ca', 'fe', 'ca', 'fe'])) + networks = ['192.0.2.0/24'] + lr = self.nbapi.lr_add(uuidutils.generate_uuid()).execute( + check_error=True) + + lrp = self.nbapi.lrp_add( + lr.uuid, uuidutils.generate_uuid(), mac, networks, + **kwargs).execute(check_error=True) + return lr, lrp, hcg + + def test__get_logical_router_port_ha_chassis_group(self): + chassis_priority = {'ch1': 1, 'ch2': 2, 'ch3': 3, 'ch4': 4} + lr, lrp, hcg = self._add_lrp_with_gw(chassis_priority) + cprio_res = self.nbapi._get_logical_router_port_ha_chassis_group(lrp) + self.assertEqual([('ch4', 4), ('ch3', 3), ('ch2', 2), ('ch1', 1)], + cprio_res) + + def test__get_logical_router_port_ha_chassis_group_with_priorities(self): + chassis_priority = {'ch1': 1, 'ch2': 2, 'ch3': 3, 'ch4': 4} + lr, lrp, hcg = self._add_lrp_with_gw(chassis_priority) + cprio_res = self.nbapi._get_logical_router_port_ha_chassis_group( + lrp, priorities=(1, 3, 4)) + self.assertEqual([('ch4', 4), ('ch3', 3), ('ch1', 1)], cprio_res) + + def test__get_logical_router_port_ha_chassis_group_no_hcg(self): + lr, lrp, hcg = self._add_lrp_with_gw(is_gw=False) + cprio_res = self.nbapi._get_logical_router_port_ha_chassis_group(lrp) + self.assertEqual([], cprio_res) + class TestIgnoreConnectionTimeout(BaseOvnIdlTest): @classmethod From 6b136fc3f561404d2cb316664f74d5bc5cfb5814 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Tue, 22 Apr 2025 23:51:49 +0000 Subject: [PATCH 19/23] [OVN] Handle HA_Chassis_Group field in LRP creation and update Now is possible to set a ``HA_Chassis_Group`` in a ``Logical_Router_Port``, using the NB API methods ``add_lrouter_port`` and ``update_lrouter_port``. Both methods support creating the ``HA_Chassis_Group`` in the same transation the LRP is created or updated. Partial-Bug: #2092271 Change-Id: Iab7581d2f4eff8ab84a2a379499490353fbe1971 --- .../drivers/ovn/mech_driver/ovsdb/commands.py | 5 +- .../ovn/mech_driver/ovsdb/test_impl_idl.py | 76 +++++++++++++++++++ 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py index bc6c9aca374..e2e3d24da62 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py @@ -517,8 +517,9 @@ def run_idl(self, txn): if col == 'gateway_chassis': col, val = _add_gateway_chassis(self.api, txn, self.name, val) - setattr(lrouter_port, col, val) + self.set_column(lrouter_port, col, val) _addvalue_to_list(lrouter, 'ports', lrouter_port) + self.result = lrouter_port.uuid class UpdateLRouterPortCommand(command.BaseCommand): @@ -543,7 +544,7 @@ def run_idl(self, txn): if col == 'gateway_chassis': col, val = _add_gateway_chassis(self.api, txn, self.name, val) - setattr(lrouter_port, col, val) + self.set_column(lrouter_port, col, val) class DelLRouterPortCommand(command.BaseCommand): diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py index 3b441f2b123..40fb6049d66 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_impl_idl.py @@ -819,6 +819,82 @@ def test__get_logical_router_port_ha_chassis_group_no_hcg(self): cprio_res = self.nbapi._get_logical_router_port_ha_chassis_group(lrp) self.assertEqual([], cprio_res) + def test_create_lrp_with_ha_chassis_group_same_txn(self): + mac = next(net_utils.random_mac_generator(['ca', 'fe', 'ca', 'fe'])) + networks = ['192.0.2.0/24'] + lr_name = uuidutils.generate_uuid() + lrp_name = uuidutils.generate_uuid() + self.nbapi.lr_add(lr_name).execute(check_error=True) + + # Create the HCG and the LRP in the same transaction. + with self.nbapi.transaction(check_error=True) as txn: + hcg_cmd = txn.add(self.nbapi.ha_chassis_group_with_hc_add( + uuidutils.generate_uuid(), {'ch1': 1, 'ch2': 2})) + txn.add(self.nbapi.add_lrouter_port( + lrp_name, lr_name, mac=mac, networks=networks, + ha_chassis_group=hcg_cmd)) + + lrp = self.nbapi.lrp_get(lrp_name).execute(check_error=True) + self.assertEqual(hcg_cmd.result.uuid, lrp.ha_chassis_group[0].uuid) + + def test_create_lrp_with_ha_chassis_group_different_txn(self): + mac = next(net_utils.random_mac_generator(['ca', 'fe', 'ca', 'fe'])) + networks = ['192.0.2.0/24'] + lr_name = uuidutils.generate_uuid() + lrp_name = uuidutils.generate_uuid() + self.nbapi.lr_add(lr_name).execute(check_error=True) + + # Create the HCG and the LRP in two consecutive transactions. + hcg = self.nbapi.ha_chassis_group_with_hc_add( + uuidutils.generate_uuid(), {'ch1': 1, 'ch2': 2}).execute( + check_error=True) + self.nbapi.add_lrouter_port( + lrp_name, lr_name, mac=mac, networks=networks, + ha_chassis_group=hcg.uuid).execute(check_error=True) + + lrp = self.nbapi.lrp_get(lrp_name).execute(check_error=True) + self.assertEqual(hcg.uuid, lrp.ha_chassis_group[0].uuid) + + def test_update_lrp_with_ha_chassis_group_same_txn(self): + mac = next(net_utils.random_mac_generator(['ca', 'fe', 'ca', 'fe'])) + networks = ['192.0.2.0/24'] + lr_name = uuidutils.generate_uuid() + lrp_name = uuidutils.generate_uuid() + self.nbapi.lr_add(lr_name).execute(check_error=True) + self.nbapi.add_lrouter_port( + lrp_name, lr_name, mac=mac, + networks=networks).execute(check_error=True) + + # Create the HCG and update the LRP in the same transaction. + with self.nbapi.transaction(check_error=True) as txn: + hcg_cmd = txn.add(self.nbapi.ha_chassis_group_with_hc_add( + uuidutils.generate_uuid(), {'ch1': 1, 'ch2': 2})) + txn.add(self.nbapi.update_lrouter_port( + lrp_name, ha_chassis_group=hcg_cmd)) + + lrp = self.nbapi.lrp_get(lrp_name).execute(check_error=True) + self.assertEqual(hcg_cmd.result.uuid, lrp.ha_chassis_group[0].uuid) + + def test_update_lrp_with_ha_chassis_group_different_txn(self): + mac = next(net_utils.random_mac_generator(['ca', 'fe', 'ca', 'fe'])) + networks = ['192.0.2.0/24'] + lr_name = uuidutils.generate_uuid() + lrp_name = uuidutils.generate_uuid() + self.nbapi.lr_add(lr_name).execute(check_error=True) + self.nbapi.add_lrouter_port( + lrp_name, lr_name, mac=mac, + networks=networks).execute(check_error=True) + + # Create the HCG and update the LRP in two consecutive transactions. + hcg = self.nbapi.ha_chassis_group_with_hc_add( + uuidutils.generate_uuid(), {'ch1': 1, 'ch2': 2}).execute( + check_error=True) + self.nbapi.update_lrouter_port( + lrp_name, ha_chassis_group=hcg.uuid).execute(check_error=True) + + lrp = self.nbapi.lrp_get(lrp_name).execute(check_error=True) + self.assertEqual(hcg.uuid, lrp.ha_chassis_group[0].uuid) + class TestIgnoreConnectionTimeout(BaseOvnIdlTest): @classmethod From d98c9249f1e5de4ef74a5a87bf61bc61a216c58c Mon Sep 17 00:00:00 2001 From: Doug Szumski Date: Thu, 11 Dec 2025 14:10:44 +0000 Subject: [PATCH 20/23] rebase with: HA_Chassis_Group create command with HA_Chassis assignation --- neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py | 1 - 1 file changed, 1 deletion(-) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py index e2e3d24da62..4556d233342 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py @@ -14,7 +14,6 @@ import abc import copy -import uuid from oslo_utils import timeutils from ovsdbapp.backend.ovs_idl import command From 8d20fe6211a2cf5507ba97e096f498b7c79d0628 Mon Sep 17 00:00:00 2001 From: Doug Szumski Date: Thu, 11 Dec 2025 14:13:07 +0000 Subject: [PATCH 21/23] rebase with HA CG create command --- neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py | 1 + 1 file changed, 1 insertion(+) diff --git a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py index 4556d233342..1b96964c011 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py @@ -18,6 +18,7 @@ from oslo_utils import timeutils from ovsdbapp.backend.ovs_idl import command from ovsdbapp.backend.ovs_idl import idlutils +from ovsdbapp.backend.ovs_idl import rowview from ovsdbapp.schema.ovn_northbound import commands as ovn_nb_commands from ovsdbapp import utils as ovsdbapp_utils From ed073a8b0ec65d2478797300c0bbc5aebe0b02d5 Mon Sep 17 00:00:00 2001 From: Doug Szumski Date: Thu, 11 Dec 2025 15:45:44 +0000 Subject: [PATCH 22/23] Rebase with whatever removed it --- .../plugins/ml2/drivers/ovn/mech_driver/test_mech_driver.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 a6c95b383fe..9f16a7559bd 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 @@ -1150,10 +1150,10 @@ def _test_set_port_status_up(self, mock_is_ext, mock_sync, is_extport_present=False): port_device_owner = 'compute:nova' if is_compute_port else '' self.mech_driver._plugin.nova_notifier = mock.Mock() - mock_sync.return_value = mock.Mock(), mock.Mock() mock_is_ext.return_value = is_extport_present self.sb_ovn.get_extport_chassis_from_cms_options.return_value = [ mock.Mock()] + mock_sync.return_value = (mock.ANY, mock.ANY) with self.network() as net1, \ self.subnet(network=net1) as subnet1, \ self.port(subnet=subnet1, is_admin=True, From b1f1dd3f8486f4b9f20f994c27958b70604234a6 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Fri, 19 Sep 2025 14:48:29 +0000 Subject: [PATCH 23/23] [OVN] Remove the check for external ports support The support for ``HA_Chassis_Group`` in the ``Logical_Switch_Ports`` was provided in OVN v20.03.0 [1] and thus the support for external ports (baremetal nodes, SR-IOV). [1]https://github.com/ovn-org/ovn/commit/b31c76000bef314b68e776d318d1ce4cf152450b Signed-off-by: Rodolfo Alonso Hernandez Change-Id: I926db2b9e53a7b127d85cf8b6337e1632aebaf57 --- .../ovn/mech_driver/ovsdb/maintenance.py | 5 --- .../ovn/mech_driver/ovsdb/ovn_client.py | 37 ++++++------------- .../ovn/mech_driver/ovsdb/ovsdb_monitor.py | 3 -- .../ovn/mech_driver/ovsdb/test_maintenance.py | 1 - .../mech_driver/ovsdb/test_ovsdb_monitor.py | 1 - .../ovn/mech_driver/test_mech_driver.py | 3 -- ...hassis_group-support-40fcd1e4cb7f0e73.yaml | 7 ++++ 7 files changed, 18 insertions(+), 39 deletions(-) create mode 100644 releasenotes/notes/ovn-ha_chassis_group-support-40fcd1e4cb7f0e73.yaml 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 1f3bffcb4c8..59cbe124165 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -753,11 +753,6 @@ def check_baremetal_ports_dhcp_options(self): Update baremetal ports DHCP options based on the "disable_ovn_dhcp_for_baremetal_ports" configuration option. """ - # If external ports is not supported stop running - # this periodic task - if not self._ovn_client.is_external_ports_supported(): - raise periodics.NeverAgain() - context = n_context.get_admin_context() ports = ports_obj.Port.get_ports_by_vnic_type_and_host( context, portbindings.VNIC_BAREMETAL) 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 ec36e2c0ea2..c1d82a5f11b 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 @@ -142,10 +142,6 @@ def _transaction(self, commands, txn=None): for cmd in commands: txn.add(cmd) - def is_external_ports_supported(self): - return self._nb_idl.is_col_present( - 'Logical_Switch_Port', 'ha_chassis_group') - def _get_allowed_addresses_from_port(self, port): if not port.get(psec.PORTSECURITY): return [], [] @@ -440,12 +436,8 @@ def _get_port_options(self, port): port_type = ovn_const.LSP_TYPE_LOCALPORT if utils.is_port_external(port): - if self.is_external_ports_supported(): - port_type = ovn_const.LSP_TYPE_EXTERNAL - else: - LOG.warning('The version of OVN used does not support ' - 'the "external ports" feature used for ' - 'SR-IOV ports with OVN native DHCP') + port_type = ovn_const.LSP_TYPE_EXTERNAL + addresses = [] port_security, new_macs = ( self._get_allowed_addresses_from_port(port)) @@ -616,8 +608,7 @@ def create_port(self, context, port): 'dhcpv6_options': dhcpv6_options } - if (self.is_external_ports_supported() and - port_info.type == ovn_const.LSP_TYPE_EXTERNAL): + if port_info.type == ovn_const.LSP_TYPE_EXTERNAL: kwargs['ha_chassis_group'], _ = ( utils.sync_ha_chassis_group_network( context, self._nb_idl, self._sb_idl, port['id'], @@ -749,15 +740,14 @@ def update_port(self, context, port, port_object=None): portbindings.VIF_TYPE_UNBOUND): columns_dict['addresses'] = [] - if self.is_external_ports_supported(): - if port_info.type == ovn_const.LSP_TYPE_EXTERNAL: - columns_dict['ha_chassis_group'], _ = ( - utils.sync_ha_chassis_group_network( - context, self._nb_idl, self._sb_idl, port['id'], - port['network_id'], txn)) - else: - # Clear the ha_chassis_group field - columns_dict['ha_chassis_group'] = [] + if port_info.type == ovn_const.LSP_TYPE_EXTERNAL: + columns_dict['ha_chassis_group'], _ = ( + utils.sync_ha_chassis_group_network( + context, self._nb_idl, self._sb_idl, port['id'], + port['network_id'], txn)) + else: + # Clear the ha_chassis_group field + columns_dict['ha_chassis_group'] = [] addr_pairs_diff = utils.compute_address_pairs_diff(ovn_port, port) @@ -2218,11 +2208,6 @@ def _check_network_changes_in_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, '') 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 039a63a5d0c..d2e3ddfbf8c 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 @@ -126,9 +126,6 @@ def handle_ha_chassis_group_changes(self, event, row, old): This method handles the inclusion and removal of Chassis to/from the default HA Chassis Group. """ - if not self.driver._ovn_client.is_external_ports_supported(): - return - is_gw_chassis = utils.is_gateway_chassis(row) # If the Chassis being created is not a gateway, ignore it if not is_gw_chassis and event == self.ROW_CREATE: 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 11adbfc53e0..afa14cffda3 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 @@ -682,7 +682,6 @@ def test_check_provider_distributed_ports_flavor_router(self): def _test_check_baremetal_ports_dhcp_options(self, dhcp_disabled=False): cfg.CONF.set_override('disable_ovn_dhcp_for_baremetal_ports', dhcp_disabled, group='ovn') - self.fake_ovn_client.is_external_ports_supported.return_value = True nb_idl = self.fake_ovn_client._nb_idl self.fake_ovn_client._get_port_options.return_value = 'fake-port-opts' 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 818a4a11fca..40ae9bc2385 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 @@ -660,7 +660,6 @@ def setUp(self): super().setUp() 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) self.is_gw_ch_mock = mock.patch.object( utils, 'is_gateway_chassis').start() 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 9f16a7559bd..4ca1f39432b 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 @@ -4577,9 +4577,6 @@ def test_create_port_with_multi_sgs_duplicate_rules(self): self.assertEqual( 3, self.mech_driver.nb_ovn.pg_add_ports.call_count) - @mock.patch('neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.' - 'ovn_client.OVNClient.is_external_ports_supported', - lambda *_: True) @mock.patch.object(ovn_utils, 'sync_ha_chassis_group_network') def _test_create_port_with_vnic_type(self, vnic_type, sync_mock): fake_grp = 'fake-default-ha-group-uuid' diff --git a/releasenotes/notes/ovn-ha_chassis_group-support-40fcd1e4cb7f0e73.yaml b/releasenotes/notes/ovn-ha_chassis_group-support-40fcd1e4cb7f0e73.yaml new file mode 100644 index 00000000000..9ffbf18a9db --- /dev/null +++ b/releasenotes/notes/ovn-ha_chassis_group-support-40fcd1e4cb7f0e73.yaml @@ -0,0 +1,7 @@ +--- +features: + - | + Removed support for OVN versions under v20.03.0. The "ha_chassis_group" + field is expected in the "Logical_Router_Port" Northbound table. For more + information, see commit `ovn: Support a new Logical_Switch_Port.type - + 'external' `_.