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

diff --git a/neutron/agent/l3_agent.py b/neutron/agent/l3_agent.py
index 7055853355fc01b00fa85867086df5dbb65a8896..dc308ca351f39b3fe1f45a209afd624796454c21 100644 (file)
--- 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 bbfa087491a6ca66a8911e9c67a79fbfd4804c95..afc27f788ec29429ec8e698c4d4218f8170e3689 100644 (file)
--- 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 6c10331123009a1fec1de9f5d72ac5cf2281f772..a1854cff8d30ef9ab8d43559c4c45276a547561e 100644 (file)
--- 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 202ae933c5408802c920e0fae1af6315fa4ce866..08dc98fb63570337dec580ca87df869f0c8694ae 100644 (file)
--- 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: