From 2bb514f2d73a64b6f954990a2ff12f5baa3a97cd Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Wed, 24 Jun 2020 09:58:09 +0000 Subject: [PATCH] L3 agent scheduler should return a valid index if manual scheduling When retrieving a vacant L3 agent binding index, if "is_manual_scheduling" is set, the method "get_vacant_binding_index" should always return a valid binding index. If the existing binding indexes are sequentially aligned, the method will return a new one on top; if there is a gap in the binding indexes list, the first free index will be returned. Closes-Bug: #1884906 Change-Id: I0a89bca0734d3e735fb357e488f85589e81d709f --- neutron/db/l3_agentschedulers_db.py | 19 ++---- neutron/scheduler/base_scheduler.py | 37 ++++++++++++ neutron/scheduler/dhcp_agent_scheduler.py | 27 +-------- .../unit/scheduler/test_base_scheduler.py | 52 ++++++++++++++++ .../scheduler/test_dhcp_agent_scheduler.py | 59 ------------------- 5 files changed, 96 insertions(+), 98 deletions(-) create mode 100644 neutron/tests/unit/scheduler/test_base_scheduler.py diff --git a/neutron/db/l3_agentschedulers_db.py b/neutron/db/l3_agentschedulers_db.py index 8cdf89716a8..51cb8ca249e 100644 --- a/neutron/db/l3_agentschedulers_db.py +++ b/neutron/db/l3_agentschedulers_db.py @@ -34,6 +34,7 @@ from neutron.objects import agent as ag_obj from neutron.objects import base as base_obj from neutron.objects import l3agent as rb_obj from neutron.objects import router as l3_objs +from neutron.scheduler import base_scheduler LOG = logging.getLogger(__name__) @@ -520,21 +521,9 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase, pager = base_obj.Pager(sorts=[('binding_index', True)]) bindings = rb_obj.RouterL3AgentBinding.get_objects( context, _pager=pager, router_id=router_id) - binding_indices = [b.binding_index for b in bindings] - all_indicies = set(range(rb_model.LOWEST_BINDING_INDEX, - num_agents + 1)) - open_slots = sorted(list(all_indicies - set(binding_indices))) - - if open_slots: - return open_slots[0] - - # Last chance: if this is a manual scheduling, we're gonna allow - # creation of a binding_index even if it will exceed - # max_l3_agents_per_router. - if is_manual_scheduling: - return max(all_indicies) + 1 - - return -1 + return base_scheduler.get_vacant_binding_index( + num_agents, bindings, rb_model.LOWEST_BINDING_INDEX, + force_scheduling=is_manual_scheduling) class AZL3AgentSchedulerDbMixin(L3AgentSchedulerDbMixin, diff --git a/neutron/scheduler/base_scheduler.py b/neutron/scheduler/base_scheduler.py index d7fa604e404..35eb2e348de 100644 --- a/neutron/scheduler/base_scheduler.py +++ b/neutron/scheduler/base_scheduler.py @@ -83,3 +83,40 @@ class BaseWeightScheduler(BaseScheduler): chosen_agents = sorted(resource_hostable_agents, key=attrgetter('load'))[0:num_agents_needed] return chosen_agents + + +def get_vacant_binding_index(num_agents, bindings, lowest_binding_index, + force_scheduling=False): + """Return a vacant binding_index to use and whether or not it exists. + + This method can be used with DHCP and L3 agent schedulers. It will return + the lowest vacant index for one of those agents. + :param num_agents: (int) number of agents (DHCP, L3) already scheduled + :param bindings: (NetworkDhcpAgentBinding, RouterL3AgentBinding) agent + binding object, must have "binding_index" field. + :param lowest_binding_index: (int) lowest index number to be scheduled. + :param force_scheduling: (optional)(boolean) if enabled, the method will + always return an index, even if this number + exceeds the maximum configured number of agents. + """ + binding_indices = [b.binding_index for b in bindings] + all_indices = set(range(lowest_binding_index, num_agents + 1)) + open_slots = sorted(list(all_indices - set(binding_indices))) + + if open_slots: + return open_slots[0] + + if not force_scheduling: + return -1 + + # Last chance: if this is a manual scheduling, we're gonna allow + # creation of a binding_index even if it will exceed + # dhcp_agents_per_network. + if max(binding_indices) == len(binding_indices): + return max(binding_indices) + 1 + else: + # Find binding index set gaps and return first free one. + all_indices = set(range(lowest_binding_index, + max(binding_indices) + 1)) + open_slots = sorted(list(all_indices - set(binding_indices))) + return open_slots[0] diff --git a/neutron/scheduler/dhcp_agent_scheduler.py b/neutron/scheduler/dhcp_agent_scheduler.py index 266dec14cca..6b835a6bcc4 100644 --- a/neutron/scheduler/dhcp_agent_scheduler.py +++ b/neutron/scheduler/dhcp_agent_scheduler.py @@ -192,32 +192,11 @@ class DhcpFilter(base_resource_filter.BaseResourceFilter): num_agents = agent_obj.Agent.count( context, agent_type=constants.AGENT_TYPE_DHCP) num_agents = min(num_agents, cfg.CONF.dhcp_agents_per_network) - bindings = network.NetworkDhcpAgentBinding.get_objects( context, network_id=network_id) - - binding_indices = [b.binding_index for b in bindings] - all_indices = set(range(ndab_model.LOWEST_BINDING_INDEX, - num_agents + 1)) - open_slots = sorted(list(all_indices - set(binding_indices))) - - if open_slots: - return open_slots[0] - - if not force_scheduling: - return -1 - - # Last chance: if this is a manual scheduling, we're gonna allow - # creation of a binding_index even if it will exceed - # dhcp_agents_per_network. - if max(binding_indices) == len(binding_indices): - return max(binding_indices) + 1 - else: - # Find binding index set gaps and return first free one. - all_indices = set(range(ndab_model.LOWEST_BINDING_INDEX, - max(binding_indices) + 1)) - open_slots = sorted(list(all_indices - set(binding_indices))) - return open_slots[0] + return base_scheduler.get_vacant_binding_index( + num_agents, bindings, ndab_model.LOWEST_BINDING_INDEX, + force_scheduling=force_scheduling) def bind(self, context, agents, network_id, force_scheduling=False): """Bind the network to the agents.""" diff --git a/neutron/tests/unit/scheduler/test_base_scheduler.py b/neutron/tests/unit/scheduler/test_base_scheduler.py new file mode 100644 index 00000000000..2acae6ab803 --- /dev/null +++ b/neutron/tests/unit/scheduler/test_base_scheduler.py @@ -0,0 +1,52 @@ +# Copyright 2020 Red Hat Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or +# implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +from unittest import mock + +from neutron.scheduler import base_scheduler + +from neutron.tests import base + + +class GetVacantBindingFilterCase(base.BaseTestCase): + + def test_get_vacant_binding_index_no_agents(self): + ret = base_scheduler.get_vacant_binding_index(0, [], 1) + self.assertEqual(-1, ret) + + def test_get_vacant_binding_index_several_agents(self): + ret = base_scheduler.get_vacant_binding_index(1, [], 1) + self.assertEqual(1, ret) + + ret = base_scheduler.get_vacant_binding_index( + 1, [mock.Mock(binding_index=1)], 1) + self.assertEqual(-1, ret) + + ret = base_scheduler.get_vacant_binding_index( + 3, [mock.Mock(binding_index=1), mock.Mock(binding_index=3)], 1) + self.assertEqual(2, ret) + + def test_get_vacant_binding_index_force_scheduling(self): + ret = base_scheduler.get_vacant_binding_index( + 3, [mock.Mock(binding_index=1), mock.Mock(binding_index=2), + mock.Mock(binding_index=3), mock.Mock(binding_index=5), + mock.Mock(binding_index=7)], 1, force_scheduling=True) + self.assertEqual(4, ret) + + ret = base_scheduler.get_vacant_binding_index( + 3, [mock.Mock(binding_index=1), mock.Mock(binding_index=2), + mock.Mock(binding_index=3), mock.Mock(binding_index=4), + mock.Mock(binding_index=5)], 1, force_scheduling=True) + self.assertEqual(6, ret) diff --git a/neutron/tests/unit/scheduler/test_dhcp_agent_scheduler.py b/neutron/tests/unit/scheduler/test_dhcp_agent_scheduler.py index 300123876b6..3cc6c8b65dc 100644 --- a/neutron/tests/unit/scheduler/test_dhcp_agent_scheduler.py +++ b/neutron/tests/unit/scheduler/test_dhcp_agent_scheduler.py @@ -31,7 +31,6 @@ from neutron.objects import agent from neutron.objects import network as network_obj from neutron.scheduler import dhcp_agent_scheduler from neutron.services.segments import db as segments_service_db -from neutron.tests import base from neutron.tests.common import helpers from neutron.tests.unit.plugins.ml2 import test_plugin from neutron.tests.unit import testlib_api @@ -931,61 +930,3 @@ class DHCPAgentAZAwareWeightSchedulerTestCase(TestDhcpSchedulerBaseTestCase): # which is also not in the same az as the first selected agent. self.assertEqual('az2-host2', agents_select[1]['host']) self.assertEqual('az2', agents_select[1]['availability_zone']) - - -class DhcpFilterTestCase(base.BaseTestCase): - - def setUp(self): - super(DhcpFilterTestCase, self).setUp() - self.mock_agent_count = mock.patch.object(agent.Agent, 'count').start() - self.mock_dhcpagent_bindings = mock.patch.object( - network_obj.NetworkDhcpAgentBinding, 'get_objects').start() - cfg.CONF.set_override('dhcp_agents_per_network', 3) - self.dhcp_filter = dhcp_agent_scheduler.DhcpFilter() - - def test_get_vacant_network_dhcp_agent_binding_index_no_agents(self): - self.mock_agent_count.return_value = 0 - self.mock_dhcpagent_bindings.return_value = [] - ret = self.dhcp_filter.get_vacant_network_dhcp_agent_binding_index( - mock.ANY, mock.ANY, False) - self.assertEqual(-1, ret) - - def test_get_vacant_network_dhcp_agent_binding_index_several_agents(self): - self.mock_agent_count.return_value = 1 - self.mock_dhcpagent_bindings.return_value = [] - ret = self.dhcp_filter.get_vacant_network_dhcp_agent_binding_index( - mock.ANY, mock.ANY, False) - self.assertEqual(1, ret) - - self.mock_agent_count.return_value = 1 - self.mock_dhcpagent_bindings.return_value = [ - mock.Mock(binding_index=1)] - ret = self.dhcp_filter.get_vacant_network_dhcp_agent_binding_index( - mock.ANY, mock.ANY, False) - self.assertEqual(-1, ret) - - self.mock_agent_count.return_value = 3 - self.mock_dhcpagent_bindings.return_value = [ - mock.Mock(binding_index=1), mock.Mock(binding_index=3)] - ret = self.dhcp_filter.get_vacant_network_dhcp_agent_binding_index( - mock.ANY, mock.ANY, False) - self.assertEqual(2, ret) - - def test_get_vacant_network_dhcp_agent_binding_index_force_sched(self): - self.mock_agent_count.return_value = 3 - self.mock_dhcpagent_bindings.return_value = [ - mock.Mock(binding_index=1), mock.Mock(binding_index=2), - mock.Mock(binding_index=3), mock.Mock(binding_index=5), - mock.Mock(binding_index=7)] - ret = self.dhcp_filter.get_vacant_network_dhcp_agent_binding_index( - mock.ANY, mock.ANY, True) - self.assertEqual(4, ret) - - self.mock_agent_count.return_value = 3 - self.mock_dhcpagent_bindings.return_value = [ - mock.Mock(binding_index=1), mock.Mock(binding_index=2), - mock.Mock(binding_index=3), mock.Mock(binding_index=4), - mock.Mock(binding_index=5)] - ret = self.dhcp_filter.get_vacant_network_dhcp_agent_binding_index( - mock.ANY, mock.ANY, True) - self.assertEqual(6, ret)