From 1cf679513471d411acbdafef1bcab1df6edd6108 Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Wed, 8 Feb 2023 13:14:19 +0100 Subject: [PATCH] Improve scheduling L3/DHCP agents, missing lower binding indexes This patch is covering an edge case that could happen when the number of DHCP agents ("dhcp_agents_per_network") or L3 agents ("max_l3_agents_per_router") has been reduced and there are more agents assigned than the current number. If the user removes any agent assignation from a L3 router or a DHCP agent, it is possible to remove first the lower binding assigned registers. Now the method ``get_vacant_binding_index`` calculates the number of agents bound and the number required. If a new one is needed, the method returns first the lower binding indexes not used. Closes-Bug: #2006496 Conflicts: neutron/common/_constants.py neutron/objects/l3agent.py Change-Id: I25145c088ffdca47acfcb7add02b1a4a615e4612 (cherry picked from commit 5250598c804a38c55ff78cfb457b73d1b3cd7e07) (cherry picked from commit 0920f17f476ce8b398deea3e54e9f90b5251cfc9) (cherry picked from commit 7dcf8be112ed205a6c694c1f3549e08b4234d82d) --- neutron/common/_constants.py | 3 ++ neutron/db/l3_agentschedulers_db.py | 4 +- neutron/db/models/l3agent.py | 8 ++-- .../db/network_dhcp_agent_binding/models.py | 9 ++-- neutron/objects/l3agent.py | 3 +- neutron/scheduler/base_scheduler.py | 46 +++++++++++++------ neutron/scheduler/dhcp_agent_scheduler.py | 6 +-- neutron/scheduler/l3_agent_scheduler.py | 5 +- .../unit/scheduler/test_base_scheduler.py | 21 +++++++++ 9 files changed, 75 insertions(+), 30 deletions(-) diff --git a/neutron/common/_constants.py b/neutron/common/_constants.py index bb1da44e264..e88cc5bc989 100644 --- a/neutron/common/_constants.py +++ b/neutron/common/_constants.py @@ -78,3 +78,6 @@ IDPOOL_SELECT_SIZE = 100 AUTO_DELETE_PORT_OWNERS = [constants.DEVICE_OWNER_DHCP, constants.DEVICE_OWNER_DISTRIBUTED, constants.DEVICE_OWNER_AGENT_GW] + +# The lowest binding index for L3 agents and DHCP agents. +LOWEST_AGENT_BINDING_INDEX = 1 diff --git a/neutron/db/l3_agentschedulers_db.py b/neutron/db/l3_agentschedulers_db.py index 51cb8ca249e..6a2be2c48c6 100644 --- a/neutron/db/l3_agentschedulers_db.py +++ b/neutron/db/l3_agentschedulers_db.py @@ -25,9 +25,9 @@ from oslo_log import log as logging import oslo_messaging from neutron.agent.common import utils as agent_utils +from neutron.common import _constants as n_const from neutron.conf.db import l3_agentschedulers_db from neutron.db import agentschedulers_db -from neutron.db.models import l3agent as rb_model from neutron.extensions import l3agentscheduler from neutron.extensions import router_availability_zone as router_az from neutron.objects import agent as ag_obj @@ -522,7 +522,7 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase, bindings = rb_obj.RouterL3AgentBinding.get_objects( context, _pager=pager, router_id=router_id) return base_scheduler.get_vacant_binding_index( - num_agents, bindings, rb_model.LOWEST_BINDING_INDEX, + num_agents, bindings, n_const.LOWEST_AGENT_BINDING_INDEX, force_scheduling=is_manual_scheduling) diff --git a/neutron/db/models/l3agent.py b/neutron/db/models/l3agent.py index 8ca12b3786d..97c68ea7dc7 100644 --- a/neutron/db/models/l3agent.py +++ b/neutron/db/models/l3agent.py @@ -15,10 +15,9 @@ from neutron_lib.db import model_base import sqlalchemy as sa from sqlalchemy import orm +from neutron.common import _constants as n_const from neutron.db.models import agent as agent_model -LOWEST_BINDING_INDEX = 1 - class RouterL3AgentBinding(model_base.BASEV2): """Represents binding between neutron routers and L3 agents.""" @@ -37,5 +36,6 @@ class RouterL3AgentBinding(model_base.BASEV2): l3_agent_id = sa.Column(sa.String(36), sa.ForeignKey("agents.id", ondelete='CASCADE'), primary_key=True) - binding_index = sa.Column(sa.Integer, nullable=False, - server_default=str(LOWEST_BINDING_INDEX)) + binding_index = sa.Column( + sa.Integer, nullable=False, + server_default=str(n_const.LOWEST_AGENT_BINDING_INDEX)) diff --git a/neutron/db/network_dhcp_agent_binding/models.py b/neutron/db/network_dhcp_agent_binding/models.py index d3147df53fd..d30f61a65b3 100644 --- a/neutron/db/network_dhcp_agent_binding/models.py +++ b/neutron/db/network_dhcp_agent_binding/models.py @@ -14,12 +14,10 @@ from neutron_lib.db import model_base import sqlalchemy as sa from sqlalchemy import orm +from neutron.common import _constants as n_const from neutron.db.models import agent as agent_model -LOWEST_BINDING_INDEX = 1 - - class NetworkDhcpAgentBinding(model_base.BASEV2): """Represents binding between neutron networks and DHCP agents.""" @@ -38,5 +36,6 @@ class NetworkDhcpAgentBinding(model_base.BASEV2): sa.ForeignKey("agents.id", ondelete='CASCADE'), primary_key=True) - binding_index = sa.Column(sa.Integer, nullable=False, - server_default=str(LOWEST_BINDING_INDEX)) + binding_index = sa.Column( + sa.Integer, nullable=False, + server_default=str(n_const.LOWEST_AGENT_BINDING_INDEX)) diff --git a/neutron/objects/l3agent.py b/neutron/objects/l3agent.py index 07606e76f3e..0c86e7973de 100644 --- a/neutron/objects/l3agent.py +++ b/neutron/objects/l3agent.py @@ -17,6 +17,7 @@ from sqlalchemy.orm import joinedload from sqlalchemy import sql +from neutron.common import _constants as n_const from neutron.db.models import agent as agent_model from neutron.db.models import l3_attrs from neutron.db.models import l3agent @@ -36,7 +37,7 @@ class RouterL3AgentBinding(base.NeutronDbObject): 'router_id': common_types.UUIDField(), 'l3_agent_id': common_types.UUIDField(), 'binding_index': obj_fields.IntegerField( - default=l3agent.LOWEST_BINDING_INDEX), + default=n_const.LOWEST_AGENT_BINDING_INDEX), } # TODO(ihrachys) return OVO objects not models diff --git a/neutron/scheduler/base_scheduler.py b/neutron/scheduler/base_scheduler.py index 35eb2e348de..a8cd7741bbd 100644 --- a/neutron/scheduler/base_scheduler.py +++ b/neutron/scheduler/base_scheduler.py @@ -99,24 +99,44 @@ def get_vacant_binding_index(num_agents, bindings, lowest_binding_index, 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))) + def get_open_slots(binding_indices, lowest_binding_index, max_number): + """Returns an ordered list of free slots + This list starts from the lowest available binding index. The number + of open slots and "binding_indices" (those already taken), must be + equal to "max_number". The list returned can be [], if + len(max_number) == len(binding_indices) (that means there are no free + slots). + """ + # NOTE(ralonsoh): check LP#2006496 for more context. The DHCP/router + # binding indexes could not be a sequential list starting from + # lowest_binding_index (that is usually 1). + open_slots = set(binding_indices) + idx = lowest_binding_index + while len(open_slots) < max_number: + # Increase sequentially the "open_slots" set until we have the + # required number of slots, that is "num_agents". + open_slots.add(idx) + idx += 1 + + # Remove those indices already used. + open_slots -= set(binding_indices) + return sorted(list(open_slots)) + + binding_indices = [b.binding_index for b in bindings] + open_slots = get_open_slots(binding_indices, lowest_binding_index, + num_agents) 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 + # Last chance: if this is a manual scheduling, we're going to 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] + # dhcp_agents_per_network/max_l3_agents_per_router. + while not open_slots: + num_agents += 1 + open_slots = get_open_slots(binding_indices, lowest_binding_index, + num_agents) + return open_slots[0] diff --git a/neutron/scheduler/dhcp_agent_scheduler.py b/neutron/scheduler/dhcp_agent_scheduler.py index 6b835a6bcc4..0cd9c1a9470 100644 --- a/neutron/scheduler/dhcp_agent_scheduler.py +++ b/neutron/scheduler/dhcp_agent_scheduler.py @@ -25,7 +25,7 @@ from oslo_config import cfg from oslo_log import log as logging from neutron.agent.common import utils as agent_utils -from neutron.db.network_dhcp_agent_binding import models as ndab_model +from neutron.common import _constants as n_const from neutron.objects import agent as agent_obj from neutron.objects import network from neutron.scheduler import base_resource_filter @@ -195,7 +195,7 @@ class DhcpFilter(base_resource_filter.BaseResourceFilter): bindings = network.NetworkDhcpAgentBinding.get_objects( context, network_id=network_id) return base_scheduler.get_vacant_binding_index( - num_agents, bindings, ndab_model.LOWEST_BINDING_INDEX, + num_agents, bindings, n_const.LOWEST_AGENT_BINDING_INDEX, force_scheduling=force_scheduling) def bind(self, context, agents, network_id, force_scheduling=False): @@ -205,7 +205,7 @@ class DhcpFilter(base_resource_filter.BaseResourceFilter): for agent in agents: binding_index = self.get_vacant_network_dhcp_agent_binding_index( context, network_id, force_scheduling) - if binding_index < ndab_model.LOWEST_BINDING_INDEX: + if binding_index < n_const.LOWEST_AGENT_BINDING_INDEX: LOG.debug('Unable to find a vacant binding_index for ' 'network %(network_id)s and agent %(agent_id)s', {'network_id': network_id, diff --git a/neutron/scheduler/l3_agent_scheduler.py b/neutron/scheduler/l3_agent_scheduler.py index 498eba4d8d8..15904fe78a5 100644 --- a/neutron/scheduler/l3_agent_scheduler.py +++ b/neutron/scheduler/l3_agent_scheduler.py @@ -27,6 +27,7 @@ from oslo_config import cfg from oslo_db import exception as db_exc from oslo_log import log as logging +from neutron.common import _constants as n_const from neutron.common import utils from neutron.conf.db import l3_hamode_db from neutron.db.models import l3agent as rb_model @@ -186,7 +187,7 @@ class L3Scheduler(object, metaclass=abc.ABCMeta): return if not is_ha: - binding_index = rb_model.LOWEST_BINDING_INDEX + binding_index = n_const.LOWEST_AGENT_BINDING_INDEX if rb_obj.RouterL3AgentBinding.objects_exist( context, router_id=router_id, binding_index=binding_index): LOG.debug('Non-HA router %s has already been scheduled', @@ -195,7 +196,7 @@ class L3Scheduler(object, metaclass=abc.ABCMeta): else: binding_index = plugin.get_vacant_binding_index( context, router_id, is_manual_scheduling) - if binding_index < rb_model.LOWEST_BINDING_INDEX: + if binding_index < n_const.LOWEST_AGENT_BINDING_INDEX: LOG.debug('Unable to find a vacant binding_index for ' 'router %(router_id)s and agent %(agent_id)s', {'router_id': router_id, diff --git a/neutron/tests/unit/scheduler/test_base_scheduler.py b/neutron/tests/unit/scheduler/test_base_scheduler.py index 2acae6ab803..6a6a82f2bb8 100644 --- a/neutron/tests/unit/scheduler/test_base_scheduler.py +++ b/neutron/tests/unit/scheduler/test_base_scheduler.py @@ -38,6 +38,21 @@ class GetVacantBindingFilterCase(base.BaseTestCase): 3, [mock.Mock(binding_index=1), mock.Mock(binding_index=3)], 1) self.assertEqual(2, ret) + # Binding list starting in 2, two elements, required three. + ret = base_scheduler.get_vacant_binding_index( + 3, [mock.Mock(binding_index=2), mock.Mock(binding_index=3)], 1) + self.assertEqual(1, ret) + + # Binding list starting in 2, two elements, required two. + ret = base_scheduler.get_vacant_binding_index( + 2, [mock.Mock(binding_index=2), mock.Mock(binding_index=3)], 1) + self.assertEqual(-1, ret) + + # Binding list starting in 2, two elements, required one. + ret = base_scheduler.get_vacant_binding_index( + 1, [mock.Mock(binding_index=2), mock.Mock(binding_index=3)], 1) + self.assertEqual(-1, 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), @@ -50,3 +65,9 @@ class GetVacantBindingFilterCase(base.BaseTestCase): mock.Mock(binding_index=3), mock.Mock(binding_index=4), mock.Mock(binding_index=5)], 1, force_scheduling=True) self.assertEqual(6, ret) + + ret = base_scheduler.get_vacant_binding_index( + 3, [mock.Mock(binding_index=2), mock.Mock(binding_index=3), + mock.Mock(binding_index=4), mock.Mock(binding_index=5), + mock.Mock(binding_index=6)], 1, force_scheduling=True) + self.assertEqual(1, ret)