Merge "Fixes an issue with FIP re-association"

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?)
+                ns_ip.del_veth(d.name)
-                pass
             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:

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)

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.mock_driver.unplug.assert_called_once_with('fg-aaaa',
-        self.assertEqual(self.mock_driver.unplug.call_count, 1)
+                                                        bridge='br-ex',
-        self.mock_driver.unplug.assert_called_with('fg-aaaa', bridge='br-ex',
+                                                        prefix='fg-',
-                                                   prefix='fg-',
+                                                        namespace='qrouter'
-                                                   namespace='qrouter-foo')
+                                                        '-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
+        with mock.patch.object(agent, '_destroy_fip_namespace') as f:
-        # TODO(mrsmith): test agent_fip_count == 0 case
+            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):

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: