From a29ea3724e1f6bb54b76d1b9915c13014272fdcd Mon Sep 17 00:00:00 2001 From: Yann Morice Date: Thu, 24 Aug 2023 15:56:48 +0200 Subject: [PATCH] [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 | 3 +- .../ovn/mech_driver/ovsdb/ovn_client.py | 3 +- neutron/scheduler/l3_ovn_scheduler.py | 62 ++++++++-- neutron/services/ovn_l3/plugin.py | 2 +- .../unit/scheduler/test_l3_ovn_scheduler.py | 116 +++++++++++++++--- .../tests/unit/services/ovn_l3/test_plugin.py | 16 +-- .../notes/bug-2030741-f4c780df9cf3db4e.yaml | 6 + 7 files changed, 170 insertions(+), 38 deletions(-) create mode 100644 releasenotes/notes/bug-2030741-f4c780df9cf3db4e.yaml diff --git a/doc/source/admin/ovn/availability_zones.rst b/doc/source/admin/ovn/availability_zones.rst index a12599c3b7c..ca8d16e28a0 100644 --- a/doc/source/admin/ovn/availability_zones.rst +++ b/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 -------------------------- 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 c47c00a2ca7..9b458fd04a2 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 @@ -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 diff --git a/neutron/scheduler/l3_ovn_scheduler.py b/neutron/scheduler/l3_ovn_scheduler.py index 09418860e5a..1a6c1226254 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, 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, - candidates, existing_chassis) + 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): 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 = { diff --git a/neutron/services/ovn_l3/plugin.py b/neutron/services/ovn_l3/plugin.py index b2c18691ada..6067bcdefc4 100644 --- a/neutron/services/ovn_l3/plugin.py +++ b/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: diff --git a/neutron/tests/unit/scheduler/test_l3_ovn_scheduler.py b/neutron/tests/unit/scheduler/test_l3_ovn_scheduler.py index 4ec2e818895..068cd4e1c0d 100644 --- a/neutron/tests/unit/scheduler/test_l3_ovn_scheduler.py +++ b/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) diff --git a/neutron/tests/unit/services/ovn_l3/test_plugin.py b/neutron/tests/unit/services/ovn_l3/test_plugin.py index 4612daed589..8d183cf5b08 100644 --- a/neutron/tests/unit/services/ovn_l3/test_plugin.py +++ b/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', [], - ['chassis1', 'chassis2']), - mock.call(self.nb_idl(), 'lrp-foo-2', [], ['chassis2']), - mock.call(self.nb_idl(), 'lrp-foo-3', [], [])]) + 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', [], [])]) # 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 diff --git a/releasenotes/notes/bug-2030741-f4c780df9cf3db4e.yaml b/releasenotes/notes/bug-2030741-f4c780df9cf3db4e.yaml new file mode 100644 index 00000000000..588fe4bcf55 --- /dev/null +++ b/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 `_