From b0306a93645ea6475a2bc045d8fc8bc4bd6f00a5 Mon Sep 17 00:00:00 2001 From: Stephen Ma Date: Wed, 20 Nov 2013 14:52:23 -0800 Subject: [PATCH] Delete duplicate internal devices in router namespace When neutron router-interface-delete is ran during L3-agent restart, the agent may fail to delete the old internal device. After the restart, when the command "neutron router-interface-add " is ran again, the router ends up having two internal devices configured with the same IP address. Closes-Bug: #1244853 Change-Id: I0d7e2db6aa7dae26d0fc3fe1b1527762cb1e3b23 --- neutron/agent/l3_agent.py | 18 +++++++++ neutron/tests/unit/test_l3_agent.py | 59 +++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+) diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py index 01132cc7946..0c17d4ffa58 100644 --- a/neutron/agent/l3_agent.py +++ b/neutron/agent/l3_agent.py @@ -385,6 +385,13 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager): prefixlen = netaddr.IPNetwork(port['subnet']['cidr']).prefixlen port['ip_cidr'] = "%s/%s" % (ips[0]['ip_address'], prefixlen) + def _get_existing_internal_devices(self, ri): + ip_wrapper = ip_lib.IPWrapper(root_helper=self.root_helper, + namespace=ri.ns_name()) + ip_devs = ip_wrapper.get_devices(exclude_loopback=True) + return [ip_dev.name for ip_dev in ip_devs if + ip_dev.name.startswith(INTERNAL_DEV_PREFIX)] + def process_router(self, ri): ri.iptables_manager.defer_apply_on() ex_gw_port = self._get_ex_gw_port(ri) @@ -407,6 +414,17 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager): self.internal_network_removed(ri, p['id'], p['ip_cidr']) ri.internal_ports.remove(p) + current_internal_devs = set(self._get_existing_internal_devices(ri)) + current_port_devs = set([self.get_internal_device_name(id) for + id in current_port_ids]) + stale_devs = current_internal_devs - current_port_devs + for stale_dev in stale_devs: + LOG.debug(_('Deleting stale internal router device: %s'), + stale_dev) + self.driver.unplug(stale_dev, + namespace=ri.ns_name(), + prefix=INTERNAL_DEV_PREFIX) + internal_cidrs = [p['ip_cidr'] for p in ri.internal_ports] # TODO(salv-orlando): RouterInfo would be a better place for # this logic too diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index 6b81ac8e7c9..4c137dbc2cb 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -15,6 +15,7 @@ # License for the specific language governing permissions and limitations # under the License. +import contextlib import copy import mock @@ -727,6 +728,64 @@ class TestBasicRouterOperations(base.BaseTestCase): self.assertThat(nat_rules.index(jump_float_rule), matchers.LessThan(nat_rules.index(internal_net_rule))) + def test_process_router_delete_stale_internal_devices(self): + class FakeDev(object): + def __init__(self, name): + self.name = name + + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + stale_devlist = [FakeDev('qr-a1b2c3d4-e5'), + FakeDev('qr-b2c3d4e5-f6')] + stale_devnames = [dev.name for dev in stale_devlist] + + get_devices_return = [FakeDev('qg-a1b2c3d4-e5'), + FakeDev('qg-b2c3d4e5-f6')] + get_devices_return.extend(stale_devlist) + self.mock_ip.get_devices.return_value = get_devices_return + + router = self._prepare_router_data(enable_snat=True, + num_internal_ports=1) + ri = l3_agent.RouterInfo(router['id'], + self.conf.root_helper, + self.conf.use_namespaces, + router=router) + + internal_ports = ri.router.get(l3_constants.INTERFACE_KEY, []) + self.assertEqual(len(internal_ports), 1) + internal_port = internal_ports[0] + + with contextlib.nested(mock.patch.object(l3_agent.L3NATAgent, + 'internal_network_removed'), + mock.patch.object(l3_agent.L3NATAgent, + 'internal_network_added'), + mock.patch.object(l3_agent.L3NATAgent, + 'external_gateway_removed'), + mock.patch.object(l3_agent.L3NATAgent, + 'external_gateway_added') + ) as (internal_network_removed, + internal_network_added, + external_gateway_removed, + external_gateway_added): + + agent.process_router(ri) + + self.assertEqual(external_gateway_added.call_count, 1) + self.assertFalse(external_gateway_removed.called) + self.assertFalse(internal_network_removed.called) + internal_network_added.assert_called_once_with( + ri, + internal_port['network_id'], + internal_port['id'], + internal_port['ip_cidr'], + internal_port['mac_address']) + self.assertEqual(self.mock_driver.unplug.call_count, + len(stale_devnames)) + calls = [mock.call(stale_devname, + namespace=ri.ns_name(), + prefix=l3_agent.INTERNAL_DEV_PREFIX) + for stale_devname in stale_devnames] + self.mock_driver.unplug.assert_has_calls(calls, any_order=True) + def test_routers_with_admin_state_down(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) self.plugin_api.get_external_network_id.return_value = None