openstack/neutron
Code Issues Proposed changes

More efficiently clean up OVS ports

Browse Source 
Previously, running neutron_ovs_cleanup on an installation with
5000 ports would time out even after setting the timeout to 3
hours. The code would do a bridge and port "by name" lookup for
every port due to being based off the ovs-vsctl implementation
where names are how everything is looked up. With this change,
the same test runs in ~1.5 mins.

This implementation adds a new OVSDB command that just looks up
the bridge, iterates over its ports, and deletes the ones that
should be deleted in a single transaction per bridge.

Change-Id: I23c81813654596d61d8d930e0bfb0f016f91bc46
(cherry picked from commit fef374131b)
This commit is contained in:
Terry Wilson 2018-01-20 01:51:43 +00:00
parent c038d67377
commit a82b5f1e75
4 changed files with 103 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
neutron/agent/ovsdb/impl_idl.py
View File

@ -14,13 +14,16 @@
from debtcollector import moves
from oslo_config import cfg
from ovsdbapp.backend.ovs_idl import command
from ovsdbapp.backend.ovs_idl import connection
from ovsdbapp.backend.ovs_idl import idlutils
from ovsdbapp.backend.ovs_idl import transaction
from ovsdbapp.backend.ovs_idl import vlog
from ovsdbapp.schema.open_vswitch import impl_idl
from neutron.agent.ovsdb.native import connection as n_connection
from neutron.conf.agent import ovs_conf
from neutron.plugins.ml2.drivers.openvswitch.agent.common import constants
NeutronOVSDBTransaction = moves.moved_class(
impl_idl.OvsVsctlTransaction,
@ -54,7 +57,39 @@ def api_factory(context):
return NeutronOvsdbIdl(_connection)
class OvsCleanup(command.BaseCommand):
def __init__(self, api, bridge, all_ports=False):
super(OvsCleanup, self).__init__(api)
self.bridge = bridge
self.all_ports = all_ports
def run_idl(self, txn):
br = idlutils.row_by_value(self.api.idl, 'Bridge', 'name', self.bridge)
for port in br.ports:
if not any(self.is_deletable_port(iface)
for iface in port.interfaces):
continue
br.delvalue('ports', port)
for iface in port.interfaces:
iface.delete()
port.delete()
def is_deletable_port(self, port):
# Deletable defined as "looks like vif port and not set to skip delete"
if self.all_ports:
return True
if constants.SKIP_CLEANUP in port.external_ids:
return False
if not all(field in port.external_ids
for field in ('iface-id', 'attached-mac')):
return False
return True
class NeutronOvsdbIdl(impl_idl.OvsdbIdl):
def __init__(self, connection):
vlog.use_python_logger()
super(NeutronOvsdbIdl, self).__init__(connection)
def ovs_cleanup(self, bridges, all_ports=False):
return OvsCleanup(self, bridges, all_ports)

41
neutron/cmd/ovs_cleanup.py
View File

@ -84,7 +84,10 @@ def main():
conf = setup_conf()
conf()
config.setup_logging()
do_main(conf)
def do_main(conf):
configuration_bridges = set([conf.ovs_integration_bridge,
conf.external_network_bridge])
ovs = ovs_lib.BaseOVS()
@ -96,22 +99,28 @@ def main():
else:
bridges = available_configuration_bridges
# Collect existing ports created by Neutron on configuration bridges.
# After deleting ports from OVS bridges, we cannot determine which
# ports were created by Neutron, so port information is collected now.
ports = collect_neutron_ports(available_configuration_bridges)
try:
# The ovs_cleanup method not added to the deprecated vsctl backend
for bridge in bridges:
LOG.info("Cleaning bridge: %s", bridge)
ovs.ovsdb.ovs_cleanup(bridge,
conf.ovs_all_ports).execute(check_error=True)
except AttributeError:
# Collect existing ports created by Neutron on configuration bridges.
# After deleting ports from OVS bridges, we cannot determine which
# ports were created by Neutron, so port information is collected now.
ports = collect_neutron_ports(available_configuration_bridges)
for bridge in bridges:
LOG.info("Cleaning bridge: %s", bridge)
ovs = ovs_lib.OVSBridge(bridge)
if conf.ovs_all_ports:
port_names = ovs.get_port_name_list()
else:
port_names = get_bridge_deletable_ports(ovs)
for port_name in port_names:
ovs.delete_port(port_name)
# Remove remaining ports created by Neutron (usually veth pair)
delete_neutron_ports(ports)
for bridge in bridges:
LOG.info("Cleaning bridge: %s", bridge)
ovs = ovs_lib.OVSBridge(bridge)
if conf.ovs_all_ports:
port_names = ovs.get_port_name_list()
else:
port_names = get_bridge_deletable_ports(ovs)
for port_name in port_names:
ovs.delete_port(port_name)
# Remove remaining ports created by Neutron (usually veth pair)
delete_neutron_ports(ports)
LOG.info("OVS cleanup completed successfully")

45
neutron/tests/functional/cmd/test_ovs_cleanup.py
View File

@ -10,12 +10,17 @@
# License for the specific language governing permissions and limitations
# under the License.
import collections
from neutron.cmd import ovs_cleanup
from neutron.common import utils
from neutron.conf.agent import cmd
from neutron.tests import base
from neutron.plugins.ml2.drivers.openvswitch.agent.common import constants
from neutron.tests.common import net_helpers
from neutron.tests.functional.agent.linux import base
class TestOVSCLIConfig(base.BaseTestCase):
class TestOVSCLIConfig(base.BaseOVSLinuxTestCase):
def setup_config(self, args=None):
self.conf = ovs_cleanup.setup_conf()
@ -26,3 +31,39 @@ class TestOVSCLIConfig(base.BaseTestCase):
# to unregister opts
self.conf.reset()
self.conf.unregister_opts(cmd.ovs_opts)
def test_do_main_default_options(self):
int_br = self.useFixture(net_helpers.OVSBridgeFixture()).bridge
ext_br = self.useFixture(net_helpers.OVSBridgeFixture()).bridge
self.conf.set_override("ovs_integration_bridge", int_br.br_name)
self.conf.set_override("external_network_bridge", ext_br.br_name)
self.conf.set_override("ovs_all_ports", False)
noskip = collections.defaultdict(list)
skip = collections.defaultdict(list)
# add two vifs, one skipped, and a non-vif port to int_br and ext_br
for br in (int_br, ext_br):
for collection in (noskip, skip):
collection[br].append(
self.useFixture(net_helpers.OVSPortFixture(br)).port.name)
# set skippable vif to be skipped
br.ovsdb.db_set(
'Interface', skip[br][0],
('external_ids', {constants.SKIP_CLEANUP: "True"})
).execute(check_error=True)
device_name = utils.get_rand_name()
skip[br].append(device_name)
br.add_port(device_name, ('type', 'internal'))
# sanity check
for collection in (noskip, skip):
for bridge, ports in collection.items():
port_list = bridge.get_port_name_list()
for port in ports:
self.assertIn(port, port_list)
ovs_cleanup.do_main(self.conf)
for br in (int_br, ext_br):
ports = br.get_port_name_list()
for vif in noskip[br]:
self.assertNotIn(vif, ports)
for port in skip[br]:
self.assertIn(port, ports)

23
neutron/tests/unit/cmd/test_ovs_cleanup.py
View File

@ -26,29 +26,6 @@ from neutron.tests import base
class TestOVSCleanup(base.BaseTestCase):
@mock.patch('neutron.agent.ovsdb.impl_idl._connection')
@mock.patch('neutron.common.config.setup_logging')
@mock.patch('neutron.cmd.ovs_cleanup.setup_conf')
@mock.patch('neutron.agent.common.ovs_lib.BaseOVS.get_bridges')
@mock.patch('neutron.agent.common.ovs_lib.OVSBridge')
@mock.patch.object(util, 'collect_neutron_ports')
@mock.patch.object(util, 'delete_neutron_ports')
def test_main(self, mock_delete, mock_collect, mock_ovs,
mock_get_bridges, mock_conf, mock_logging, mock_conn):
bridges = ['br-int', 'br-ex']
ports = ['p1', 'p2', 'p3']
conf = mock.Mock()
conf.ovs_all_ports = False
conf.ovs_integration_bridge = 'br-int'
conf.external_network_bridge = 'br-ex'
mock_conf.return_value = conf
mock_get_bridges.return_value = bridges
mock_collect.return_value = ports
util.main()
mock_collect.assert_called_once_with(set(bridges))
mock_delete.assert_called_once_with(ports)
def test_collect_neutron_ports(self):
port1 = ovs_lib.VifPort('tap1234', 1, uuidutils.generate_uuid(),
'11:22:33:44:55:66', 'br')