Merge "[OVN] The L3 scheduler does not use all chassis by default" into stable/2023.1

Author: Zuul

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.