From 1b5730b42c6306e26ec5e803f3e349b203756ba8 Mon Sep 17 00:00:00 2001 From: Akihiro MOTOKI Date: Sun, 30 Dec 2012 05:37:13 +0900 Subject: [PATCH] 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 | 34 ++++++++- quantum/tests/unit/test_agent_ovs_cleanup.py | 80 +++++++++++++++----- 2 files changed, 93 insertions(+), 21 deletions(-) diff --git a/quantum/agent/ovs_cleanup_util.py b/quantum/agent/ovs_cleanup_util.py index a744c16db..b49b252cd 100644 --- a/quantum/agent/ovs_cleanup_util.py +++ b/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")) diff --git a/quantum/tests/unit/test_agent_ovs_cleanup.py b/quantum/tests/unit/test_agent_ovs_cleanup.py index 646485722..1f3556103 100644 --- a/quantum/tests/unit/test_agent_ovs_cleanup.py +++ b/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'): - br_patch = mock.patch('quantum.agent.linux.ovs_lib.get_bridges') - with br_patch as mock_get_bridges: - mock_get_bridges.return_value = ['br-int', 'br-ex'] - with mock.patch( - 'quantum.agent.linux.ovs_lib.OVSBridge') as ovs: - setup_conf = mock.patch( - 'quantum.agent.ovs_cleanup_util.setup_conf') - with setup_conf as mock_setup_conf: - conf = mock.Mock() - confroot_helper = 'sudo' - conf.ovs_all_ports = False - conf.ovs_integration_bridge = 'br-int' - conf.external_network_bridge = 'br-ex' - - mock_setup_conf.return_value = conf - - util.main() - ovs.assert_has_calls([mock.call().delete_ports( - all_ports=False)]) + bridges = ['br-int', 'br-ex'] + ports = ['p1', 'p2', 'p3'] + conf = mock.Mock() + conf.AGENT.root_helper = 'dummy_sudo' + conf.ovs_all_ports = False + conf.ovs_integration_bridge = 'br-int' + conf.external_network_bridge = 'br-ex' + with contextlib.nested( + mock.patch('quantum.common.config.setup_logging'), + mock.patch('quantum.agent.ovs_cleanup_util.setup_conf', + return_value=conf), + mock.patch('quantum.agent.linux.ovs_lib.get_bridges', + return_value=bridges), + 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') + + 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) + + def test_delete_quantum_ports(self): + ports = ['tap1234', 'tap5678', 'tap09ab'] + 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()]) -- 2.45.2