From: Michael Smith <michael.smith6@hp.com>
Date: Tue, 2 Sep 2014 17:09:12 +0000 (+0000)
Subject: Avoid an extra database query in schedule_snat_router
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=5a36f0ad24903a3a8ec213840ab164834917a32e;p=openstack-build%2Fneutron-build.git

Avoid an extra database query in schedule_snat_router

bind_snat_servicenode already knows the chosen_agent.  By waiting
until after it is called, we can use it to avoid another db query.

Closes-bug: #1353266
Closes-bug: #1356639
Co-Authored-By: Swaminathan Vasudevan <swaminathan.vasudevan@hp.com>

Change-Id: Iba5668b2717e188d5289ede982bfc1b46f4eeb5b
---

diff --git a/neutron/db/l3_dvrscheduler_db.py b/neutron/db/l3_dvrscheduler_db.py
index d26dae202..2797c73a8 100644
--- a/neutron/db/l3_dvrscheduler_db.py
+++ b/neutron/db/l3_dvrscheduler_db.py
@@ -237,6 +237,7 @@ class L3_DVRsch_db_mixin(l3agent_sch_db.L3AgentSchedulerDbMixin):
         """Bind the snat router to the chosen l3 service agent."""
         chosen_snat_agent = random.choice(snat_candidates)
         self.bind_snat_router(context, router_id, chosen_snat_agent)
+        return chosen_snat_agent
 
     def unbind_snat_servicenode(self, context, router_id):
         """Unbind the snat router to the chosen l3 service agent."""
@@ -290,18 +291,6 @@ class L3_DVRsch_db_mixin(l3agent_sch_db.L3AgentSchedulerDbMixin):
 
     def schedule_snat_router(self, context, router_id, sync_router):
         """Schedule the snat router on l3 service agent."""
-        binding = (context.session.
-                   query(CentralizedSnatL3AgentBinding).
-                   filter_by(router_id=router_id).first())
-        if binding:
-            l3_agent_id = binding.l3_agent_id
-            l3_agent = binding.l3_agent
-            LOG.debug('SNAT Router %(router_id)s has already been '
-                      'hosted by L3 agent '
-                      '%(l3_agent_id)s', {'router_id': router_id,
-                                          'l3_agent_id': l3_agent_id})
-            self.bind_dvr_router_servicenode(context, router_id, l3_agent)
-            return
         active_l3_agents = self.get_l3_agents(context, active=True)
         if not active_l3_agents:
             LOG.warn(_('No active L3 agents found for SNAT'))
@@ -309,4 +298,7 @@ class L3_DVRsch_db_mixin(l3agent_sch_db.L3AgentSchedulerDbMixin):
         snat_candidates = self.get_snat_candidates(sync_router,
                                                    active_l3_agents)
         if snat_candidates:
-            self.bind_snat_servicenode(context, router_id, snat_candidates)
+            chosen_agent = self.bind_snat_servicenode(
+                context, router_id, snat_candidates)
+            self.bind_dvr_router_servicenode(
+                context, router_id, chosen_agent)
diff --git a/neutron/tests/unit/test_l3_schedulers.py b/neutron/tests/unit/test_l3_schedulers.py
index 7c0319c2b..97d04152b 100644
--- a/neutron/tests/unit/test_l3_schedulers.py
+++ b/neutron/tests/unit/test_l3_schedulers.py
@@ -953,25 +953,6 @@ class L3DvrSchedulerTestCase(testlib_api.SqlTestCase,
         }
         return agent, router
 
-    def test_schedule_snat_router_with_gateway_and_nobinding(self):
-        agent, router = self._prepare_schedule_snat_tests()
-        with contextlib.nested(
-            mock.patch.object(query.Query, 'first'),
-            mock.patch.object(self.dut, 'get_l3_agents'),
-            mock.patch.object(self.dut, 'get_snat_candidates'),
-            mock.patch.object(self.dut, 'get_router'),
-            mock.patch.object(self.dut, 'bind_dvr_router_servicenode'),
-            mock.patch.object(self.dut, 'bind_snat_servicenode')) as (
-                mock_query, mock_agents,
-                mock_candidates, mock_rd, mock_dvr, mock_bind):
-            mock_rd.return_value = router
-            mock_query.return_value = []
-            mock_agents.return_value = [agent]
-            mock_candidates.return_value = [agent]
-            self.dut.schedule_snat_router(
-                self.adminContext, 'foo_router_id', router)
-        self.assertFalse(mock_dvr.called)
-
     def test_schedule_router_unbind_snat_servicenode_negativetest(self):
         router = {
             'id': 'foo_router_id',