diff --git a/doc/source/admin/ovn/l3_scheduler.rst b/doc/source/admin/ovn/l3_scheduler.rst index b8b57edfb26..31fd69dff99 100644 --- a/doc/source/admin/ovn/l3_scheduler.rst +++ b/doc/source/admin/ovn/l3_scheduler.rst @@ -26,10 +26,9 @@ The maximum number of ``Gateway_Chassis`` that can be assigned to a the highest priority a ``Gateway_Chassis`` will have is 5. If no gateway chassis are available during the ``Logical_Router_Port`` -scheduling, no ``Gateway_Chassis`` will be assigned and the value -"neutron-ovn-invalid-chassis" will be set in the "options" column of the -``Logical_Router`` register; that value will be used to detect an unhosted -router gateway port. +scheduling, no ``Gateway_Chassis`` will be assigned and no value will be set +in the "options" column of the ``Logical_Router`` register; that will be used +to detect an unhosted router gateway port. Types of schedulers @@ -88,7 +87,7 @@ Both the ``OVNGatewayChanceScheduler`` and the ``OVNGatewayLeastLoadedScheduler`` schedulers have the Availability Zones (AZ) in consideration. If a router has any AZ defined, the schedulers will select only those chassis located in the AZs. If no chassis meets this condition, the -``Logical_Router_Port`` will be assigned to the "neutron-ovn-invalid-chassis". +``Logical_Router_Port`` won't be assigned to any chassis and won't be bound. Once the list of candidate ``Chassis`` (depending on the scheduler selected) is created, this list is reordered to prioritize these ``Chassis`` from diff --git a/neutron/common/ovn/constants.py b/neutron/common/ovn/constants.py index 7c34d8d212f..82f0bef7438 100644 --- a/neutron/common/ovn/constants.py +++ b/neutron/common/ovn/constants.py @@ -109,12 +109,6 @@ ACL_ACTION_ALLOW_RELATED = 'allow-related' ACL_ACTION_ALLOW_STATELESS = 'allow-stateless' ACL_ACTION_ALLOW = 'allow' -# When a OVN L3 gateway is created, it needs to be bound to a chassis. In -# case a chassis is not found OVN_GATEWAY_INVALID_CHASSIS will be set in -# the options column of the Logical Router. This value is used to detect -# unhosted router gateways to schedule. -OVN_GATEWAY_INVALID_CHASSIS = 'neutron-ovn-invalid-chassis' - # NOTE(lucasagomes): These options were last synced from # https://github.com/ovn-org/ovn/blob/feb5d6e81d5a0290aa3618a229c860d01200422e/lib/ovn-l7.h # diff --git a/neutron/common/ovn/utils.py b/neutron/common/ovn/utils.py index 8cdd37a0a2d..f4f1afdda65 100644 --- a/neutron/common/ovn/utils.py +++ b/neutron/common/ovn/utils.py @@ -669,10 +669,7 @@ def is_gateway_chassis_invalid(chassis_name, gw_chassis, @type chassis_with_azs: {} @return Boolean """ - - if chassis_name == constants.OVN_GATEWAY_INVALID_CHASSIS: - return True - elif chassis_name not in chassis_physnets: + if chassis_name not in chassis_physnets: return True elif physnet and physnet not in chassis_physnets.get(chassis_name): return True 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 e48a4f138c1..946bc81cf3f 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/commands.py @@ -412,10 +412,9 @@ class ScheduleNewGatewayCommand(command.BaseCommand): chassis = self.scheduler.select( self.api, self.sb_api, self.g_name, candidates=candidates, target_lrouter=lrouter) - - setattr( - lrouter_port, - *_add_gateway_chassis(self.api, txn, self.g_name, chassis)) + if chassis: + setattr(lrouter_port, + *_add_gateway_chassis(self.api, txn, self.g_name, chassis)) class AddLRouterPortCommand(command.BaseCommand): 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 45082113410..f8f0d5079c6 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 @@ -551,8 +551,16 @@ class OvsdbNbOvnIdl(nb_impl_idl.OvnNbApiIdlImpl, Backend): def get_unhosted_gateways(self, port_physnet_dict, chassis_with_physnets, all_gw_chassis, chassis_with_azs): + """Return the GW LRPs with no chassis assigned + + If the LRP belongs to a tunnelled network (physnet=None), it won't be + hosted to any chassis. + """ unhosted_gateways = set() for port, physnet in port_physnet_dict.items(): + if not physnet: + continue + lrp_name = '%s%s' % (ovn_const.LRP_PREFIX, port) original_state = self.get_gateway_chassis_binding(lrp_name) az_hints = self.get_gateway_chassis_az_hints(lrp_name) 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 632792f69eb..fbaf91064ec 100644 --- a/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py +++ b/neutron/plugins/ml2/drivers/ovn/mech_driver/ovsdb/maintenance.py @@ -29,6 +29,7 @@ from neutron_lib.exceptions import l3 as l3_exc from oslo_config import cfg from oslo_log import log from oslo_serialization import jsonutils +from oslo_utils import strutils from oslo_utils import timeutils from ovsdbapp.backend.ovs_idl import event as row_event @@ -1204,6 +1205,29 @@ class DBInconsistenciesPeriodics(SchemaAwarePeriodicsBase): raise periodics.NeverAgain() + # TODO(ralonsoh): Remove this method in the C+2 cycle (next SLURP release) + @has_lock_periodic(spacing=600, run_immediately=True) + def remove_invalid_gateway_chassis_from_unbound_lrp(self): + """Removes all invalid 'Gateway_Chassis' from unbound LRPs""" + is_gw = ovn_const.OVN_ROUTER_IS_EXT_GW + lrp_list = [] + for lr in self._nb_idl.lr_list().execute(check_error=True): + for lrp in self._nb_idl.lrp_list(lr.uuid).execute( + check_error=True): + if (is_gw in lrp.external_ids and + strutils.bool_from_string(lrp.external_ids[is_gw]) and + lrp.gateway_chassis and + lrp.gateway_chassis[0].chassis_name == + 'neutron-ovn-invalid-chassis'): + lrp_list.append(lrp) + + with self._nb_idl.transaction(check_error=True) as txn: + for lrp in lrp_list: + txn.add(self._nb_idl.lrp_del_gateway_chassis( + lrp.uuid, 'neutron-ovn-invalid-chassis')) + + raise periodics.NeverAgain() + class HashRingHealthCheckPeriodics(object): diff --git a/neutron/scheduler/l3_ovn_scheduler.py b/neutron/scheduler/l3_ovn_scheduler.py index deeb9d492fb..e2ded1059a4 100644 --- a/neutron/scheduler/l3_ovn_scheduler.py +++ b/neutron/scheduler/l3_ovn_scheduler.py @@ -47,7 +47,7 @@ class OVNGatewayScheduler(object, metaclass=abc.ABCMeta): physnet, chassis_physnets, existing_chassis, az_hints, chassis_with_azs): chassis_list = copy.copy(existing_chassis) - for chassis_name in existing_chassis: + for chassis_name in existing_chassis or []: if utils.is_gateway_chassis_invalid(chassis_name, gw_chassis, physnet, chassis_physnets, az_hints, chassis_with_azs): @@ -73,7 +73,7 @@ class OVNGatewayScheduler(object, metaclass=abc.ABCMeta): if not candidates: LOG.warning('Gateway %s was not scheduled on any chassis, no ' 'candidates are available', gateway_name) - return [ovn_const.OVN_GATEWAY_INVALID_CHASSIS] + return chassis_count = min( ovn_const.MAX_GW_CHASSIS - len(existing_chassis), len(candidates) @@ -97,8 +97,7 @@ class OVNGatewayScheduler(object, metaclass=abc.ABCMeta): azs = set() # Check if candidates list valid - if not candidates or ( - candidates == [ovn_const.OVN_GATEWAY_INVALID_CHASSIS]): + if not candidates: return candidates chassis_with_azs = sb_idl.get_chassis_and_azs() 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 3c724c25aef..0272722864e 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 @@ -175,9 +175,9 @@ class _TestMaintenanceHelper(base.TestOVNFunctionalBase): res = req.get_response(self.api) return self.deserialize(self.fmt, res)['router'] - def _update_router_name(self, net_id, new_name): - data = {'router': {'name': new_name}} - req = self.new_update_request('routers', data, net_id, self.fmt) + def _update_router(self, router_id, router_dict): + data = {'router': router_dict} + req = self.new_update_request('routers', data, router_id, self.fmt) res = req.get_response(self.api) return self.deserialize(self.fmt, res)['router'] @@ -591,8 +591,8 @@ class TestMaintenance(_TestMaintenanceHelper): new_obj_name = 'routertest_updated' with mock.patch.object(self._l3_ovn_client, 'update_router'): - new_neutron_obj = self._update_router_name(neutron_obj['id'], - new_obj_name) + new_neutron_obj = self._update_router(neutron_obj['id'], + {'name': new_obj_name}) # Assert the revision numbers are out-of-sync ovn_obj = self._find_router_row_by_name(obj_name) @@ -1190,6 +1190,36 @@ class TestMaintenance(_TestMaintenanceHelper): self.assertIn(router1['id'], pra_res_ids) self.assertIn(router2['id'], pra_res_ids) + def test_remove_invalid_gateway_chassis_from_unbound_lrp(self): + net1 = self._create_network(uuidutils.generate_uuid(), external=True) + subnet1 = self._create_subnet(uuidutils.generate_uuid(), net1['id']) + external_gateway_info = { + 'enable_snat': True, 'network_id': net1['id'], + 'external_fixed_ips': [{'ip_address': '10.0.0.2', + 'subnet_id': subnet1['id']}]} + router = self._create_router( + uuidutils.generate_uuid(), + external_gateway_info=external_gateway_info) + + # Manually add the LRP.gateway_chassis with name + # 'neutron-ovn-invalid-chassis' + lr = self.nb_api.lookup('Logical_Router', utils.ovn_name(router['id'])) + lrp = lr.ports[0] + self.nb_api.lrp_set_gateway_chassis( + lrp.uuid, 'neutron-ovn-invalid-chassis').execute(check_error=True) + gc = self.nb_api.db_find_rows( + 'Gateway_Chassis', + ('chassis_name', '=', 'neutron-ovn-invalid-chassis')).execute( + check_error=True)[0] + + self.assertRaises( + periodics.NeverAgain, + self.maint.remove_invalid_gateway_chassis_from_unbound_lrp) + self.assertIsNone(self.nb_api.lookup('Gateway_Chassis', gc.uuid, + default=None)) + lr = self.nb_api.lookup('Logical_Router', utils.ovn_name(router['id'])) + self.assertEqual([], lr.ports[0].gateway_chassis) + class TestLogMaintenance(_TestMaintenanceHelper, test_log_driver.LogApiTestCaseBase): diff --git a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py index 7cc1db9ad5f..d58de77da28 100644 --- a/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py +++ b/neutron/tests/functional/plugins/ml2/drivers/ovn/mech_driver/ovsdb/test_ovsdb_monitor.py @@ -20,6 +20,7 @@ import fixtures as og_fixtures from neutron_lib.api.definitions import allowedaddresspairs from neutron_lib.api.definitions import external_net from neutron_lib.api.definitions import portbindings +from neutron_lib.api.definitions import provider_net from neutron_lib.plugins import constants as plugin_constants from neutron_lib.plugins import directory from oslo_concurrency import processutils @@ -517,8 +518,10 @@ class TestSBDbMonitor(base.TestOVNFunctionalBase, test_l3.L3NatTestCaseMixin): def setUp(self, **kwargs): super().setUp(**kwargs) - self.chassis = self.add_fake_chassis('ovs-host1', - enable_chassis_as_gw=True) + self.physnet = 'public' + self.chassis = self.add_fake_chassis( + 'ovs-host1', physical_nets=[self.physnet], + enable_chassis_as_gw=True) self.l3_plugin = directory.get_plugin(plugin_constants.L3) ext_mgr = test_l3.L3TestExtensionManager() self.ext_api = test_extensions.setup_extensions_middleware(ext_mgr) @@ -545,10 +548,12 @@ class TestSBDbMonitor(base.TestOVNFunctionalBase, test_l3.L3NatTestCaseMixin): return True return lrp.external_ids == pb.external_ids - kwargs = {'arg_list': (external_net.EXTERNAL,), - external_net.EXTERNAL: True} + arg_dict = {external_net.EXTERNAL: True, + provider_net.NETWORK_TYPE: 'flat', + provider_net.PHYSICAL_NETWORK: self.physnet} ext_net = self._make_network(self.fmt, 'ext_net', True, as_admin=True, - **kwargs) + arg_list=tuple(arg_dict.keys()), + **arg_dict) self._make_subnet(self.fmt, ext_net, '10.251.0.1', '10.251.0.0/24', enable_dhcp=True) router = self._make_router(self.fmt, self._tenant_id) diff --git a/neutron/tests/functional/services/ovn_l3/test_plugin.py b/neutron/tests/functional/services/ovn_l3/test_plugin.py index 84b85b0eb3e..18021a011a6 100644 --- a/neutron/tests/functional/services/ovn_l3/test_plugin.py +++ b/neutron/tests/functional/services/ovn_l3/test_plugin.py @@ -41,6 +41,7 @@ class TestRouter(base.TestOVNFunctionalBase): self.chassis2 = self.add_fake_chassis( 'ovs-host2', physical_nets=['physnet2', 'physnet3'], enable_chassis_as_gw=True, azs=[]) + self.physnet_used = ['physnet1', 'physnet2', 'physnet3'] self.cr_lrp_pb_event = events.WaitForCrLrpPortBindingEvent() self.sb_api.idl.notify_handler.watch_event(self.cr_lrp_pb_event) @@ -86,13 +87,11 @@ class TestRouter(base.TestOVNFunctionalBase): ip_version=n_consts.IP_VERSION_4) return network - def _set_redirect_chassis_to_invalid_chassis(self, ovn_client): + def _unset_lrp_gw_chassis(self, ovn_client): with ovn_client._nb_idl.transaction(check_error=True) as txn: - for lrp in self.nb_api.tables[ - 'Logical_Router_Port'].rows.values(): + for lrp in self.nb_api.tables['Logical_Router_Port'].rows.values(): txn.add(ovn_client._nb_idl.update_lrouter_port( - lrp.name, - gateway_chassis=[ovn_const.OVN_GATEWAY_INVALID_CHASSIS])) + lrp.name, gateway_chassis=[])) def _get_gwc_dict(self): sched_info = {} @@ -117,11 +116,11 @@ class TestRouter(base.TestOVNFunctionalBase): # exception occasionally. continue - gw_port_id = router.get('gw_port_id') - logical_port = 'cr-lrp-%s' % gw_port_id - self.assertTrue(self.cr_lrp_pb_event.wait(logical_port), - msg='lrp %s failed to bind' % logical_port) if bind_chassis: + gw_port_id = router.get('gw_port_id') + logical_port = 'cr-lrp-%s' % gw_port_id + self.assertTrue(self.cr_lrp_pb_event.wait(logical_port), + msg='lrp %s failed to bind' % logical_port) self.sb_api.lsp_bind(logical_port, bind_chassis, may_exist=True).execute(check_error=True) return routers @@ -165,22 +164,24 @@ class TestRouter(base.TestOVNFunctionalBase): def fake_select(*args, **kwargs): self.assertCountEqual(candidates, kwargs['candidates']) # We are not interested in further processing, let us return - # INVALID_CHASSIS to avoid errors - return [ovn_const.OVN_GATEWAY_INVALID_CHASSIS] + # a random chassis name to avoid errors. If there are no + # candidates, this method returns None. + return ['a-random-chassis'] if candidates else None with mock.patch.object(self.l3_plugin.scheduler, 'select', - side_effect=fake_select) as plugin_select: + side_effect=fake_select) as plugin_select: gw_info = {'network_id': ext1['network']['id']} self._create_router('router1', gw_info=gw_info, az_hints=router_az_hints) self.assertTrue(plugin_select.called) plugin_select.reset_mock() - # set redirect-chassis to neutron-ovn-invalid-chassis, so - # that schedule_unhosted_gateways will try to schedule it - self._set_redirect_chassis_to_invalid_chassis(ovn_client) + # Unset the redirect-chassis so that schedule_unhosted_gateways + # will try to schedule it. + self._unset_lrp_gw_chassis(ovn_client) self.l3_plugin.schedule_unhosted_gateways() - self.assertTrue(plugin_select.called) + check = self.assertTrue if candidates else self.assertFalse + check(plugin_select.called) def test_gateway_chassis_with_cms_and_bridge_mappings(self): # Both chassis1 and chassis3 are having proper bridge mappings, @@ -201,7 +202,7 @@ class TestRouter(base.TestOVNFunctionalBase): 'ext1', 'vlan', 'physnet1', 1, "10.0.0.1", "10.0.0.0/24") # As we have 'gateways' in the system, but without required # chassis we should not schedule gw in that case at all. - self._set_redirect_chassis_to_invalid_chassis(ovn_client) + self._unset_lrp_gw_chassis(ovn_client) with mock.patch.object(self.l3_plugin.scheduler, 'select', side_effect=[self.chassis1]): gw_info = {'network_id': ext1['network']['id']} @@ -242,7 +243,7 @@ class TestRouter(base.TestOVNFunctionalBase): 'ext1', 'vlan', 'physnet1', 1, "10.0.0.1", "10.0.0.0/24") # As we have 'gateways' in the system, but without required # chassis we should not schedule gw in that case at all. - self._set_redirect_chassis_to_invalid_chassis(ovn_client) + self._unset_lrp_gw_chassis(ovn_client) with mock.patch.object(self.l3_plugin.scheduler, 'select', side_effect=[self.chassis1]): gw_info = {'network_id': ext1['network']['id']} @@ -264,7 +265,7 @@ class TestRouter(base.TestOVNFunctionalBase): def test_gateway_chassis_no_physnet_tunnelled_network(self): # The GW network is tunnelled, no physnet defined --> no possible # candidates. - self._check_gateway_chassis_candidates([], physnet=None) + self._check_gateway_chassis_candidates(None, physnet=None) def test_gateway_chassis_least_loaded_scheduler(self): # This test will create 4 routers each with its own gateway. @@ -372,11 +373,8 @@ class TestRouter(base.TestOVNFunctionalBase): Test cases when subnets are added to an external network after router has been configured to use that network via "set --external-gateway" """ - with mock.patch.object(self.l3_plugin.scheduler, 'select', - return_value=[ - ovn_const.OVN_GATEWAY_INVALID_CHASSIS - ]) as plugin_select: + return_value=self.chassis1) as plugin_select: router1 = self._create_router('router1', gw_info=None) router_id = router1['id'] self.assertIsNone(self._get_gw_port(router_id), @@ -490,8 +488,8 @@ class TestRouter(base.TestOVNFunctionalBase): def fake_select(*args, **kwargs): self.assertCountEqual(self.candidates, kwargs['candidates']) # We are not interested in further processing, let us return - # INVALID_CHASSIS to avoid errors - return [ovn_const.OVN_GATEWAY_INVALID_CHASSIS] + # a random chassis name to avoid errors. + return ['a-random-chassis'] with mock.patch.object(self.l3_plugin.scheduler, 'select', side_effect=fake_select) as plugin_select: @@ -499,9 +497,9 @@ class TestRouter(base.TestOVNFunctionalBase): gw_info = {'network_id': ext1['network']['id']} router1 = self._create_router('router1', gw_info=gw_info) - # set redirect-chassis to neutron-ovn-invalid-chassis, so - # that schedule_unhosted_gateways will try to schedule it - self._set_redirect_chassis_to_invalid_chassis(ovn_client) + # Unset the redirect-chassis so that schedule_unhosted_gateways + # will try to schedule it. + self._unset_lrp_gw_chassis(ovn_client) self.l3_plugin.schedule_unhosted_gateways() self.candidates = [self.chassis1, self.chassis2] @@ -509,7 +507,7 @@ class TestRouter(base.TestOVNFunctionalBase): self.l3_plugin.update_router( self.context, router1['id'], {'router': {l3_apidef.EXTERNAL_GW_INFO: gw_info}}) - self._set_redirect_chassis_to_invalid_chassis(ovn_client) + self._unset_lrp_gw_chassis(ovn_client) self.l3_plugin.schedule_unhosted_gateways() self.candidates = [] @@ -517,17 +515,17 @@ class TestRouter(base.TestOVNFunctionalBase): self.l3_plugin.update_router( self.context, router1['id'], {'router': {l3_apidef.EXTERNAL_GW_INFO: gw_info}}) - self._set_redirect_chassis_to_invalid_chassis(ovn_client) + self._unset_lrp_gw_chassis(ovn_client) self.l3_plugin.schedule_unhosted_gateways() # We can't test call_count for these mocks, as we have disabled # maintenance_worker which will trigger chassis events # and eventually calling schedule_unhosted_gateways. - # However, we know for sure that these mocks must have been - # called at least 3 times because that is the number of times - # this test invokes them: 1x create_router + 2x update_router; - # and 3x schedule_unhosted_gateways for plugin_select mock. - self.assertGreaterEqual(plugin_select.call_count, 3) + # The router is created with a gateway port and updated twice. + # However, the "plugin_select" is called only twice because the + # third gateway network used is type "geneve" and the LRP are not + # hosted in any chassis. + self.assertGreaterEqual(plugin_select.call_count, 2) def test_router_gateway_port_binding_host_id(self): # Test setting chassis on chassisredirect port in Port_Binding table, @@ -613,7 +611,8 @@ class TestRouter(base.TestOVNFunctionalBase): def test_create_delete_router_multiple_gw_ports(self): ext4 = self._create_ext_network( - 'ext4', 'flat', 'physnet4', None, "40.0.0.1", "40.0.0.0/24") + 'ext4', 'flat', self.physnet_used[0], None, '40.0.0.1', + '40.0.0.0/24') router = self._create_router('router4') gws = self._add_external_gateways( router['id'], @@ -638,7 +637,8 @@ class TestRouter(base.TestOVNFunctionalBase): def test_create_router_multiple_gw_ports_ecmp(self): ext5 = self._create_ext_network( - 'ext5', 'flat', 'physnet5', None, "10.0.50.1", "10.0.50.0/24") + 'ext5', 'flat', self.physnet_used[1], None, '10.0.50.1', + '10.0.50.0/24') router = self._create_router('router5', enable_ecmp=True) gws = self._add_external_gateways( router['id'], @@ -740,12 +740,10 @@ class TestRouter(base.TestOVNFunctionalBase): 'ext1', 'flat', 'physnet6', None, "10.0.60.1", "10.0.60.0/24") gw_info = {'network_id': ext1['network']['id']} - # Attempt to add 4 routers, since there are no chassis all will be - # scheduled on the ovn_const.OVN_GATEWAY_INVALID_CHASSIS. - num_routers = len(self._create_routers_wait_pb(1, 4, gw_info)) - self.assertEqual( - {ovn_const.OVN_GATEWAY_INVALID_CHASSIS: {1: num_routers}}, - self._get_gwc_dict()) + # Attempt to add 4 routers, since there are no chassis, none of them + # will be scheduled on any chassis. + num_routers = len(self._create_routers_wait_pb(1, 4, gw_info=gw_info)) + self.assertEqual({}, self._get_gwc_dict()) # Add 2 chassis and rebalance gateways. # @@ -758,9 +756,7 @@ class TestRouter(base.TestOVNFunctionalBase): with mock.patch.object( self.l3_plugin, 'schedule_unhosted_gateways'): chassis_list.extend(self._add_chassis(1, 2, ['physnet6'])) - self.assertEqual( - {ovn_const.OVN_GATEWAY_INVALID_CHASSIS: {1: num_routers}}, - self._get_gwc_dict()) + self.assertEqual({}, self._get_gwc_dict()) # Wrap `self.l3_plugin._nb_ovn.transaction` so that we can assert on # number of calls. diff --git a/neutron/tests/unit/common/ovn/test_utils.py b/neutron/tests/unit/common/ovn/test_utils.py index b38e177ebe0..30b435d072a 100644 --- a/neutron/tests/unit/common/ovn/test_utils.py +++ b/neutron/tests/unit/common/ovn/test_utils.py @@ -266,7 +266,7 @@ class TestGateWayChassisValidity(base.BaseTestCase): def test_gateway_chassis_due_to_invalid_chassis_name(self): # Return True since chassis is invalid - self.chassis_name = constants.OVN_GATEWAY_INVALID_CHASSIS + self.chassis_name = None self.assertTrue(utils.is_gateway_chassis_invalid( self.chassis_name, self.gw_chassis, self.physnet, self.chassis_physnets, self.az_hints, self.chassis_azs)) 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 c66b67ef2ed..72d7f52b765 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 @@ -181,8 +181,7 @@ class TestNBImplIdlOvn(TestDBImplIdlOvn): 'external_ids': {ovn_const.OVN_ROUTER_NAME_EXT_ID_KEY: 'lr-id-a', ovn_const.OVN_ROUTER_IS_EXT_GW: str(True)}, 'networks': ['10.0.3.0/24'], - 'options': {ovn_const.OVN_GATEWAY_CHASSIS_KEY: - ovn_const.OVN_GATEWAY_INVALID_CHASSIS}}, + 'options': {ovn_const.OVN_GATEWAY_CHASSIS_KEY: None}}, {'name': 'xrp-id-b1', 'external_ids': {}, 'networks': ['20.0.1.0/24']}, {'name': utils.ovn_lrouter_port_name('orp-id-b2'), @@ -627,8 +626,7 @@ class TestNBImplIdlOvn(TestDBImplIdlOvn): expected = {'host-1': [utils.ovn_lrouter_port_name('orp-id-a1'), utils.ovn_lrouter_port_name('orp-id-a2')], 'host-2': [utils.ovn_lrouter_port_name('orp-id-b2')], - ovn_const.OVN_GATEWAY_INVALID_CHASSIS: [ - utils.ovn_name('orp-id-a3')]} + } self.assertCountEqual(bindings, expected) bindings = self.nb_ovn_idl.get_all_chassis_gateway_bindings([]) @@ -654,7 +652,7 @@ class TestNBImplIdlOvn(TestDBImplIdlOvn): self.assertEqual(chassis, ['host-2']) chassis = self.nb_ovn_idl.get_gateway_chassis_binding( utils.ovn_lrouter_port_name('orp-id-a3')) - self.assertEqual(chassis, ['neutron-ovn-invalid-chassis']) + self.assertEqual([], chassis) chassis = self.nb_ovn_idl.get_gateway_chassis_binding( utils.ovn_lrouter_port_name('orp-id-b3')) self.assertEqual([], chassis) diff --git a/neutron/tests/unit/scheduler/test_l3_ovn_scheduler.py b/neutron/tests/unit/scheduler/test_l3_ovn_scheduler.py index 66f836c7952..6c2c523c5e6 100644 --- a/neutron/tests/unit/scheduler/test_l3_ovn_scheduler.py +++ b/neutron/tests/unit/scheduler/test_l3_ovn_scheduler.py @@ -50,8 +50,7 @@ class TestOVNGatewayScheduler(base.BaseTestCase): self.new_gateway_name = 'lrp_new' self.fake_chassis_gateway_mappings = { 'None': {'Chassis': [], - 'Gateways': { - 'g1': [ovn_const.OVN_GATEWAY_INVALID_CHASSIS]}}, + 'Gateways': {'g1': None}}, 'Multiple1': {'Chassis': ['hv1', 'hv2', 'hv3', 'hv4', 'hv5'], 'Gateways': { 'g1': ['hv1', 'hv2', 'hv4', 'hv3', 'hv5'], @@ -102,6 +101,7 @@ class TestOVNGatewayScheduler(base.BaseTestCase): for chassis in details['Chassis']: details['Chassis_Bindings'].setdefault(chassis, []) for gw, chassis_list in details['Gateways'].items(): + chassis_list = chassis_list or [] max_prio = len(chassis_list) for idx, chassis in enumerate(chassis_list): prio = max_prio - idx @@ -138,7 +138,7 @@ class OVNGatewayChanceScheduler(TestOVNGatewayScheduler): gateway_name = random.choice(list(mapping['Gateways'].keys())) chassis = self.select(mapping, gateway_name, chassis_and_azs, candidates=mapping['Chassis']) - self.assertEqual([ovn_const.OVN_GATEWAY_INVALID_CHASSIS], chassis) + self.assertIsNone(chassis) def test_no_chassis_available_for_new_gateway(self): mapping = self.fake_chassis_gateway_mappings['None'] @@ -146,7 +146,7 @@ class OVNGatewayChanceScheduler(TestOVNGatewayScheduler): gateway_name = self.new_gateway_name chassis = self.select(mapping, gateway_name, chassis_and_azs, candidates=mapping['Chassis']) - self.assertEqual([ovn_const.OVN_GATEWAY_INVALID_CHASSIS], chassis) + self.assertIsNone(chassis) def test_random_chassis_available_for_new_gateway(self): mapping = self.fake_chassis_gateway_mappings['Multiple1'] @@ -161,7 +161,7 @@ class OVNGatewayChanceScheduler(TestOVNGatewayScheduler): chassis_and_azs = self.fake_chassis_and_azs['None'] gateway_name = self.new_gateway_name chassis = self.select(mapping, gateway_name, chassis_and_azs) - self.assertEqual([ovn_const.OVN_GATEWAY_INVALID_CHASSIS], chassis) + self.assertIsNone(chassis) self.mock_log.warning.assert_called_once_with( 'Gateway %s was not scheduled on any chassis, no candidates are ' 'available', gateway_name) @@ -179,15 +179,14 @@ class OVNGatewayChanceScheduler(TestOVNGatewayScheduler): nb_idl=nb_idl, gw_chassis=["temp"], physnet='phys-network-1', chassis_physnets=chassis_physnets, - existing_chassis=['temp', - ovn_const.OVN_GATEWAY_INVALID_CHASSIS])) + existing_chassis=['temp', None])) # Check if invalid is removed -II self.assertFalse( self.filter_existing_chassis( nb_idl=nb_idl, gw_chassis=["temp"], physnet='phys-network-1', chassis_physnets=chassis_physnets, - existing_chassis=[ovn_const.OVN_GATEWAY_INVALID_CHASSIS])) + existing_chassis=None)) # Check if chassis removed when physnet doesnt exist self.assertFalse( self.filter_existing_chassis( @@ -236,7 +235,7 @@ class OVNGatewayLeastLoadedScheduler(TestOVNGatewayScheduler): gateway_name = random.choice(list(mapping['Gateways'].keys())) chassis = self.select(mapping, gateway_name, chassis_and_azs, candidates=mapping['Chassis']) - self.assertEqual([ovn_const.OVN_GATEWAY_INVALID_CHASSIS], chassis) + self.assertIsNone(chassis) def test_no_chassis_available_for_new_gateway(self): mapping = self.fake_chassis_gateway_mappings['None'] @@ -244,7 +243,7 @@ class OVNGatewayLeastLoadedScheduler(TestOVNGatewayScheduler): gateway_name = self.new_gateway_name chassis = self.select(mapping, gateway_name, chassis_and_azs, candidates=mapping['Chassis']) - self.assertEqual([ovn_const.OVN_GATEWAY_INVALID_CHASSIS], chassis) + self.assertIsNone(chassis) def test_least_loaded_chassis_available_for_new_gateway1(self): mapping = self.fake_chassis_gateway_mappings['Multiple1'] diff --git a/releasenotes/notes/ovn-l3-scheduler-unbound-gw-ports-46ced10f810d845d.yaml b/releasenotes/notes/ovn-l3-scheduler-unbound-gw-ports-46ced10f810d845d.yaml new file mode 100644 index 00000000000..ff3e9fb4ca7 --- /dev/null +++ b/releasenotes/notes/ovn-l3-scheduler-unbound-gw-ports-46ced10f810d845d.yaml @@ -0,0 +1,13 @@ +--- +other: + - | + The artifact of creating a gateway chassis called + "neutron-ovn-invalid-chassis" when a "Logical_Router_Port" cannot be + assigned to any chassis is removed. Now no gateway chassis is created and + the "Logical_Router_Port" field will be empty. +upgrade: + - | + Any "Logical_Router_Port" with a gateway chassis named + "neutron-ovn-invalid-chassis" will be updated and this chassis will be + deleted. An unhosted (unbound) "Logical_Router_Port" will have no gateway + assigned.