x/vmware-nsx
Code Issues Proposed changes

Fix UnboundLocalError raised during L3 router sync task

Browse Source 
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
This commit is contained in:
armando-migliaccio 2014-07-01 15:08:11 -07:00
parent b5cd4c7cef
commit fc01ddaaa5
2 changed files with 23 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
neutron/agent/l3_agent.py
View File

@ -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"))

16
neutron/tests/unit/test_l3_agent.py
View File

@ -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,