From 5a480be4a806e9f9b6e87f217a43fcc5fd8553dc Mon Sep 17 00:00:00 2001 From: YAMAMOTO Takashi Date: Tue, 27 Jan 2015 15:32:19 +0900 Subject: [PATCH] Fix a usage error of joinedload + filter in l3 scheduler This commit fixes admin_state_up filtering in get_l3_agents_hosting_routers. Also, adapt its callers which rely on the current broken implementation. Details: With the current coding, joinedload() produces a JOIN and the following filter() on the columns from the joined table would create another JOIN of the same table. (As t1 in the following example). It doesn't seem to be the intended behaviour. As a consequence the filter (WHERE clause in the following examples) doesn't work as expected. Queries before this fix looked like the following, where t1 and t2 are Agent and RouterL3AgentBinding respectively: SELECT t2.aaa, t1_1.bbb, ... FROM t1, t2 LEFT OUTER JOIN t1 AS t1_1 ON t1_1.ccc = t2.ddd WHERE t1.eee = ...; After the fix, it would be: SELECT t2.aaa, t1.bbb, ... FROM t2 JOIN t1 ON t1.ccc = t2.ddd WHERE t1.eee = ...; Reference: http://docs.sqlalchemy.org/en/rel_0_9/orm/loading_relationships.html#contains-eager Partial-Bug: #1414905 Closes-Bug: #1410841 Change-Id: I2243cdfda5c6fe5ef67f96e3274d5381a6e50e62 --- neutron/db/l3_agentschedulers_db.py | 7 +++++-- neutron/scheduler/l3_agent_scheduler.py | 2 +- neutron/tests/unit/test_l3_schedulers.py | 25 ++++++++++++++++++++++++ 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/neutron/db/l3_agentschedulers_db.py b/neutron/db/l3_agentschedulers_db.py index b9295447912..c0c2052eace 100644 --- a/neutron/db/l3_agentschedulers_db.py +++ b/neutron/db/l3_agentschedulers_db.py @@ -309,11 +309,14 @@ class L3AgentSchedulerDbMixin(l3agentscheduler.L3AgentSchedulerPluginBase, if not router_ids: return [] query = context.session.query(RouterL3AgentBinding) + query = query.options(orm.contains_eager( + RouterL3AgentBinding.l3_agent)) + query = query.join(RouterL3AgentBinding.l3_agent) if len(router_ids) > 1: - query = query.options(joinedload('l3_agent')).filter( + query = query.filter( RouterL3AgentBinding.router_id.in_(router_ids)) else: - query = query.options(joinedload('l3_agent')).filter( + query = query.filter( RouterL3AgentBinding.router_id == router_ids[0]) if admin_state_up is not None: query = (query.filter(agents_db.Agent.admin_state_up == diff --git a/neutron/scheduler/l3_agent_scheduler.py b/neutron/scheduler/l3_agent_scheduler.py index aa407056cd7..bb873a73b69 100644 --- a/neutron/scheduler/l3_agent_scheduler.py +++ b/neutron/scheduler/l3_agent_scheduler.py @@ -65,7 +65,7 @@ class L3Scheduler(object): unscheduled_routers = [] for router in routers: l3_agents = plugin.get_l3_agents_hosting_routers( - context, [router['id']], admin_state_up=True) + context, [router['id']]) if l3_agents: LOG.debug('Router %(router_id)s has already been ' 'hosted by L3 agent %(agent_id)s', diff --git a/neutron/tests/unit/test_l3_schedulers.py b/neutron/tests/unit/test_l3_schedulers.py index 6ecf3bf5d54..069fb10523d 100644 --- a/neutron/tests/unit/test_l3_schedulers.py +++ b/neutron/tests/unit/test_l3_schedulers.py @@ -695,6 +695,31 @@ class L3SchedulerTestBaseMixin(object): l3_agent, router['id']) self.assertTrue(val) + def test_get_l3_agents_hosting_routers(self): + agent = self._register_l3_agent('host_6') + router = self._make_router(self.fmt, + tenant_id=str(uuid.uuid4()), + name='r1') + ctx = self.adminContext + router_id = router['router']['id'] + self.plugin.router_scheduler.bind_router(ctx, router_id, agent) + agents = self.get_l3_agents_hosting_routers(ctx, + [router_id]) + self.assertEqual([agent.id], [agt.id for agt in agents]) + agents = self.get_l3_agents_hosting_routers(ctx, + [router_id], + admin_state_up=True) + self.assertEqual([agent.id], [agt.id for agt in agents]) + + self._set_l3_agent_admin_state(ctx, agent.id, False) + agents = self.get_l3_agents_hosting_routers(ctx, + [router_id]) + self.assertEqual([agent.id], [agt.id for agt in agents]) + agents = self.get_l3_agents_hosting_routers(ctx, + [router_id], + admin_state_up=True) + self.assertEqual([], agents) + class L3SchedulerTestCase(l3_agentschedulers_db.L3AgentSchedulerDbMixin, l3_db.L3_NAT_db_mixin,