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

diff --git a/neutron/db/l3_agentschedulers_db.py b/neutron/db/l3_agentschedulers_db.py
index b929544791276c48825d9fdaf0f0adf9caec20e1..c0c2052eace37f3f2f39b160ea3adf407924fd99 100644 (file)
--- 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 aa407056cd7d86cd56809f8b1c4c82e8a895c477..bb873a73b695ecb6c24a84e34ba2ac6f0791e0a8 100644 (file)
--- 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 6ecf3bf5d54c5b5c3da6cd611d509934672a2201..069fb10523d0adf624d927774018413aaa387b2b 100644 (file)
--- 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,