From fc24c6c221e51a7cfba2cf499dd89c34667b8859 Mon Sep 17 00:00:00 2001 From: Michael Smith Date: Wed, 6 Aug 2014 15:02:35 -0700 Subject: [PATCH] Fixes an issue with FIP re-association When the last FIP is disassociated, the namespace and interfaces should be removed. The internal interface wasn't removed before without problems, but now the namespace cannot be removed with that interface present. The fix is to remove the internal FIP interface before removing the namespace. Change-Id: I021c658ecde584821f67b7a8de0205e8e938bb2d Closes-bug: 1353287 --- neutron/agent/l3_agent.py | 8 ++-- neutron/agent/linux/ip_lib.py | 4 ++ neutron/tests/unit/test_l3_agent.py | 49 ++++++++++++++++++++----- neutron/tests/unit/test_linux_ip_lib.py | 6 +++ 4 files changed, 55 insertions(+), 12 deletions(-) diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py index 7055853355..dc308ca351 100644 --- a/neutron/agent/l3_agent.py +++ b/neutron/agent/l3_agent.py @@ -539,8 +539,7 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager): for d in ns_ip.get_devices(exclude_loopback=True): if d.name.startswith(FIP_2_ROUTER_DEV_PREFIX): # internal link between IRs and FIP NS - # TODO(mrsmith): remove IR interfaces (IP pool?) - pass + ns_ip.del_veth(d.name) elif d.name.startswith(FIP_EXT_DEV_PREFIX): # single port from FIP NS to br-ext # TODO(mrsmith): remove br-ext interface @@ -562,6 +561,8 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager): # device is on default bridge self.driver.unplug(d.name, namespace=ns, prefix=INTERNAL_DEV_PREFIX) + elif d.name.startswith(ROUTER_2_FIP_DEV_PREFIX): + ns_ip.del_veth(d.name) elif d.name.startswith(EXTERNAL_DEV_PREFIX): self.driver.unplug(d.name, bridge=self.conf.external_network_bridge, @@ -1443,12 +1444,13 @@ class L3NATAgent(firewall_l3_agent.FWaaSL3AgentRpcCallback, manager.Manager): #remove default route entry device = ip_lib.IPDevice(rtr_2_fip_name, self.root_helper, namespace=ri.ns_name) + ns_ip = ip_lib.IPWrapper(self.root_helper, namespace=fip_ns_name) device.route.delete_gateway(ri.fip_2_rtr, table=FIP_RT_TBL) self.local_ips.add(ri.rtr_2_fip.rsplit('.', 1)[1]) ri.rtr_2_fip = None self.local_ips.add(ri.fip_2_rtr.rsplit('.', 1)[1]) ri.fip_2_rtr = None - # TODO(mrsmith): remove interface + ns_ip.del_veth(fip_2_rtr_name) # clean up fip-namespace if this is the last FIP self.agent_fip_count = self.agent_fip_count - 1 if self.agent_fip_count == 0: diff --git a/neutron/agent/linux/ip_lib.py b/neutron/agent/linux/ip_lib.py index bbfa087491..afc27f788e 100644 --- a/neutron/agent/linux/ip_lib.py +++ b/neutron/agent/linux/ip_lib.py @@ -130,6 +130,10 @@ class IPWrapper(SubProcessBase): return (IPDevice(name1, self.root_helper, self.namespace), IPDevice(name2, self.root_helper, namespace2)) + def del_veth(self, name): + """Delete a virtual interface between two namespaces.""" + self._as_root('', 'link', ('del', name)) + def ensure_namespace(self, name): if not self.netns.exists(name): ip = self.netns.add(name) diff --git a/neutron/tests/unit/test_l3_agent.py b/neutron/tests/unit/test_l3_agent.py index 6c10331123..a1854cff8d 100644 --- a/neutron/tests/unit/test_l3_agent.py +++ b/neutron/tests/unit/test_l3_agent.py @@ -1456,17 +1456,38 @@ class TestBasicRouterOperations(base.BaseTestCase): namespaces = ['qrouter-foo', 'qrouter-bar'] self.mock_ip.get_namespaces.return_value = namespaces - self.mock_ip.get_devices.return_value = [FakeDev('fr-aaaa'), + self.mock_ip.get_devices.return_value = [FakeDev('fpr-aaaa'), FakeDev('fg-aaaa')] agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) agent._destroy_fip_namespace(namespaces[0]) - # TODO(mrsmith): update for fr interface - self.assertEqual(self.mock_driver.unplug.call_count, 1) - self.mock_driver.unplug.assert_called_with('fg-aaaa', bridge='br-ex', - prefix='fg-', - namespace='qrouter-foo') + self.mock_driver.unplug.assert_called_once_with('fg-aaaa', + bridge='br-ex', + prefix='fg-', + namespace='qrouter' + '-foo') + self.mock_ip.del_veth.assert_called_once_with('fpr-aaaa') + + def test_destroy_namespace(self): + class FakeDev(object): + def __init__(self, name): + self.name = name + + namespace = 'qrouter-bar' + + self.mock_ip.get_namespaces.return_value = [namespace] + self.mock_ip.get_devices.return_value = [FakeDev('qr-aaaa'), + FakeDev('rfp-aaaa')] + + agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) + + agent._destroy_namespace(namespace) + self.mock_driver.unplug.assert_called_once_with('qr-aaaa', + prefix='qr-', + namespace='qrouter' + '-bar') + self.mock_ip.del_veth.assert_called_once_with('rfp-aaaa') def test_destroy_router_namespace_skips_ns_removal(self): agent = l3_agent.L3NATAgent(HOSTNAME, self.conf) @@ -1866,20 +1887,30 @@ class TestBasicRouterOperations(base.BaseTestCase): 'network_id': _uuid(), 'mac_address': 'ca:fe:de:ad:be:ef', 'ip_cidr': '20.0.0.30/24'} - fip_cidr = '11.22.33.44/24' ri = l3_agent.RouterInfo(router['id'], self.conf.root_helper, self.conf.use_namespaces, router=router) ri.dist_fip_count = 2 ri.floating_ips_dict['11.22.33.44'] = FIP_PRI + ri.fip_2_rtr = '11.22.33.42' + ri.rtr_2_fip = '11.22.33.40' agent.agent_gateway_port = agent_gw_port agent.floating_ip_removed_dist(ri, fip_cidr) self.mock_rule.delete_rule_priority.assert_called_with(FIP_PRI) self.mock_ip_dev.route.delete_route.assert_called_with(fip_cidr, ri.rtr_2_fip) - # TODO(mrsmith): test ri.dist_fip_count == 0 - # TODO(mrsmith): test agent_fip_count == 0 case + with mock.patch.object(agent, '_destroy_fip_namespace') as f: + ri.dist_fip_count = 1 + agent.agent_fip_count = 1 + fip_ns_name = agent.get_fip_ns_name( + str(agent._fetch_external_net_id())) + agent.floating_ip_removed_dist(ri, fip_cidr) + self.mock_ip.del_veth.assert_called_once_with( + agent.get_fip_int_device_name(router['id'])) + self.mock_ip_dev.route.delete_gateway.assert_called_once_with( + '11.22.33.42', table=16) + f.assert_called_once_with(fip_ns_name) class TestL3AgentEventHandler(base.BaseTestCase): diff --git a/neutron/tests/unit/test_linux_ip_lib.py b/neutron/tests/unit/test_linux_ip_lib.py index 202ae933c5..08dc98fb63 100644 --- a/neutron/tests/unit/test_linux_ip_lib.py +++ b/neutron/tests/unit/test_linux_ip_lib.py @@ -266,6 +266,12 @@ class TestIpWrapper(base.BaseTestCase): 'peer', 'name', 'tap1'), 'sudo', None) + def test_del_veth(self): + ip_lib.IPWrapper('sudo').del_veth('fpr-1234') + self.execute.assert_called_once_with('', 'link', + ('del', 'fpr-1234'), + 'sudo', None) + def test_add_veth_with_namespaces(self): ns2 = 'ns2' with mock.patch.object(ip_lib.IPWrapper, 'ensure_namespace') as en: