[OVN] The L3 scheduler does not use all chassis by default

Any OVN scheduler, inheriting from ``OVNGatewayScheduler``, calls
``_schedule_gateway`` to make the decision of in what chassis the
router gatweway should be located. Before this patch, if the list
of candidates was empty, the scheduler used all available chassis
as candidate list. This patch is removing this default behaviour.
In a deployment, only those chassis marked explicitly with
"ovn-cms-options=enable-chassis-as-gw" can be used as gateway
chassis.

After enabling this patch, any existing router gateway port will
preserve the assigned chassis; any new router gateway will be
scheduled only on the chassis configured as gateways.

If a router gateway port cannot find any chassis to be scheduled,
the "neutron-ovn-invalid-chassis" will be used instead and a
warning message will be printed in the logs.

Closes-Bug: #2019217
Change-Id: If0f843463edfd7edc5c897cc098de31444f9d81b
(cherry picked from commit 413044f)

Author: ralonsoh

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

@@ -1651,8 +1651,7 @@ def _create_lrouter_port(self, context, router, port, txn=None):
                 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
 

neutron/scheduler/l3_ovn_scheduler.py

@@ -35,7 +35,7 @@ def __init__(self):
         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 @@ def filter_existing_chassis(self, nb_idl, gw_chassis,
                 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 @@ def _schedule_gateway(self, nb_idl, sb_idl, gateway_name, candidates,
                     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 @@ def _schedule_gateway(self, nb_idl, sb_idl, gateway_name, candidates,
     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 @@ def _select_gateway_chassis(self, nb_idl, candidates):
 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

neutron/services/ovn_l3/plugin.py

@@ -434,7 +434,7 @@ def schedule_unhosted_gateways(self, event_from_chassis=None):
                 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:

neutron/tests/unit/scheduler/test_l3_ovn_scheduler.py

@@ -30,16 +30,11 @@ def __init__(self, chassis_gateway_mapping, gateway):
                                                                  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 @@ def setUp(self):
                     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 @@ def setUp(self):
     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 @@ def setUp(self):
     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 @@ def test_least_loaded_chassis_available_for_new_gateway1(self):
     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):

neutron/tests/unit/services/ovn_l3/test_plugin.py

@@ -34,6 +34,7 @@
 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.db import extraroute_db
 from neutron import manager as neutron_manager
@@ -65,6 +66,7 @@ def _start_mock(self, path, return_value, new_callable=None):
     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()
@@ -1559,12 +1561,10 @@ def test_schedule_unhosted_gateways(self, get_gppm):
                       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',

releasenotes/notes/ovn-l3-scheduler-only-on-gw-chassis-33c22c1f5f7a73d4.yaml

@@ -0,0 +1,6 @@
+---
+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.