[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

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,
+                self._nb_idl, lrouter_port_name, candidates=candidates)
-                candidates=candidates)
             if selected_chassis:
                 columns['gateway_chassis'] = selected_chassis
 

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

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:

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, gateway_name,
-        return self.l3_scheduler.select(nb_idl, sb_idl,
+                                        candidates=candidates)
-                                        gateway_name)
 
     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):

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(),
+            mock.call(self.nb_idl(), 'lrp-foo-1', [],
-                      'lrp-foo-1', [], ['chassis1', 'chassis2']),
+                      ['chassis1', 'chassis2']),
-            mock.call(self.nb_idl(), self.sb_idl(),
+            mock.call(self.nb_idl(), 'lrp-foo-2', [], ['chassis2']),
-                      'lrp-foo-2', [], ['chassis2']),
+            mock.call(self.nb_idl(), 'lrp-foo-3', [], [])])
-            mock.call(self.nb_idl(), self.sb_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,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.