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 d75db6a00ff..1f8186b21cf 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 @@ -1687,7 +1687,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 8e238e39d37..9190262dd8a 100644 --- a/neutron/tests/unit/services/ovn_l3/test_plugin.py +++ b/neutron/tests/unit/services/ovn_l3/test_plugin.py @@ -1811,10 +1811,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', @@ -1897,13 +1899,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 `_