openstack
/
neutron-vpnaas
Code Issues Proposed changes

Refactor _process_routers to handle a single router 

The method _process_routers no longer handles multiple routers.  The
only caller of this method would construct a list of exactly one
router in order to make the call.  This made the for loop unnecessary.
The method's logic is too heavy for its current purpose.  This commit
removes much of the weight.

The use of the sets in this method is also no longer necessary.  It
became clear that all of it boiled down to "if the router is not
compatible with with this agent but it is known in router_info from
before then we need to remove it."  This is an exceptional condition
that shouldn't be handled in this method so I raise an exception and
handle it in process_router_update where other router removal is
handled.  Logging was added for this exceptional condition.

The eventlet pool was also obsolete.  It was used to spawn two methods
and there was a waitall at the end.  The other refactoring made it
clear that the two spawns were mutually exclusive.  There was only one
thread spawned for any given invocation of the method and the eventlet
pool is overkill.

Change-Id: Ibeac591b08565d10b2a9730e25a54f2cd11fc2bc
Closes-Bug: #1378398
This commit is contained in:
Carl Baldwin 2014-10-02 20:35:21 +00:00
parent a6af571e01
commit 29c7d39a65
2 changed files with 11 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
neutron/services/vpn/agent.py
View File

@ -130,15 +130,15 @@ class VPNAgent(l3_agent.L3NATAgentWithStateReport):
for device in self.devices:
device.destroy_router(router_id)
def _process_routers(self, routers):
def _process_router_if_compatible(self, router):
"""Router sync event.
This method overwrites parent class method.
:param routers: list of routers
:param router: a router
"""
super(VPNAgent, self)._process_routers(routers)
super(VPNAgent, self)._process_router_if_compatible(router)
for device in self.devices:
device.sync(self.context, routers)
device.sync(self.context, [router])
def main():

15
neutron/tests/unit/services/vpn/test_vpn_agent.py
View File

@ -183,15 +183,14 @@ class TestVPNAgent(base.BaseTestCase):
self.agent._router_removed(router_id)
device.destroy_router.assert_called_once_with(router_id)
def test_process_routers(self):
def test_process_router_if_compatible(self):
self.plugin_api.get_external_network_id.return_value = None
routers = [
{'id': _uuid(),
'admin_state_up': True,
'routes': [],
'external_gateway_info': {}}]
router = {'id': _uuid(),
'admin_state_up': True,
'routes': [],
'external_gateway_info': {}}
device = mock.Mock()
self.agent.devices = [device]
self.agent._process_routers(routers)
device.sync.assert_called_once_with(mock.ANY, routers)
self.agent._process_router_if_compatible(router)
device.sync.assert_called_once_with(mock.ANY, [router])