OVS cleanup utility removes veth pairs

Fixes bug 1094185

OVSInterfaceDriver plug() creates a veth pair and add it to an OVS
bridge when ovs_use_veth=True. It is useful that OVS cleanup utility
removes such veth paris in addition to removing OVS ports.

Veth pairs to be removed are veth devices corresponding to ports
removed when all_ovs_port==False. Veth pairs created to connect
the integration bridge and physical bridges are not removed.

Change-Id: Ia87f0464e69a45dd0bdfd907ac71a003df9fc23f

quantum/agent/ovs_cleanup_util.py

@@ -17,6 +17,7 @@
 
 from quantum.agent import l3_agent
 from quantum.agent.linux import interface
+from quantum.agent.linux import ip_lib
 from quantum.agent.linux import ovs_lib
 from quantum.common import config
 from quantum.openstack.common import cfg
@@ -54,6 +55,27 @@ def setup_conf():
     return conf
 
 
+def collect_quantum_ports(bridges, root_helper):
+    """Collect ports created by Quantum from OVS"""
+    ports = []
+    for bridge in bridges:
+        ovs = ovs_lib.OVSBridge(bridge, root_helper)
+        ports += [port.port_name for port in ovs.get_vif_ports()]
+    return ports
+
+
+def delete_quantum_ports(ports, root_helper):
+    """Delete non-internal ports created by Quantum
+
+    Non-internal OVS ports need to be removed manually.
+    """
+    for port in ports:
+        if ip_lib.device_exists(port):
+            device = ip_lib.IPDevice(port, root_helper)
+            device.link.delete()
+            LOG.info(_("Delete %s"), port)
+
+
 def main():
     """Main method for cleaning up OVS bridges.
 
@@ -66,15 +88,25 @@ def main():
     configuration_bridges = set([conf.ovs_integration_bridge,
                                  conf.external_network_bridge])
     ovs_bridges = set(ovs_lib.get_bridges(conf.AGENT.root_helper))
+    available_configuration_bridges = configuration_bridges & ovs_bridges
 
     if conf.ovs_all_ports:
         bridges = ovs_bridges
     else:
-        bridges = configuration_bridges & ovs_bridges
+        bridges = available_configuration_bridges
+
+    # Collect existing ports created by Quantum on configuration bridges.
+    # After deleting ports from OVS bridges, we cannot determine which
+    # ports were created by Quantum, so port information is collected now.
+    ports = collect_quantum_ports(available_configuration_bridges,
+                                  conf.AGENT.root_helper)
 
     for bridge in bridges:
         LOG.info(_("Cleaning %s"), bridge)
         ovs = ovs_lib.OVSBridge(bridge, conf.AGENT.root_helper)
         ovs.delete_ports(all_ports=conf.ovs_all_ports)
 
+    # Remove remaining ports created by Quantum (usually veth pair)
+    delete_quantum_ports(ports, conf.AGENT.root_helper)
+
     LOG.info(_("OVS cleanup completed successfully"))

quantum/tests/unit/test_agent_ovs_cleanup.py

@@ -15,10 +15,15 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import contextlib
+import itertools
 import mock
 import unittest2 as unittest
 
+from quantum.agent.linux import ip_lib
+from quantum.agent.linux import ovs_lib
 from quantum.agent import ovs_cleanup_util as util
+from quantum.openstack.common import uuidutils
 
 
 class TestOVSCleanup(unittest.TestCase):
@@ -31,23 +36,58 @@ class TestOVSCleanup(unittest.TestCase):
             self.assertEqual(conf.AGENT.root_helper, 'sudo')
 
     def test_main(self):
-        with mock.patch('quantum.common.config.setup_logging'):
+        bridges = ['br-int', 'br-ex']
-            br_patch = mock.patch('quantum.agent.linux.ovs_lib.get_bridges')
+        ports = ['p1', 'p2', 'p3']
-            with br_patch as mock_get_bridges:
+        conf = mock.Mock()
-                mock_get_bridges.return_value = ['br-int', 'br-ex']
+        conf.AGENT.root_helper = 'dummy_sudo'
-                with mock.patch(
+        conf.ovs_all_ports = False
-                    'quantum.agent.linux.ovs_lib.OVSBridge') as ovs:
+        conf.ovs_integration_bridge = 'br-int'
-                    setup_conf = mock.patch(
+        conf.external_network_bridge = 'br-ex'
-                        'quantum.agent.ovs_cleanup_util.setup_conf')
+        with contextlib.nested(
-                    with setup_conf as mock_setup_conf:
+            mock.patch('quantum.common.config.setup_logging'),
-                        conf = mock.Mock()
+            mock.patch('quantum.agent.ovs_cleanup_util.setup_conf',
-                        confroot_helper = 'sudo'
+                       return_value=conf),
-                        conf.ovs_all_ports = False
+            mock.patch('quantum.agent.linux.ovs_lib.get_bridges',
-                        conf.ovs_integration_bridge = 'br-int'
+                       return_value=bridges),
-                        conf.external_network_bridge = 'br-ex'
+            mock.patch('quantum.agent.linux.ovs_lib.OVSBridge'),
+            mock.patch.object(util, 'collect_quantum_ports',
+                              return_value=ports),
+            mock.patch.object(util, 'delete_quantum_ports')
+        ) as (_log, _conf, _get, ovs, collect, delete):
+            util.main()
+            ovs.assert_has_calls([mock.call().delete_ports(all_ports=False)])
+            collect.assert_called_once_with(set(bridges), 'dummy_sudo')
+            delete.assert_called_once_with(ports, 'dummy_sudo')
 
-                        mock_setup_conf.return_value = conf
+    def test_collect_quantum_ports(self):
+        port1 = ovs_lib.VifPort('tap1234', 1, uuidutils.generate_uuid(),
+                                '11:22:33:44:55:66', 'br')
+        port2 = ovs_lib.VifPort('tap5678', 2, uuidutils.generate_uuid(),
+                                '77:88:99:aa:bb:cc', 'br')
+        port3 = ovs_lib.VifPort('tap90ab', 3, uuidutils.generate_uuid(),
+                                '99:00:aa:bb:cc:dd', 'br')
+        ports = [[port1, port2], [port3]]
+        portnames = [p.port_name for p in itertools.chain(*ports)]
+        with mock.patch('quantum.agent.linux.ovs_lib.OVSBridge') as ovs:
+            ovs.return_value.get_vif_ports.side_effect = ports
+            bridges = ['br-int', 'br-ex']
+            ret = util.collect_quantum_ports(bridges, 'dummy_sudo')
+            self.assertEqual(ret, portnames)
 
-                        util.main()
+    def test_delete_quantum_ports(self):
-                        ovs.assert_has_calls([mock.call().delete_ports(
+        ports = ['tap1234', 'tap5678', 'tap09ab']
-                            all_ports=False)])
+        port_found = [True, False, True]
+        delete_ports = [p for p, found
+                        in itertools.izip(ports, port_found) if found]
+        with contextlib.nested(
+            mock.patch.object(ip_lib, 'device_exists',
+                              side_effect=port_found),
+            mock.patch.object(ip_lib, 'IPDevice')
+        ) as (device_exists, ip_dev):
+            util.delete_quantum_ports(ports, 'dummy_sudo')
+            device_exists.assert_has_calls([mock.call(p) for p in ports])
+            ip_dev.assert_has_calls(
+                [mock.call('tap1234', 'dummy_sudo'),
+                 mock.call().link.delete(),
+                 mock.call('tap09ab', 'dummy_sudo'),
+                 mock.call().link.delete()])