From: armando-migliaccio <armamig@gmail.com>
Date: Tue, 1 Jul 2014 22:08:11 +0000 (-0700)
Subject: Fix UnboundLocalError raised during L3 router sync task
X-Git-Url: https://review.fuel-infra.org/gitweb?a=commitdiff_plain;h=fc01ddaaa5f1170b746f47837104352bfe36eaa5;p=openstack-build%2Fneutron-build.git

Fix UnboundLocalError raised during L3 router sync task

This can be fixed in a number of ways: a) consolidating the
two except clauses into one; b) adding a 'return' after the
last except clause c) by calling the cleanup method only on
success; d) initializing 'routers' before usage.

Approach c) has the benefit of stating the developer's intent
more explicitly and minimize chances of regression.

Closes-bug: #1336566

Change-Id: Ib91ac5bb07869cbec6825b60d7c4c5b23c4c4d3a
---

diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py
index e4c5aff49..e2da8967c 100644
--- a/neutron/agent/l3_agent.py
+++ b/neutron/agent/l3_agent.py
@@ -843,6 +843,9 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
 
     @periodic_task.periodic_task
     @lockutils.synchronized('l3-agent', 'neutron-')
+    def periodic_sync_routers_task(self, context):
+        self._sync_routers_task(context)
+
     def _sync_routers_task(self, context):
         if self.services_sync:
             super(L3NATAgent, self).process_services_sync(context)
@@ -864,15 +867,13 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
         except n_rpc.RPCException:
             LOG.exception(_("Failed synchronizing routers due to RPC error"))
             self.fullsync = True
-            return
         except Exception:
             LOG.exception(_("Failed synchronizing routers"))
             self.fullsync = True
-
-        # Resync is not necessary for the cleanup of stale
-        # namespaces.
-        if self._clean_stale_namespaces:
-            self._cleanup_namespaces(routers)
+        else:
+            # Resync is not necessary for the cleanup of stale namespaces
+            if self._clean_stale_namespaces:
+                self._cleanup_namespaces(routers)
 
     def after_start(self):
         LOG.info(_("L3 agent started"))
diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py
index 841425dfb..511b2f421 100644
--- a/neutron/tests/unit/test_l3_agent.py
+++ b/neutron/tests/unit/test_l3_agent.py
@@ -91,6 +91,22 @@ class TestBasicRouterOperations(base.BaseTestCase):
             'neutron.openstack.common.loopingcall.FixedIntervalLoopingCall')
         self.looping_call_p.start()
 
+    def test__sync_routers_task_raise_exception(self):
+        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
+        self.plugin_api.get_routers.side_effect = Exception()
+        with mock.patch.object(agent, '_cleanup_namespaces') as f:
+            agent._sync_routers_task(agent.context)
+        self.assertFalse(f.called)
+
+    def test__sync_routers_task_call_clean_stale_namespaces(self):
+        agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
+        self.plugin_api.get_routers.return_value = mock.ANY
+        with mock.patch.object(agent, '_cleanup_namespaces') as f:
+            with mock.patch.object(agent, '_process_routers') as g:
+                agent._sync_routers_task(agent.context)
+        self.assertTrue(f.called)
+        g.assert_called_with(mock.ANY, all_routers=True)
+
     def test_router_info_create(self):
         id = _uuid()
         ri = l3_agent.RouterInfo(id, self.conf.root_helper,