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 506d5482461..a956d60a028 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 @@ -1593,8 +1593,7 @@ class OVNClient(object): physnet, availability_zone_hints=common_utils.get_az_hints( router)) selected_chassis = self._ovn_scheduler.select( - self._nb_idl, self._sb_idl, lrouter_port_name, - candidates=candidates) + self._nb_idl, lrouter_port_name, candidates=candidates) if selected_chassis: columns['gateway_chassis'] = selected_chassis diff --git a/neutron/scheduler/l3_ovn_scheduler.py b/neutron/scheduler/l3_ovn_scheduler.py index c762cdb91f0..19f1d780f3a 100644 --- a/neutron/scheduler/l3_ovn_scheduler.py +++ b/neutron/scheduler/l3_ovn_scheduler.py @@ -35,7 +35,7 @@ class OVNGatewayScheduler(object, metaclass=abc.ABCMeta): pass @abc.abstractmethod - def select(self, nb_idl, sb_idl, gateway_name, candidates=None): + def select(self, nb_idl, gateway_name, candidates=None): """Schedule the gateway port of a router to an OVN chassis. Schedule the gateway router port only if it is not already @@ -57,10 +57,10 @@ class OVNGatewayScheduler(object, metaclass=abc.ABCMeta): chassis_list.remove(chassis_name) return chassis_list - def _schedule_gateway(self, nb_idl, sb_idl, gateway_name, candidates, + def _schedule_gateway(self, nb_idl, gateway_name, candidates, existing_chassis): existing_chassis = existing_chassis or [] - candidates = candidates or self._get_chassis_candidates(sb_idl) + candidates = candidates or [] candidates = list(set(candidates) - set(existing_chassis)) # If no candidates, or gateway scheduled on MAX_GATEWAY_CHASSIS nodes # or all candidates in existing_chassis, return existing_chassis. @@ -70,6 +70,8 @@ class OVNGatewayScheduler(object, metaclass=abc.ABCMeta): len(existing_chassis) == ovn_const.MAX_GW_CHASSIS): return existing_chassis 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] chassis_count = ovn_const.MAX_GW_CHASSIS - len(existing_chassis) # The actual binding of the gateway to a chassis via the options @@ -88,20 +90,13 @@ class OVNGatewayScheduler(object, metaclass=abc.ABCMeta): def _select_gateway_chassis(self, nb_idl, candidates): """Choose a chassis from candidates based on a specific policy.""" - def _get_chassis_candidates(self, sb_idl): - # TODO(azbiswas): Allow selection of a specific type of chassis when - # the upstream code merges. - # return (sb_idl.get_all_chassis('gateway_router') or - # sb_idl.get_all_chassis()) - return sb_idl.get_all_chassis() - class OVNGatewayChanceScheduler(OVNGatewayScheduler): """Randomly select an chassis for a gateway port of a router""" - def select(self, nb_idl, sb_idl, gateway_name, candidates=None, + def select(self, nb_idl, gateway_name, candidates=None, existing_chassis=None): - return self._schedule_gateway(nb_idl, sb_idl, gateway_name, + return self._schedule_gateway(nb_idl, gateway_name, candidates, existing_chassis) def _select_gateway_chassis(self, nb_idl, candidates): @@ -113,9 +108,9 @@ class OVNGatewayChanceScheduler(OVNGatewayScheduler): class OVNGatewayLeastLoadedScheduler(OVNGatewayScheduler): """Select the least loaded chassis for a gateway port of a router""" - def select(self, nb_idl, sb_idl, gateway_name, candidates=None, + def select(self, nb_idl, gateway_name, candidates=None, existing_chassis=None): - return self._schedule_gateway(nb_idl, sb_idl, gateway_name, + return self._schedule_gateway(nb_idl, gateway_name, candidates, existing_chassis) @staticmethod diff --git a/neutron/services/ovn_l3/plugin.py b/neutron/services/ovn_l3/plugin.py index 7d370089a2e..f74baba3005 100644 --- a/neutron/services/ovn_l3/plugin.py +++ b/neutron/services/ovn_l3/plugin.py @@ -435,7 +435,7 @@ class OVNL3RouterPlugin(service_base.ServicePluginBase, chassis_physnets=chassis_with_physnets, availability_zone_hints=az_hints) chassis = self.scheduler.select( - self._nb_ovn, self._sb_ovn, g_name, candidates=candidates, + self._nb_ovn, g_name, candidates=candidates, existing_chassis=existing_chassis) if primary and primary != chassis[0]: if primary not in chassis: diff --git a/neutron/tests/unit/scheduler/test_l3_ovn_scheduler.py b/neutron/tests/unit/scheduler/test_l3_ovn_scheduler.py index 3c10462b585..cd7ea4499d3 100644 --- a/neutron/tests/unit/scheduler/test_l3_ovn_scheduler.py +++ b/neutron/tests/unit/scheduler/test_l3_ovn_scheduler.py @@ -30,16 +30,11 @@ class FakeOVNGatewaySchedulerNbOvnIdl(object): None)) -class FakeOVNGatewaySchedulerSbOvnIdl(object): - def __init__(self, chassis_gateway_mapping): - self.get_all_chassis = mock.Mock( - return_value=chassis_gateway_mapping['Chassis']) - - class TestOVNGatewayScheduler(base.BaseTestCase): def setUp(self): super(TestOVNGatewayScheduler, self).setUp() + self.mock_log = mock.patch.object(l3_ovn_scheduler, 'LOG').start() # Overwritten by derived classes self.l3_scheduler = None @@ -82,12 +77,11 @@ class TestOVNGatewayScheduler(base.BaseTestCase): if chassis in details['Chassis_Bindings']: details['Chassis_Bindings'][chassis].append((gw, 0)) - def select(self, chassis_gateway_mapping, gateway_name): + def select(self, chassis_gateway_mapping, gateway_name, candidates=None): nb_idl = FakeOVNGatewaySchedulerNbOvnIdl(chassis_gateway_mapping, gateway_name) - sb_idl = FakeOVNGatewaySchedulerSbOvnIdl(chassis_gateway_mapping) - return self.l3_scheduler.select(nb_idl, sb_idl, - gateway_name) + return self.l3_scheduler.select(nb_idl, gateway_name, + candidates=candidates) def filter_existing_chassis(self, *args, **kwargs): return self.l3_scheduler.filter_existing_chassis( @@ -108,21 +102,33 @@ class OVNGatewayChanceScheduler(TestOVNGatewayScheduler): def test_no_chassis_available_for_existing_gateway(self): mapping = self.fake_chassis_gateway_mappings['None'] gateway_name = random.choice(list(mapping['Gateways'].keys())) - chassis = self.select(mapping, gateway_name) + chassis = self.select(mapping, gateway_name, + candidates=mapping['Chassis']) self.assertEqual([ovn_const.OVN_GATEWAY_INVALID_CHASSIS], chassis) def test_no_chassis_available_for_new_gateway(self): mapping = self.fake_chassis_gateway_mappings['None'] gateway_name = self.new_gateway_name - chassis = self.select(mapping, gateway_name) + chassis = self.select(mapping, gateway_name, + candidates=mapping['Chassis']) self.assertEqual([ovn_const.OVN_GATEWAY_INVALID_CHASSIS], chassis) def test_random_chassis_available_for_new_gateway(self): mapping = self.fake_chassis_gateway_mappings['Multiple1'] gateway_name = self.new_gateway_name - chassis = self.select(mapping, gateway_name) + chassis = self.select(mapping, gateway_name, + candidates=mapping['Chassis']) self.assertCountEqual(chassis, mapping.get('Chassis')) + def test_no_candidates_provided(self): + mapping = self.fake_chassis_gateway_mappings['None'] + gateway_name = self.new_gateway_name + chassis = self.select(mapping, gateway_name) + self.assertEqual([ovn_const.OVN_GATEWAY_INVALID_CHASSIS], chassis) + self.mock_log.warning.assert_called_once_with( + 'Gateway %s was not scheduled on any chassis, no candidates are ' + 'available', gateway_name) + def test_filter_existing_chassis(self): # filter_existing_chassis is scheduler independent, but calling # it from Base class didnt seem right. Also, there is no need to have @@ -171,19 +177,22 @@ class OVNGatewayLeastLoadedScheduler(TestOVNGatewayScheduler): def test_no_chassis_available_for_existing_gateway(self): mapping = self.fake_chassis_gateway_mappings['None'] gateway_name = random.choice(list(mapping['Gateways'].keys())) - chassis = self.select(mapping, gateway_name) + chassis = self.select(mapping, gateway_name, + candidates=mapping['Chassis']) self.assertEqual([ovn_const.OVN_GATEWAY_INVALID_CHASSIS], chassis) def test_no_chassis_available_for_new_gateway(self): mapping = self.fake_chassis_gateway_mappings['None'] gateway_name = self.new_gateway_name - chassis = self.select(mapping, gateway_name) + chassis = self.select(mapping, gateway_name, + candidates=mapping['Chassis']) self.assertEqual([ovn_const.OVN_GATEWAY_INVALID_CHASSIS], chassis) def test_least_loaded_chassis_available_for_new_gateway1(self): mapping = self.fake_chassis_gateway_mappings['Multiple1'] gateway_name = self.new_gateway_name - chassis = self.select(mapping, gateway_name) + chassis = self.select(mapping, gateway_name, + candidates=mapping['Chassis']) self.assertCountEqual(chassis, mapping.get('Chassis')) # least loaded will be the first one in the list, # networking-ovn will assign highest priority to this first element @@ -192,28 +201,32 @@ class OVNGatewayLeastLoadedScheduler(TestOVNGatewayScheduler): def test_least_loaded_chassis_available_for_new_gateway2(self): mapping = self.fake_chassis_gateway_mappings['Multiple2'] gateway_name = self.new_gateway_name - chassis = self.select(mapping, gateway_name) + chassis = self.select(mapping, gateway_name, + candidates=mapping['Chassis']) # hv1 will have least priority self.assertEqual(chassis[2], 'hv1') def test_least_loaded_chassis_available_for_new_gateway3(self): mapping = self.fake_chassis_gateway_mappings['Multiple3'] gateway_name = self.new_gateway_name - chassis = self.select(mapping, gateway_name) + chassis = self.select(mapping, gateway_name, + candidates=mapping['Chassis']) # least loaded chassis will be in the front of the list self.assertEqual(['hv1', 'hv3', 'hv2'], chassis) def test_least_loaded_chassis_with_rebalance(self): mapping = self.fake_chassis_gateway_mappings['Multiple4'] gateway_name = self.new_gateway_name - chassis = self.select(mapping, gateway_name) + chassis = self.select(mapping, gateway_name, + candidates=mapping['Chassis']) # least loaded chassis will be in the front of the list self.assertEqual(['hv2', 'hv1'], chassis) def test_existing_chassis_available_for_existing_gateway(self): mapping = self.fake_chassis_gateway_mappings['Multiple1'] gateway_name = random.choice(list(mapping['Gateways'].keys())) - chassis = self.select(mapping, gateway_name) + chassis = self.select(mapping, gateway_name, + candidates=mapping['Chassis']) self.assertEqual(ovn_const.MAX_GW_CHASSIS, len(chassis)) def test__get_chassis_load_by_prios_several_ports(self): diff --git a/neutron/tests/unit/services/ovn_l3/test_plugin.py b/neutron/tests/unit/services/ovn_l3/test_plugin.py index a4f1512555b..04a65b4e202 100644 --- a/neutron/tests/unit/services/ovn_l3/test_plugin.py +++ b/neutron/tests/unit/services/ovn_l3/test_plugin.py @@ -32,6 +32,7 @@ from oslo_utils import uuidutils from neutron.api.v2 import router as api_router from neutron.common.ovn import constants as ovn_const from neutron.common.ovn import utils +from neutron.conf.plugins.ml2.drivers import driver_type as driver_type_conf from neutron.conf.plugins.ml2.drivers.ovn import ovn_conf as config from neutron import manager as neutron_manager from neutron.plugins.ml2 import managers @@ -62,6 +63,7 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): def setUp(self): mock.patch('neutron.plugins.ml2.drivers.ovn.mech_driver.ovsdb.' 'impl_idl_ovn.Backend.schema_helper').start() + driver_type_conf.register_ml2_drivers_geneve_opts(cfg=cfg.CONF) cfg.CONF.set_override('max_header_size', 38, group='ml2_type_geneve') super(TestOVNL3RouterPlugin, self).setUp() revision_plugin.RevisionPlugin() @@ -1526,12 +1528,10 @@ class TestOVNL3RouterPlugin(test_mech_driver.Ml2PluginV2TestCase): chassis_physnets=chassis_mappings, cms=chassis, availability_zone_hints=[])] * 3) self.mock_schedule.assert_has_calls([ - mock.call(self.nb_idl(), self.sb_idl(), - 'lrp-foo-1', [], ['chassis1', 'chassis2']), - mock.call(self.nb_idl(), self.sb_idl(), - 'lrp-foo-2', [], ['chassis2']), - mock.call(self.nb_idl(), self.sb_idl(), - 'lrp-foo-3', [], [])]) + mock.call(self.nb_idl(), 'lrp-foo-1', [], + ['chassis1', 'chassis2']), + mock.call(self.nb_idl(), 'lrp-foo-2', [], ['chassis2']), + mock.call(self.nb_idl(), 'lrp-foo-3', [], [])]) # make sure that for second port primary chassis stays untouched self.nb_idl().update_lrouter_port.assert_has_calls([ mock.call('lrp-foo-1', diff --git a/releasenotes/notes/ovn-l3-scheduler-only-on-gw-chassis-33c22c1f5f7a73d4.yaml b/releasenotes/notes/ovn-l3-scheduler-only-on-gw-chassis-33c22c1f5f7a73d4.yaml new file mode 100644 index 00000000000..1f52acfe930 --- /dev/null +++ b/releasenotes/notes/ovn-l3-scheduler-only-on-gw-chassis-33c22c1f5f7a73d4.yaml @@ -0,0 +1,12 @@ +--- +features: + - | + The OVN L3 scheduler will assign chassis explicitly configured as gateways + to the router gateway ports (OVN logical router ports). If no candidates are + available, the "neutron-ovn-invalid-chassis" will be used instead and a + warning message will be printed in the logs. +upgrade: + - | + In ML2/OVN, any new router gateway port (OVN logical router port) will be + scheduled only on those chassis configured as gateway. Any existing router + gateway port will preserve the current chassis assignation.