[ovn] AZs distribution in L3 port scheduler

Update l3 ovn schedulers (chance, leastloaded) to ensure that LRP gateways are distributed over chassis in the different eligible AZs.

Previous version already ensure that LRP gateways were scheduled over chassis in eligible AZs. But, depending on the deployment characteristics, all these chassis could be in the same AZ. In some use-cases, it could be needed to have LRP gateways in different AZs to be resilient on failures.

This patch re-order the list of eligible chassis to add a priority on selecting chassis in different AZs.

This should provide a solution for users who need to have their router gateways scheduled on chassis from different AZs.

Closes-Bug: #2030741
Change-Id: I72973abbb8b0f9cc5848fd3b4f6463c38c6595f8

doc/source/admin/ovn/availability_zones.rst

@@ -231,7 +231,8 @@ Each entry on this table represents an instance of the gateway port
 the ``chassis_name`` column indicates which Chassis that port instance
 is scheduled onto. If we co-relate each entry and their ``chassis_name``
 we will see that this port has been only scheduled to Chassis matching
-with the router's availability zones.
+with the router's availability zones and with priority to distribute
+over each zones.
 
 Network Availability Zones
 --------------------------

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

@@ -1654,7 +1654,8 @@ class OVNClient(object):
                 physnet, availability_zone_hints=common_utils.get_az_hints(
                     router))
             selected_chassis = self._ovn_scheduler.select(
-                self._nb_idl, lrouter_port_name, candidates=candidates)
+                self._nb_idl, self._sb_idl, lrouter_port_name,
+                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, gateway_name, candidates=None,
+    def select(self, nb_idl, sb_idl, gateway_name, candidates=None,
                existing_chassis=None):
         """Schedule the gateway port of a router to an OVN chassis.
 
@@ -58,7 +58,7 @@ class OVNGatewayScheduler(object, metaclass=abc.ABCMeta):
                 chassis_list.remove(chassis_name)
         return chassis_list
 
-    def _schedule_gateway(self, nb_idl, gateway_name, candidates,
+    def _schedule_gateway(self, nb_idl, sb_idl, gateway_name, candidates,
                           existing_chassis):
         existing_chassis = existing_chassis or []
         candidates = candidates or []
@@ -82,7 +82,7 @@ class OVNGatewayScheduler(object, metaclass=abc.ABCMeta):
         # column or gateway_chassis column in the OVN_Northbound is done
         # by the caller
         chassis = self._select_gateway_chassis(
-            nb_idl, candidates, 1, chassis_count
+            nb_idl, sb_idl, candidates, 1, chassis_count
         )[:chassis_count]
         # priority of existing chassis is higher than candidates
         chassis = existing_chassis + chassis
@@ -91,8 +91,44 @@ class OVNGatewayScheduler(object, metaclass=abc.ABCMeta):
                   gateway_name, chassis)
         return chassis
 
+    def _reorder_by_az(self, nb_idl, sb_idl, candidates):
+        chassis_selected = []
+        other_chassis = []
+        azs = set()
+
+        # Check if candidates list valid
+        if not candidates or (
+                candidates == [ovn_const.OVN_GATEWAY_INVALID_CHASSIS]):
+            return candidates
+
+        chassis_with_azs = sb_idl.get_chassis_and_azs()
+
+        # Get list of all AZs
+        for chassis in candidates:
+            try:
+                azs.update(chassis_with_azs[chassis])
+            except KeyError:
+                continue
+
+        for chassis in candidates:
+            # Verify if chassis is in an AZ not already used
+            # and delete AZs of chassis from list
+            try:
+                chassis_azs = chassis_with_azs[chassis]
+                if azs.intersection(chassis_azs):
+                    azs = azs.difference(chassis_azs)
+                    chassis_selected += [chassis]
+                else:
+                    other_chassis += [chassis]
+            except KeyError:
+                other_chassis += [chassis]
+
+        chassis_selected += other_chassis
+
+        return chassis_selected
+
     @abc.abstractmethod
-    def _select_gateway_chassis(self, nb_idl, candidates,
+    def _select_gateway_chassis(self, nb_idl, sb_idl, candidates,
                                 priority_min, priority_max):
         """Choose a chassis from candidates based on a specific policy.
 
@@ -105,27 +141,27 @@ class OVNGatewayScheduler(object, metaclass=abc.ABCMeta):
 class OVNGatewayChanceScheduler(OVNGatewayScheduler):
     """Randomly select an chassis for a gateway port of a router"""
 
-    def select(self, nb_idl, gateway_name, candidates=None,
+    def select(self, nb_idl, sb_idl, gateway_name, candidates=None,
                existing_chassis=None):
-        return self._schedule_gateway(nb_idl, gateway_name,
+        return self._schedule_gateway(nb_idl, sb_idl, gateway_name,
-                                      candidates, existing_chassis)
+              candidates, existing_chassis)
 
-    def _select_gateway_chassis(self, nb_idl, candidates,
+    def _select_gateway_chassis(self, nb_idl, sb_idl, candidates,
                                 priority_min, priority_max):
         candidates = copy.deepcopy(candidates)
         random.shuffle(candidates)
-        return candidates
+        return self._reorder_by_az(nb_idl, sb_idl, candidates)
 
 
 class OVNGatewayLeastLoadedScheduler(OVNGatewayScheduler):
     """Select the least loaded chassis for a gateway port of a router"""
 
-    def select(self, nb_idl, gateway_name, candidates=None,
+    def select(self, nb_idl, sb_idl, gateway_name, candidates=None,
                existing_chassis=None):
-        return self._schedule_gateway(nb_idl, gateway_name,
+        return self._schedule_gateway(nb_idl, sb_idl, gateway_name,
                                       candidates, existing_chassis)
 
-    def _select_gateway_chassis(self, nb_idl, candidates,
+    def _select_gateway_chassis(self, nb_idl, sb_idl, candidates,
                                 priority_min, priority_max):
         """Returns a lit of chassis from candidates ordered by priority
         (highest first). Each chassis in every priority will be selected, as it
@@ -149,7 +185,7 @@ class OVNGatewayLeastLoadedScheduler(OVNGatewayScheduler):
                 [chassis for chassis, load in chassis_load.items()
                  if load == leastload])
             selected_chassis.append(chassis)
-        return selected_chassis
+        return self._reorder_by_az(nb_idl, sb_idl, selected_chassis)
 
 
 OVN_SCHEDULER_STR_TO_CLASS = {

neutron/services/ovn_l3/plugin.py

@@ -328,7 +328,7 @@ class OVNL3RouterPlugin(service_base.ServicePluginBase,
                 chassis_physnets=chassis_with_physnets,
                 availability_zone_hints=az_hints)
             chassis = self.scheduler.select(
-                self._nb_ovn, g_name, candidates=candidates,
+                self._nb_ovn, self._sb_ovn, g_name, candidates=candidates,
                 existing_chassis=filtered_existing_chassis)
             if primary and primary != chassis[0]:
                 if primary not in chassis:

neutron/tests/unit/scheduler/test_l3_ovn_scheduler.py

@@ -30,6 +30,11 @@ class FakeOVNGatewaySchedulerNbOvnIdl(object):
                                                                  None))
 
 
+class FakeOVNGatewaySchedulerSbOvnIdl(object):
+    def __init__(self, chassis_and_azs):
+        self.get_chassis_and_azs = mock.Mock(return_value=chassis_and_azs)
+
+
 class TestOVNGatewayScheduler(base.BaseTestCase):
 
     def setUp(self):
@@ -79,6 +84,15 @@ class TestOVNGatewayScheduler(base.BaseTestCase):
                               'g3': ['hv3', 'hv2', 'hv1'],
                               'g4': ['hv3', 'hv2', 'hv1']}}}
 
+        self.fake_chassis_and_azs = {
+            'None': {},
+            'Multiple1': {},
+            'Multiple2': {},
+            'Multiple3': {},
+            'Multiple4': {},
+            'Multiple5': {},
+            'Multiple6': {}}
+
         # Determine the chassis to gateway list bindings
         for details in self.fake_chassis_gateway_mappings.values():
             self.assertNotIn(self.new_gateway_name, details['Gateways'])
@@ -92,10 +106,12 @@ class TestOVNGatewayScheduler(base.BaseTestCase):
                     if chassis in details['Chassis_Bindings']:
                         details['Chassis_Bindings'][chassis].append((gw, prio))
 
-    def select(self, chassis_gateway_mapping, gateway_name, candidates=None):
+    def select(self, chassis_gateway_mapping, gateway_name,
+               chassis_and_azs, candidates=None):
         nb_idl = FakeOVNGatewaySchedulerNbOvnIdl(chassis_gateway_mapping,
                                                  gateway_name)
-        return self.l3_scheduler.select(nb_idl, gateway_name,
+        sb_idl = FakeOVNGatewaySchedulerSbOvnIdl(chassis_and_azs)
+        return self.l3_scheduler.select(nb_idl, sb_idl, gateway_name,
                                         candidates=candidates)
 
     def filter_existing_chassis(self, *args, **kwargs):
@@ -116,29 +132,33 @@ class OVNGatewayChanceScheduler(TestOVNGatewayScheduler):
 
     def test_no_chassis_available_for_existing_gateway(self):
         mapping = self.fake_chassis_gateway_mappings['None']
+        chassis_and_azs = self.fake_chassis_and_azs['None']
         gateway_name = random.choice(list(mapping['Gateways'].keys()))
-        chassis = self.select(mapping, gateway_name,
+        chassis = self.select(mapping, gateway_name, chassis_and_azs,
                               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']
+        chassis_and_azs = self.fake_chassis_and_azs['None']
         gateway_name = self.new_gateway_name
-        chassis = self.select(mapping, gateway_name,
+        chassis = self.select(mapping, gateway_name, chassis_and_azs,
                               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']
+        chassis_and_azs = self.fake_chassis_and_azs['Multiple1']
         gateway_name = self.new_gateway_name
-        chassis = self.select(mapping, gateway_name,
+        chassis = self.select(mapping, gateway_name, chassis_and_azs,
                               candidates=mapping['Chassis'])
         self.assertCountEqual(chassis, mapping.get('Chassis'))
 
     def test_no_candidates_provided(self):
         mapping = self.fake_chassis_gateway_mappings['None']
+        chassis_and_azs = self.fake_chassis_and_azs['None']
         gateway_name = self.new_gateway_name
-        chassis = self.select(mapping, gateway_name)
+        chassis = self.select(mapping, gateway_name, chassis_and_azs)
         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 '
@@ -183,6 +203,25 @@ class OVNGatewayChanceScheduler(TestOVNGatewayScheduler):
                 existing_chassis=['temp']))
 
 
+class OVNGatewayChanceSchedulerWithAZ(OVNGatewayChanceScheduler):
+
+    def setUp(self):
+        super(OVNGatewayChanceSchedulerWithAZ, self).setUp()
+
+        self.fake_chassis_and_azs = {
+            'None': {},
+            'Multiple1': {'hv1': {'az-1'},
+                          'hv2': {'az-0'},
+                          'hv3': {'az-0'},
+                          'hv4': {'az-0'},
+                          'hv5': {'az-0'}},
+            'Multiple2': {},
+            'Multiple3': {},
+            'Multiple4': {},
+            'Multiple5': {},
+            'Multiple6': {}}
+
+
 class OVNGatewayLeastLoadedScheduler(TestOVNGatewayScheduler):
 
     def setUp(self):
@@ -191,22 +230,25 @@ class OVNGatewayLeastLoadedScheduler(TestOVNGatewayScheduler):
 
     def test_no_chassis_available_for_existing_gateway(self):
         mapping = self.fake_chassis_gateway_mappings['None']
+        chassis_and_azs = self.fake_chassis_and_azs['None']
         gateway_name = random.choice(list(mapping['Gateways'].keys()))
-        chassis = self.select(mapping, gateway_name,
+        chassis = self.select(mapping, gateway_name, chassis_and_azs,
                               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']
+        chassis_and_azs = self.fake_chassis_and_azs['None']
         gateway_name = self.new_gateway_name
-        chassis = self.select(mapping, gateway_name,
+        chassis = self.select(mapping, gateway_name, chassis_and_azs,
                               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']
+        chassis_and_azs = self.fake_chassis_and_azs['Multiple1']
         gateway_name = self.new_gateway_name
-        chassis = self.select(mapping, gateway_name,
+        chassis = self.select(mapping, gateway_name, chassis_and_azs,
                               candidates=mapping['Chassis'])
         self.assertCountEqual(chassis, mapping.get('Chassis'))
         # least loaded will be the first one in the list,
@@ -215,32 +257,36 @@ class OVNGatewayLeastLoadedScheduler(TestOVNGatewayScheduler):
 
     def test_least_loaded_chassis_available_for_new_gateway2(self):
         mapping = self.fake_chassis_gateway_mappings['Multiple2']
+        chassis_and_azs = self.fake_chassis_and_azs['Multiple2']
         gateway_name = self.new_gateway_name
-        chassis = self.select(mapping, gateway_name,
+        chassis = self.select(mapping, gateway_name, chassis_and_azs,
                               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']
+        chassis_and_azs = self.fake_chassis_and_azs['Multiple3']
         gateway_name = self.new_gateway_name
-        chassis = self.select(mapping, gateway_name,
+        chassis = self.select(mapping, gateway_name, chassis_and_azs,
                               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']
+        chassis_and_azs = self.fake_chassis_and_azs['Multiple4']
         gateway_name = self.new_gateway_name
-        chassis = self.select(mapping, gateway_name,
+        chassis = self.select(mapping, gateway_name, chassis_and_azs,
                               candidates=mapping['Chassis'])
         # least loaded chassis will be in the front of the list
         self.assertEqual(['hv2', 'hv1'], chassis)
 
     def test_least_loaded_chassis_per_priority(self):
         mapping = self.fake_chassis_gateway_mappings['Multiple5']
+        chassis_and_azs = self.fake_chassis_and_azs['Multiple5']
         gateway_name = self.new_gateway_name
-        chassis = self.select(mapping, gateway_name,
+        chassis = self.select(mapping, gateway_name, chassis_and_azs,
                               candidates=mapping['Chassis'])
         # we should now have the following hv's per priority:
         # p5: hv2 (since it currently does not have p5 ports)
@@ -258,8 +304,9 @@ class OVNGatewayLeastLoadedScheduler(TestOVNGatewayScheduler):
 
     def test_least_loaded_chassis_per_priority2(self):
         mapping = self.fake_chassis_gateway_mappings['Multiple6']
+        chassis_and_azs = self.fake_chassis_and_azs['Multiple6']
         gateway_name = self.new_gateway_name
-        chassis = self.select(mapping, gateway_name,
+        chassis = self.select(mapping, gateway_name, chassis_and_azs,
                               candidates=mapping['Chassis'])
         # we should now have the following hv's per priority:
         # p3: hv2 (since it currently does not have p3 ports)
@@ -273,7 +320,46 @@ class OVNGatewayLeastLoadedScheduler(TestOVNGatewayScheduler):
 
     def test_existing_chassis_available_for_existing_gateway(self):
         mapping = self.fake_chassis_gateway_mappings['Multiple1']
+        chassis_and_azs = self.fake_chassis_and_azs['Multiple1']
         gateway_name = random.choice(list(mapping['Gateways'].keys()))
-        chassis = self.select(mapping, gateway_name,
+        chassis = self.select(mapping, gateway_name, chassis_and_azs,
                               candidates=mapping['Chassis'])
         self.assertEqual(ovn_const.MAX_GW_CHASSIS, len(chassis))
+
+
+class OVNGatewayLeastLoadedSchedulerWithAZ(OVNGatewayLeastLoadedScheduler):
+
+    def setUp(self):
+        super(OVNGatewayLeastLoadedSchedulerWithAZ, self).setUp()
+
+        self.fake_chassis_and_azs = {
+            'None': {},
+            'Multiple1': {'hv1': {'az-0'},
+                          'hv2': {'az-0'},
+                          'hv3': {'az-1', 'az-3'},
+                          'hv4': {'az-2'},
+                          'hv5': {'az-1', 'az-2'}},
+            'Multiple2': {},
+            'Multiple3': {},
+            'Multiple4': {},
+            'Multiple5': {},
+            'Multiple6': {}}
+
+    def test_least_loaded_chassis_available_for_new_gateway1(self):
+        mapping = self.fake_chassis_gateway_mappings['Multiple1']
+        chassis_and_azs = self.fake_chassis_and_azs['Multiple1']
+        gateway_name = self.new_gateway_name
+        chassis = self.select(mapping, gateway_name, chassis_and_azs,
+                              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
+        #
+        # Order of chassis is now changed to take care of spreading over AZs :
+        # hv5 is in az-1 and az-2 so it can stay first (priority order)
+        # hv4 is in az-2, already in hv5 so it goes to the remaining
+        # hv3 is in az-1, already in hv5 but az-3 is not, so hv3 is the next
+        # one
+        # hv2 is in az-0 which is new, so it's the next
+        # After, hv4 and hv1 are remaining ones, stay in original order.
+        self.assertEqual(['hv5', 'hv3', 'hv2', 'hv4', 'hv1'], chassis)

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

@@ -1787,10 +1787,12 @@ 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(), 'lrp-foo-1', [],
+            mock.call(self.nb_idl(), self.sb_idl(),
-                      ['chassis1', 'chassis2']),
+                      'lrp-foo-1', [], ['chassis1', 'chassis2']),
-            mock.call(self.nb_idl(), 'lrp-foo-2', [], ['chassis2']),
+            mock.call(self.nb_idl(), self.sb_idl(),
-            mock.call(self.nb_idl(), 'lrp-foo-3', [], [])])
+                      'lrp-foo-2', [], ['chassis2']),
+            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',
@@ -1873,13 +1875,13 @@ 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(), 'lrp-foo-1',
+            mock.call(self.nb_idl(), self.sb_idl(), 'lrp-foo-1',
                       ['chassis1', 'chassis3', 'chassis4'],
                       ['chassis1']),
-            mock.call(self.nb_idl(), 'lrp-foo-2',
+            mock.call(self.nb_idl(), self.sb_idl(), 'lrp-foo-2',
                       ['chassis1', 'chassis3', 'chassis4'],
                       ['chassis4']),
-            mock.call(self.nb_idl(), 'lrp-foo-3',
+            mock.call(self.nb_idl(), self.sb_idl(), 'lrp-foo-3',
                       ['chassis1', 'chassis3', 'chassis4'],
                       ['chassis4', 'chassis3', 'chassis1'])])
         # make sure that the primary chassis stays untouched

releasenotes/notes/bug-2030741-f4c780df9cf3db4e.yaml

@@ -0,0 +1,6 @@
+---
+fixes:
+  - |
+    Fixes L3 OVN schedulers (chance, leastloaded) to distribute Logical
+    Router Port (LRP) gateways among all eligible AZs (if any).
+    For more information, see bug `2030741 <https://launchpad.net/bugs/2030741>`_