openstack
/
neutron
Code Issues Proposed changes

l3_agent: make process_router more robust 

If internal_network_added/removed fails, _sync_routers_task will call
process_router to do fault recovery. Because the port is already
added/removed to/from ri.internal_ports, internal_network_added or
internal_network_removed will not be called again.

The patch fix this issue by calling ri.internal_ports.append/removed
only if internal_network_added/removed succeed. Without the patch,
the added testcases would fail.

Change-Id: I2d2e004caa670c1624257c1d7ccc900438b42d08
Co-Authored-By: Hirofumi Ichihara <ichihara.hirofumi@lab.ntt.co.jp>
Closes-Bug: #1255871
This commit is contained in:
Jian Wen 2013-11-27 22:23:26 +08:00
parent 771f4faeb6
commit dd71021347
2 changed files with 57 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
neutron/agent/l3_agent.py
View File

@ -391,13 +391,13 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager):
p['id'] not in current_port_ids]
for p in new_ports:
self._set_subnet_info(p)
ri.internal_ports.append(p)
self.internal_network_added(ri, p['network_id'], p['id'],
p['ip_cidr'], p['mac_address'])
ri.internal_ports.append(p)
for p in old_ports:
ri.internal_ports.remove(p)
self.internal_network_removed(ri, p['id'], p['ip_cidr'])
ri.internal_ports.remove(p)
internal_cidrs = [p['ip_cidr'] for p in ri.internal_ports]
# TODO(salv-orlando): RouterInfo would be a better place for

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

@ -574,6 +574,61 @@ class TestBasicRouterOperations(base.BaseTestCase):
self._verify_snat_rules(nat_rules_delta, router, negate=True)
self.send_arp.assert_called_once()
def test_process_router_internal_network_added_unexpected_error(self):
agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
router = self._prepare_router_data()
ri = l3_agent.RouterInfo(router['id'], self.conf.root_helper,
self.conf.use_namespaces, router=router)
with mock.patch.object(
l3_agent.L3NATAgent,
'internal_network_added') as internal_network_added:
# raise RuntimeError to simulate that an unexpected exception
# occurrs
internal_network_added.side_effect = RuntimeError
self.assertRaises(RuntimeError, agent.process_router, ri)
self.assertNotIn(
router[l3_constants.INTERFACE_KEY][0], ri.internal_ports)
# The unexpected exception has been fixed manually
internal_network_added.side_effect = None
# _sync_routers_task finds out that _rpc_loop failed to process the
# router last time, it will retry in the next run.
agent.process_router(ri)
# We were able to add the port to ri.internal_ports
self.assertIn(
router[l3_constants.INTERFACE_KEY][0], ri.internal_ports)
def test_process_router_internal_network_removed_unexpected_error(self):
agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
router = self._prepare_router_data()
ri = l3_agent.RouterInfo(router['id'], self.conf.root_helper,
self.conf.use_namespaces, router=router)
# add an internal port
agent.process_router(ri)
with mock.patch.object(
l3_agent.L3NATAgent,
'internal_network_removed') as internal_net_removed:
# raise RuntimeError to simulate that an unexpected exception
# occurrs
internal_net_removed.side_effect = RuntimeError
ri.internal_ports[0]['admin_state_up'] = False
# The above port is set to down state, remove it.
self.assertRaises(RuntimeError, agent.process_router, ri)
self.assertIn(
router[l3_constants.INTERFACE_KEY][0], ri.internal_ports)
# The unexpected exception has been fixed manually
internal_net_removed.side_effect = None
# _sync_routers_task finds out that _rpc_loop failed to process the
# router last time, it will retry in the next run.
agent.process_router(ri)
# We were able to remove the port from ri.internal_ports
self.assertNotIn(
router[l3_constants.INTERFACE_KEY][0], ri.internal_ports)
def test_handle_router_snat_rules_add_back_jump(self):
agent = l3_agent.L3NATAgent(HOSTNAME, self.conf)
ri = mock.MagicMock()