From 1105d732b2cb6ec66d042c85968d47fe6d733f5f Mon Sep 17 00:00:00 2001 From: armando-migliaccio Date: Wed, 20 Aug 2014 12:15:42 -0700 Subject: [PATCH] Avoid auto-scheduling for distributed routers The reason for this is twofold: - It may relieve contention on DB access while both servers and l3 agents are busy setting up and syncing routers down respectively. - It prevents accidental placement of namespaces during the L3 sync_routers process, as auto scheduling without taking into account the state of the L3 agents, as well as the state of the routers being processed, may overrule the placement decision made during router operations. Partial-bug: #1356121 Partial-bug: #1359326 Change-Id: Ia677ce212145d6cee65adeb1d8ae594e6ac5e34d --- neutron/scheduler/l3_agent_scheduler.py | 35 ++++++++++++------------ neutron/tests/unit/test_l3_schedulers.py | 23 ++++++++++------ 2 files changed, 31 insertions(+), 27 deletions(-) diff --git a/neutron/scheduler/l3_agent_scheduler.py b/neutron/scheduler/l3_agent_scheduler.py index 5e3823fb5..6854a01a0 100644 --- a/neutron/scheduler/l3_agent_scheduler.py +++ b/neutron/scheduler/l3_agent_scheduler.py @@ -41,23 +41,13 @@ class L3Scheduler(object): """ pass - def dvr_has_binding(self, context, router_id, l3_agent_id): - router_binding_model = l3_agentschedulers_db.RouterL3AgentBinding - - query = context.session.query(router_binding_model) - query = query.filter(router_binding_model.router_id == router_id, - router_binding_model.l3_agent_id == l3_agent_id) - - return query.count() > 0 - def filter_unscheduled_routers(self, context, plugin, routers): """Filter from list of routers the ones that are not scheduled.""" unscheduled_routers = [] for router in routers: l3_agents = plugin.get_l3_agents_hosting_routers( context, [router['id']], admin_state_up=True) - # TODO(armando-migliaccio): remove dvr-related check - if l3_agents and not router.get('distributed', False): + if l3_agents: LOG.debug(('Router %(router_id)s has already been ' 'hosted by L3 agent %(agent_id)s'), {'router_id': router['id'], @@ -79,19 +69,28 @@ class L3Scheduler(object): context, filters={'id': unscheduled_router_ids}) return [] - def get_routers_to_schedule(self, context, plugin, router_ids=None): + def get_routers_to_schedule(self, context, plugin, + router_ids=None, exclude_distributed=False): """Verify that the routers specified need to be scheduled. :param context: the context :param plugin: the core plugin :param router_ids: the list of routers to be checked for scheduling + :param exclude_distributed: whether or not to consider dvr routers :returns: the list of routers to be scheduled """ if router_ids is not None: routers = plugin.get_routers(context, filters={'id': router_ids}) - return self.filter_unscheduled_routers(context, plugin, routers) + unscheduled_routers = self.filter_unscheduled_routers( + context, plugin, routers) else: - return self.get_unscheduled_routers(context, plugin) + unscheduled_routers = self.get_unscheduled_routers(context, plugin) + + if exclude_distributed: + unscheduled_routers = [ + r for r in unscheduled_routers if not r.get('distributed') + ] + return unscheduled_routers def get_routers_can_schedule(self, context, plugin, routers, l3_agent): """Get the subset of routers that can be scheduled on the L3 agent.""" @@ -121,8 +120,11 @@ class L3Scheduler(object): if not l3_agent: return False + # NOTE(armando-migliaccio): DVR routers should not be auto + # scheduled because auto-scheduling may interfere with the + # placement rules for IR and SNAT namespaces. unscheduled_routers = self.get_routers_to_schedule( - context, plugin, router_ids) + context, plugin, router_ids, exclude_distributed=True) if not unscheduled_routers: return False @@ -174,9 +176,6 @@ class L3Scheduler(object): def bind_routers(self, context, routers, l3_agent): for router in routers: - if (router.get('distributed', False) and - self.dvr_has_binding(context, router['id'], l3_agent.id)): - continue self.bind_router(context, router['id'], l3_agent) def bind_router(self, context, router_id, chosen_agent): diff --git a/neutron/tests/unit/test_l3_schedulers.py b/neutron/tests/unit/test_l3_schedulers.py index 14ef8327d..d13a78dbc 100644 --- a/neutron/tests/unit/test_l3_schedulers.py +++ b/neutron/tests/unit/test_l3_schedulers.py @@ -171,6 +171,20 @@ class L3SchedulerBaseTestCase(base.BaseTestCase): mock_get.assert_called_once_with(mock.ANY, self.plugin) self.assertEqual(expected_routers, unscheduled_routers) + def test_get_routers_to_schedule_exclude_distributed(self): + routers = [ + {'id': 'foo_router1', 'distributed': True}, {'id': 'foo_router_2'} + ] + expected_routers = [{'id': 'foo_router_2'}] + with mock.patch.object(self.scheduler, + 'get_unscheduled_routers') as mock_get: + mock_get.return_value = routers + unscheduled_routers = self.scheduler.get_routers_to_schedule( + mock.ANY, self.plugin, + router_ids=None, exclude_distributed=True) + mock_get.assert_called_once_with(mock.ANY, self.plugin) + self.assertEqual(expected_routers, unscheduled_routers) + def _test_get_routers_can_schedule(self, routers, agent, target_routers): self.plugin.get_l3_agent_candidates.return_value = agent result = self.scheduler.get_routers_can_schedule( @@ -207,15 +221,6 @@ class L3SchedulerBaseTestCase(base.BaseTestCase): self.scheduler.bind_routers(mock.ANY, routers, mock.ANY) mock_bind.assert_called_once_with(mock.ANY, 'foo_router', mock.ANY) - def test_bind_routers_dvr(self): - routers = [{'id': 'foo_router', 'distributed': True}] - agent = agents_db.Agent(id='foo_agent') - with mock.patch.object(self.scheduler, 'dvr_has_binding') as mock_dvr: - with mock.patch.object(self.scheduler, 'bind_router') as mock_bind: - self.scheduler.bind_routers(mock.ANY, routers, agent) - mock_dvr.assert_called_once_with(mock.ANY, 'foo_router', 'foo_agent') - self.assertFalse(mock_bind.called) - class L3SchedulerTestExtensionManager(object): -- 2.45.2