From cf37563c8393f964e7f390f13c43070791360cc1 Mon Sep 17 00:00:00 2001 From: Brian Haley Date: Wed, 23 May 2018 17:07:03 -0400 Subject: [PATCH] Remove deprecated vsctl ovsdb_interface api This was deprecated in https://review.openstack.org/#/c/503070/ so remove all the vsctl-related code, leaving just the native ovsdb api. Also removed renamed ovs_vsctl_timeout value, which was changed to ovsdb_timeout in https://review.openstack.org/#/c/518391/ Change-Id: I50dfcea3deb41df1bd01fd06b76522453a6ba50b --- neutron/agent/common/ovs_lib.py | 14 +- neutron/agent/ovsdb/api.py | 26 - neutron/agent/ovsdb/impl_idl.py | 2 +- neutron/agent/ovsdb/impl_vsctl.py | 345 ------------- neutron/agent/ovsdb/native/connection.py | 4 + neutron/cmd/ovs_cleanup.py | 61 +-- neutron/cmd/sanity_check.py | 3 - neutron/conf/agent/ovs_conf.py | 1 - neutron/conf/agent/ovsdb_api.py | 11 - .../openvswitch/agent/ovs_neutron_agent.py | 2 +- .../test_ovs_agent_qos_extension.py | 8 +- neutron/tests/functional/agent/linux/base.py | 15 +- .../tests/functional/agent/test_firewall.py | 7 +- .../tests/functional/agent/test_ovs_flows.py | 10 +- .../tests/functional/agent/test_ovs_lib.py | 3 +- .../services/logapi/test_logging.py | 2 - .../tests/unit/agent/common/test_ovs_lib.py | 487 +----------------- .../openvswitch_firewall/test_firewall.py | 2 +- neutron/tests/unit/cmd/test_ovs_cleanup.py | 50 +- ...ce-ovs-vsctl-timeout-a618ec8e27552202.yaml | 7 + 20 files changed, 53 insertions(+), 1007 deletions(-) delete mode 100644 neutron/agent/ovsdb/impl_vsctl.py create mode 100644 releasenotes/notes/removed-ovsdb_interface-ovs-vsctl-timeout-a618ec8e27552202.yaml diff --git a/neutron/agent/common/ovs_lib.py b/neutron/agent/common/ovs_lib.py index 0c2a911070c..1926fbb3238 100644 --- a/neutron/agent/common/ovs_lib.py +++ b/neutron/agent/common/ovs_lib.py @@ -30,7 +30,7 @@ import tenacity from neutron._i18n import _ from neutron.agent.common import ip_lib from neutron.agent.common import utils -from neutron.agent.ovsdb import api as ovsdb_api +from neutron.agent.ovsdb import impl_idl from neutron.common import constants as common_constants from neutron.common import utils as common_utils from neutron.conf.agent import ovs_conf @@ -70,15 +70,15 @@ CTRL_BURST_LIMIT_MIN = 25 def _ovsdb_result_pending(result): - """Return True if ovs-vsctl indicates the result is still pending.""" - # ovs-vsctl can return '[]' for an ofport that has not yet been assigned + """Return True if ovsdb indicates the result is still pending.""" + # ovsdb can return '[]' for an ofport that has not yet been assigned return result == [] def _ovsdb_retry(fn): """Decorator for retrying when OVS has yet to assign an ofport. - The instance's vsctl_timeout is used as the max waiting time. This relies + The instance's ovsdb_timeout is used as the max waiting time. This relies on the fact that instance methods receive self as the first argument. """ @six.wraps(fn) @@ -89,7 +89,7 @@ def _ovsdb_retry(fn): retry=tenacity.retry_if_result(_ovsdb_result_pending), wait=tenacity.wait_exponential(multiplier=0.01, max=1), stop=tenacity.stop_after_delay( - self.vsctl_timeout))(fn) + self.ovsdb_timeout))(fn) return new_fn(*args, **kwargs) return wrapped @@ -113,8 +113,8 @@ class VifPort(object): class BaseOVS(object): def __init__(self): - self.vsctl_timeout = cfg.CONF.OVS.ovsdb_timeout - self.ovsdb = ovsdb_api.from_config(self) + self.ovsdb_timeout = cfg.CONF.OVS.ovsdb_timeout + self.ovsdb = impl_idl.api_factory() def add_manager(self, connection_uri, timeout=_SENTINEL): """Have ovsdb-server listen for manager connections diff --git a/neutron/agent/ovsdb/api.py b/neutron/agent/ovsdb/api.py index d56eebc61ab..2c54cea4724 100644 --- a/neutron/agent/ovsdb/api.py +++ b/neutron/agent/ovsdb/api.py @@ -15,21 +15,6 @@ import collections import uuid -from oslo_config import cfg -from oslo_utils import importutils - -from neutron.conf.agent import ovsdb_api - - -ovsdb_api.register_ovsdb_api_opts() - - -def from_config(context, iface_name=None): - """Return the configured OVSDB API implementation""" - iface = importutils.import_module( - ovsdb_api.interface_map[iface_name or cfg.CONF.OVS.ovsdb_interface]) - return iface.api_factory(context) - def val_to_py(val): """Convert a json ovsdb return value to native python object""" @@ -41,14 +26,3 @@ def val_to_py(val): elif val[0] == "map": return {val_to_py(x): val_to_py(y) for x, y in val[1]} return val - - -def py_to_val(pyval): - """Convert python value to ovs-vsctl value argument""" - if isinstance(pyval, bool): - return 'true' if pyval is True else 'false' - elif pyval == '': - return '""' - else: - # NOTE(twilson) If a Command object, return its record_id as a value - return getattr(pyval, "record_id", pyval) diff --git a/neutron/agent/ovsdb/impl_idl.py b/neutron/agent/ovsdb/impl_idl.py index 50612251973..44242144b1f 100644 --- a/neutron/agent/ovsdb/impl_idl.py +++ b/neutron/agent/ovsdb/impl_idl.py @@ -42,7 +42,7 @@ ovs_conf.register_ovs_agent_opts() _connection = None -def api_factory(context): +def api_factory(): global _connection if _connection is None: _connection = connection.Connection( diff --git a/neutron/agent/ovsdb/impl_vsctl.py b/neutron/agent/ovsdb/impl_vsctl.py deleted file mode 100644 index 07bd212f90f..00000000000 --- a/neutron/agent/ovsdb/impl_vsctl.py +++ /dev/null @@ -1,345 +0,0 @@ -# Copyright (c) 2014 OpenStack Foundation -# -# Licensed under the Apache License, Version 2.0 (the "License"); you may -# not use this file except in compliance with the License. You may obtain -# a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT -# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the -# License for the specific language governing permissions and limitations -# under the License. - -import collections -import itertools -import uuid - -from oslo_log import log as logging -from oslo_serialization import jsonutils -from oslo_utils import excutils -from oslo_utils import uuidutils -from ovsdbapp import api as ovsdb_api -import six - -from neutron.agent.common import utils -from neutron.agent.ovsdb import api as ovsdb - -LOG = logging.getLogger(__name__) - - -def api_factory(context): - return OvsdbVsctl(context) - - -class Transaction(ovsdb_api.Transaction): - def __init__(self, context, check_error=False, log_errors=True, opts=None): - self.context = context - self.check_error = check_error - self.log_errors = log_errors - self.opts = ["--timeout=%d" % self.context.vsctl_timeout, - '--oneline', '--format=json'] - if opts: - self.opts += opts - self.commands = [] - - def add(self, command): - self.commands.append(command) - return command - - def commit(self): - args = [] - for cmd in self.commands: - cmd.result = None - args += cmd.vsctl_args() - res = self.run_vsctl(args) - if res is None: - return - res = res.replace(r'\\', '\\').splitlines() - for i, record in enumerate(res): - self.commands[i].result = record - return [cmd.result for cmd in self.commands] - - def run_vsctl(self, args): - full_args = ["ovs-vsctl"] + self.opts + args - try: - # We log our own errors, so never have utils.execute do it - return utils.execute(full_args, run_as_root=True, - log_fail_as_error=False).rstrip() - except Exception as e: - with excutils.save_and_reraise_exception() as ctxt: - if self.log_errors: - LOG.error("Unable to execute %(cmd)s. " - "Exception: %(exception)s", - {'cmd': full_args, 'exception': e}) - if not self.check_error: - ctxt.reraise = False - - -class BaseCommand(ovsdb_api.Command): - def __init__(self, context, cmd, opts=None, args=None): - self.context = context - self.cmd = cmd - self.opts = [] if opts is None else opts - self.args = [] if args is None else args - - def execute(self, check_error=False, log_errors=True): - with Transaction(self.context, check_error=check_error, - log_errors=log_errors) as txn: - txn.add(self) - return self.result - - def vsctl_args(self): - return itertools.chain(('--',), self.opts, (self.cmd,), self.args) - - -class MultiLineCommand(BaseCommand): - """Command for ovs-vsctl commands that return multiple lines""" - @property - def result(self): - return self._result - - @result.setter - def result(self, raw_result): - self._result = raw_result.split(r'\n') if raw_result else [] - - -class DbCommand(BaseCommand): - def __init__(self, context, cmd, opts=None, args=None, columns=None): - if opts is None: - opts = [] - if columns: - opts += ['--columns=%s' % ",".join(columns)] - super(DbCommand, self).__init__(context, cmd, opts, args) - - @property - def result(self): - return self._result - - @result.setter - def result(self, raw_result): - # If check_error=False, run_vsctl can return None - if not raw_result: - self._result = None - return - - try: - json = jsonutils.loads(raw_result) - except (ValueError, TypeError) as e: - # This shouldn't happen, but if it does and we check_errors - # log and raise. - with excutils.save_and_reraise_exception(): - LOG.error("Could not parse: %(raw_result)s. " - "Exception: %(exception)s", - {'raw_result': raw_result, 'exception': e}) - - headings = json['headings'] - data = json['data'] - results = [] - for record in data: - obj = {} - for pos, heading in enumerate(headings): - obj[heading] = ovsdb.val_to_py(record[pos]) - results.append(obj) - self._result = results - - -class DbGetCommand(DbCommand): - @DbCommand.result.setter - def result(self, val): - # super()'s never worked for setters http://bugs.python.org/issue14965 - DbCommand.result.fset(self, val) - # DbCommand will return [{'column': value}] and we just want value. - if self._result: - self._result = list(self._result[0].values())[0] - - -class DbCreateCommand(BaseCommand): - def __init__(self, context, opts=None, args=None): - super(DbCreateCommand, self).__init__(context, "create", opts, args) - # NOTE(twilson) pre-commit result used for intra-transaction reference - self.record_id = "@%s" % uuidutils.generate_uuid() - self.opts.append("--id=%s" % self.record_id) - - @property - def result(self): - return self._result - - @result.setter - def result(self, val): - self._result = uuid.UUID(val) if val else val - - -class BrExistsCommand(DbCommand): - @DbCommand.result.setter - def result(self, val): - self._result = val is not None - - def execute(self): - return super(BrExistsCommand, self).execute(check_error=False, - log_errors=False) - - -class OvsdbVsctl(ovsdb_api.API): - def __init__(self, context): - super(OvsdbVsctl, self).__init__() - self.context = context - - def create_transaction(self, check_error=False, log_errors=True, **kwargs): - return Transaction(self.context, check_error, log_errors, **kwargs) - - def add_manager(self, connection_uri): - # This will add a new manager without overriding existing ones. - conn_uri = 'target="%s"' % connection_uri - args = ['create', 'Manager', conn_uri, '--', 'add', 'Open_vSwitch', - '.', 'manager_options', '@manager'] - return BaseCommand(self.context, '--id=@manager', args=args) - - def get_manager(self): - return MultiLineCommand(self.context, 'get-manager') - - def remove_manager(self, connection_uri): - args = ['get', 'Manager', connection_uri, '--', 'remove', - 'Open_vSwitch', '.', 'manager_options', '@manager'] - return BaseCommand(self.context, '--id=@manager', args=args) - - def add_br(self, name, may_exist=True, datapath_type=None): - opts = ['--may-exist'] if may_exist else None - params = [name] - if datapath_type: - params += ['--', 'set', 'Bridge', name, - 'datapath_type=%s' % datapath_type] - return BaseCommand(self.context, 'add-br', opts, params) - - def del_br(self, name, if_exists=True): - opts = ['--if-exists'] if if_exists else None - return BaseCommand(self.context, 'del-br', opts, [name]) - - def br_exists(self, name): - return BrExistsCommand(self.context, 'list', args=['Bridge', name]) - - def port_to_br(self, name): - return BaseCommand(self.context, 'port-to-br', args=[name]) - - def iface_to_br(self, name): - return BaseCommand(self.context, 'iface-to-br', args=[name]) - - def list_br(self): - return MultiLineCommand(self.context, 'list-br') - - def br_get_external_id(self, name, field): - return BaseCommand(self.context, 'br-get-external-id', - args=[name, field]) - - def db_create(self, table, **col_values): - args = [table] - args += _set_colval_args(*col_values.items()) - return DbCreateCommand(self.context, args=args) - - def db_destroy(self, table, record): - args = [table, record] - return BaseCommand(self.context, 'destroy', args=args) - - def db_set(self, table, record, *col_values): - args = [table, record] - args += _set_colval_args(*col_values) - return BaseCommand(self.context, 'set', args=args) - - def db_add(self, table, record, column, *values): - args = [table, record, column] - for value in values: - if isinstance(value, collections.Mapping): - args += ["{}={}".format(ovsdb.py_to_val(k), ovsdb.py_to_val(v)) - for k, v in value.items()] - else: - args.append(ovsdb.py_to_val(value)) - return BaseCommand(self.context, 'add', args=args) - - def db_clear(self, table, record, column): - return BaseCommand(self.context, 'clear', args=[table, record, - column]) - - def db_get(self, table, record, column): - # Use the 'list' command as it can return json and 'get' cannot so that - # we can get real return types instead of treating everything as string - # NOTE: openvswitch can return a single atomic value for fields that - # are sets, but only have one value. This makes directly iterating over - # the result of a db_get() call unsafe. - return DbGetCommand(self.context, 'list', args=[table, record], - columns=[column]) - - def db_list(self, table, records=None, columns=None, if_exists=False): - opts = ['--if-exists'] if if_exists else None - args = [table] - if records: - args += records - return DbCommand(self.context, 'list', opts=opts, args=args, - columns=columns) - - def db_find(self, table, *conditions, **kwargs): - columns = kwargs.pop('columns', None) - args = itertools.chain([table], - *[_set_colval_args(c) for c in conditions]) - return DbCommand(self.context, 'find', args=args, columns=columns) - - def set_controller(self, bridge, controllers): - return BaseCommand(self.context, 'set-controller', - args=[bridge] + list(controllers)) - - def del_controller(self, bridge): - return BaseCommand(self.context, 'del-controller', args=[bridge]) - - def get_controller(self, bridge): - return MultiLineCommand(self.context, 'get-controller', args=[bridge]) - - def set_fail_mode(self, bridge, mode): - return BaseCommand(self.context, 'set-fail-mode', args=[bridge, mode]) - - def add_port(self, bridge, port, may_exist=True): - opts = ['--may-exist'] if may_exist else None - return BaseCommand(self.context, 'add-port', opts, [bridge, port]) - - def del_port(self, port, bridge=None, if_exists=True): - opts = ['--if-exists'] if if_exists else None - args = filter(None, [bridge, port]) - return BaseCommand(self.context, 'del-port', opts, args) - - def list_ports(self, bridge): - return MultiLineCommand(self.context, 'list-ports', args=[bridge]) - - def list_ifaces(self, bridge): - return MultiLineCommand(self.context, 'list-ifaces', args=[bridge]) - - def db_remove(self, table, record, column, *values, **keyvalues): - raise NotImplementedError() - - def db_find_rows(self, table, *conditions, **kwargs): - raise NotImplementedError() - - def db_list_rows(self, table, record=None, if_exists=False): - raise NotImplementedError() - - -def _set_colval_args(*col_values): - args = [] - # TODO(twilson) This is ugly, but set/find args are very similar except for - # op. Will try to find a better way to default this op to '=' - for entry in col_values: - if len(entry) == 2: - col, op, val = entry[0], '=', entry[1] - else: - col, op, val = entry - if isinstance(val, collections.Mapping): - args += ["%s:%s%s%s" % ( - col, k, op, ovsdb.py_to_val(v)) for k, v in val.items()] - elif (isinstance(val, collections.Sequence) and - not isinstance(val, six.string_types)): - if len(val) == 0: - args.append("%s%s%s" % (col, op, "[]")) - else: - args.append( - "%s%s%s" % (col, op, ",".join(map(ovsdb.py_to_val, val)))) - else: - args.append("%s%s%s" % (col, op, ovsdb.py_to_val(val))) - return args diff --git a/neutron/agent/ovsdb/native/connection.py b/neutron/agent/ovsdb/native/connection.py index 16fef16065e..e7677078b47 100644 --- a/neutron/agent/ovsdb/native/connection.py +++ b/neutron/agent/ovsdb/native/connection.py @@ -24,12 +24,16 @@ import tenacity from neutron.agent.ovsdb.native import exceptions as ovsdb_exc from neutron.agent.ovsdb.native import helpers +from neutron.conf.agent import ovsdb_api TransactionQueue = moves.moved_class(_connection.TransactionQueue, 'TransactionQueue', __name__) Connection = moves.moved_class(_connection.Connection, 'Connection', __name__) +ovsdb_api.register_ovsdb_api_opts() + + def configure_ssl_conn(): """Configures required settings for an SSL based OVSDB client connection diff --git a/neutron/cmd/ovs_cleanup.py b/neutron/cmd/ovs_cleanup.py index 9373a9033a6..da1d2c6cf3c 100644 --- a/neutron/cmd/ovs_cleanup.py +++ b/neutron/cmd/ovs_cleanup.py @@ -17,12 +17,10 @@ from oslo_config import cfg from oslo_log import log as logging from neutron.agent.common import ovs_lib -from neutron.agent.linux import ip_lib from neutron.common import config from neutron.conf.agent import cmd from neutron.conf.agent import common as agent_config from neutron.conf.agent.l3 import config as l3_config -from neutron.plugins.ml2.drivers.openvswitch.agent.common import constants LOG = logging.getLogger(__name__) @@ -47,38 +45,6 @@ def setup_conf(): return conf -def get_bridge_deletable_ports(br): - """Get bridge deletable ports - - Return a list of OVS Bridge ports, excluding the ports who should not be - cleaned. such ports are tagged with the 'skip_cleanup' key in external_ids. - """ - return [port.port_name for port in br.get_vif_ports() - if constants.SKIP_CLEANUP not in - br.get_port_external_ids(port.port_name)] - - -def collect_neutron_ports(bridges): - """Collect ports created by Neutron from OVS.""" - ports = [] - for bridge in bridges: - ovs = ovs_lib.OVSBridge(bridge) - ports += get_bridge_deletable_ports(ovs) - return ports - - -def delete_neutron_ports(ports): - """Delete non-internal ports created by Neutron - - Non-internal OVS ports need to be removed manually. - """ - for port in ports: - device = ip_lib.IPDevice(port) - if device.exists(): - device.link.delete() - LOG.info("Deleting port: %s", port) - - def main(): """Main method for cleaning up OVS bridges. @@ -103,28 +69,9 @@ def do_main(conf): else: bridges = 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.ovsdb.ovs_cleanup(bridge, + conf.ovs_all_ports).execute(check_error=True) LOG.info("OVS cleanup completed successfully") diff --git a/neutron/cmd/sanity_check.py b/neutron/cmd/sanity_check.py index 82020c549d2..9042e4d926f 100644 --- a/neutron/cmd/sanity_check.py +++ b/neutron/cmd/sanity_check.py @@ -193,7 +193,6 @@ def check_vf_extended_management(): def check_ovsdb_native(): - cfg.CONF.set_override('ovsdb_interface', 'native', group='OVS') result = checks.ovsdb_native_supported() if not result: LOG.error('Check for native OVSDB support failed.') @@ -365,8 +364,6 @@ def enable_tests_from_config(): cfg.CONF.set_default('arp_responder', True) if not cfg.CONF.AGENT.use_helper_for_ns_read: cfg.CONF.set_default('read_netns', True) - if cfg.CONF.OVS.ovsdb_interface == 'native': - cfg.CONF.set_default('ovsdb_native', True) if cfg.CONF.l3_ha: cfg.CONF.set_default('keepalived_ipv6_support', True) cfg.CONF.set_default('ip_nonlocal_bind', True) diff --git a/neutron/conf/agent/ovs_conf.py b/neutron/conf/agent/ovs_conf.py index f81d20fce4f..617ccfe1e3c 100644 --- a/neutron/conf/agent/ovs_conf.py +++ b/neutron/conf/agent/ovs_conf.py @@ -23,7 +23,6 @@ DEFAULT_OVSDB_TIMEOUT = 10 OPTS = [ cfg.IntOpt('ovsdb_timeout', default=DEFAULT_OVSDB_TIMEOUT, - deprecated_name='ovs_vsctl_timeout', deprecated_group='DEFAULT', help=_('Timeout in seconds for ovsdb commands. ' 'If the timeout expires, ovsdb commands will fail with ' 'ALARMCLOCK error.')), diff --git a/neutron/conf/agent/ovsdb_api.py b/neutron/conf/agent/ovsdb_api.py index 14f6ba2a9a8..3f524fe31c5 100644 --- a/neutron/conf/agent/ovsdb_api.py +++ b/neutron/conf/agent/ovsdb_api.py @@ -17,18 +17,7 @@ from neutron._i18n import _ from oslo_config import cfg -interface_map = { - 'vsctl': 'neutron.agent.ovsdb.impl_vsctl', - 'native': 'neutron.agent.ovsdb.impl_idl', -} - - API_OPTS = [ - cfg.StrOpt('ovsdb_interface', - deprecated_for_removal=True, - choices=interface_map.keys(), - default='native', - help=_('The interface for interacting with the OVSDB')), cfg.StrOpt('ovsdb_connection', default='tcp:127.0.0.1:6640', help=_('The connection string for the OVSDB backend. ' diff --git a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py index 22b5706a085..47b4ed943c8 100644 --- a/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/ml2/drivers/openvswitch/agent/ovs_neutron_agent.py @@ -1021,7 +1021,7 @@ class OVSNeutronAgent(l2population_rpc.L2populationRpcCallBackTunnelMixin, ''' # Ensure the integration bridge is created. - # ovs_lib.OVSBridge.create() will run + # ovs_lib.OVSBridge.create() will run the equivalent of # ovs-vsctl -- --may-exist add-br BRIDGE_NAME # which does nothing if bridge already exists. self.int_br.create() diff --git a/neutron/tests/functional/agent/l2/extensions/test_ovs_agent_qos_extension.py b/neutron/tests/functional/agent/l2/extensions/test_ovs_agent_qos_extension.py index 3840f2a4e72..72981c0c85c 100644 --- a/neutron/tests/functional/agent/l2/extensions/test_ovs_agent_qos_extension.py +++ b/neutron/tests/functional/agent/l2/extensions/test_ovs_agent_qos_extension.py @@ -27,7 +27,6 @@ from neutron.objects.qos import policy from neutron.objects.qos import rule from neutron.tests.common.agents import l2_extensions from neutron.tests.functional.agent.l2 import base -from neutron.tests.functional.agent.linux import base as linux_base load_tests = testscenarios.load_tests_apply_scenarios @@ -179,16 +178,11 @@ class OVSAgentQoSExtensionTestFramework(base.OVSAgentTestFramework): class TestOVSAgentQosExtension(OVSAgentQoSExtensionTestFramework): - interface_scenarios = linux_base.BaseOVSLinuxTestCase.scenarios - - direction_scenarios = [ + scenarios = [ ('ingress', {'direction': constants.INGRESS_DIRECTION}), ('egress', {'direction': constants.EGRESS_DIRECTION}) ] - scenarios = testscenarios.multiply_scenarios( - interface_scenarios, direction_scenarios) - def setUp(self): super(TestOVSAgentQosExtension, self).setUp() self.test_bw_limit_rule_1.direction = self.direction diff --git a/neutron/tests/functional/agent/linux/base.py b/neutron/tests/functional/agent/linux/base.py index 9b520a52366..47cfa030c9b 100644 --- a/neutron/tests/functional/agent/linux/base.py +++ b/neutron/tests/functional/agent/linux/base.py @@ -12,8 +12,6 @@ # License for the specific language governing permissions and limitations # under the License. -import testscenarios - from neutron.tests.common.exclusive_resources import ip_address from neutron.tests.functional import base @@ -26,18 +24,7 @@ MARKED_BLOCK_RULE = '-m mark --mark %s -j DROP' % MARK_VALUE ICMP_BLOCK_RULE = '-p icmp -j DROP' -# Regarding MRO, it goes BaseOVSLinuxTestCase, WithScenarios, -# BaseSudoTestCase, ..., UnitTest, object. setUp is not defined in -# WithScenarios, so it will correctly be found in BaseSudoTestCase. -class BaseOVSLinuxTestCase(testscenarios.WithScenarios, base.BaseSudoTestCase): - scenarios = [ - ('vsctl', dict(ovsdb_interface='vsctl')), - ('native', dict(ovsdb_interface='native')), - ] - - def setUp(self): - super(BaseOVSLinuxTestCase, self).setUp() - self.config(group='OVS', ovsdb_interface=self.ovsdb_interface) +class BaseOVSLinuxTestCase(base.BaseSudoTestCase): def get_test_net_address(self, block): """Return exclusive address based on RFC 5737. diff --git a/neutron/tests/functional/agent/test_firewall.py b/neutron/tests/functional/agent/test_firewall.py index 7d3e9b25aab..f3e9168405c 100644 --- a/neutron/tests/functional/agent/test_firewall.py +++ b/neutron/tests/functional/agent/test_firewall.py @@ -36,7 +36,6 @@ from neutron.conf.agent import securitygroups_rpc as security_config from neutron.tests.common import conn_testers from neutron.tests.common import helpers from neutron.tests.functional.agent.linux import base as linux_base -from neutron.tests.functional import base from neutron.tests.functional import constants as test_constants LOG = logging.getLogger(__name__) @@ -76,7 +75,7 @@ def _add_rule(sg_rules, base, port_range_min=None, port_range_max=None): sg_rules.append(rule) -class BaseFirewallTestCase(base.BaseSudoTestCase): +class BaseFirewallTestCase(linux_base.BaseOVSLinuxTestCase): FAKE_SECURITY_GROUP_ID = uuidutils.generate_uuid() MAC_SPOOFED = "fa:16:3e:9a:2f:48" scenarios_iptables = testscenarios.multiply_scenarios( @@ -87,8 +86,7 @@ class BaseFirewallTestCase(base.BaseSudoTestCase): scenarios_ovs_fw_interfaces = testscenarios.multiply_scenarios( [('OVS Firewall Driver', {'initialize': 'initialize_ovs', - 'firewall_name': 'openvswitch'})], - linux_base.BaseOVSLinuxTestCase.scenarios) + 'firewall_name': 'openvswitch'})]) scenarios = scenarios_iptables + scenarios_ovs_fw_interfaces @@ -126,7 +124,6 @@ class BaseFirewallTestCase(base.BaseSudoTestCase): return tester, firewall_drv def initialize_ovs(self): - self.config(group='OVS', ovsdb_interface=self.ovsdb_interface) # Tests for ovs requires kernel >= 4.3 and OVS >= 2.5 if not checks.ovs_conntrack_supported(): self.skipTest("Open vSwitch with conntrack is not installed " diff --git a/neutron/tests/functional/agent/test_ovs_flows.py b/neutron/tests/functional/agent/test_ovs_flows.py index 86fb92f34c1..fd136c78c48 100644 --- a/neutron/tests/functional/agent/test_ovs_flows.py +++ b/neutron/tests/functional/agent/test_ovs_flows.py @@ -36,9 +36,9 @@ from neutron.tests.common import base as common_base from neutron.tests.common import helpers from neutron.tests.common import net_helpers from neutron.tests.functional.agent import test_ovs_lib -from neutron.tests.functional import base from neutron.tests import tools +load_tests = testscenarios.load_tests_apply_scenarios OVS_TRACE_FINAL_FLOW = 'Final flow' OVS_TRACE_DATAPATH_ACTIONS = 'Datapath actions' @@ -47,14 +47,12 @@ cfg.CONF.import_group('OVS', 'neutron.plugins.ml2.drivers.openvswitch.agent.' 'common.config') -class OVSAgentTestBase(test_ovs_lib.OVSBridgeTestBase, - base.BaseSudoTestCase): - scenarios = testscenarios.multiply_scenarios([ +class OVSAgentTestBase(test_ovs_lib.OVSBridgeTestBase): + scenarios = [ ('ofctl', {'main_module': ('neutron.plugins.ml2.drivers.openvswitch.' 'agent.openflow.ovs_ofctl.main')}), ('native', {'main_module': ('neutron.plugins.ml2.drivers.openvswitch.' - 'agent.openflow.native.main')})], - test_ovs_lib.OVSBridgeTestBase.scenarios) + 'agent.openflow.native.main')})] def setUp(self): super(OVSAgentTestBase, self).setUp() diff --git a/neutron/tests/functional/agent/test_ovs_lib.py b/neutron/tests/functional/agent/test_ovs_lib.py index 331c3e8677f..7aee4af65b1 100644 --- a/neutron/tests/functional/agent/test_ovs_lib.py +++ b/neutron/tests/functional/agent/test_ovs_lib.py @@ -528,7 +528,6 @@ class OVSBridgeTestCase(OVSBridgeTestBase): if_exists=False)) txn.add(ovsdb.db_set('Interface', port_name, ('type', 'internal'))) - # native gives a more specific exception than vsctl self.assertRaises((RuntimeError, idlutils.RowNotFound), del_port_mod_iface) @@ -583,7 +582,7 @@ class OVSLibTestCase(base.BaseOVSLinuxTestCase): self.assertIn(conn_uri, self.ovs.get_manager()) self.assertEqual(self.ovs.db_get_val('Manager', conn_uri, 'inactivity_probe'), - self.ovs.vsctl_timeout * 1000) + self.ovs.ovsdb_timeout * 1000) self.ovs.remove_manager(conn_uri) self.assertNotIn(conn_uri, self.ovs.get_manager()) diff --git a/neutron/tests/functional/services/logapi/test_logging.py b/neutron/tests/functional/services/logapi/test_logging.py index 438adca36f7..1b6494485d3 100644 --- a/neutron/tests/functional/services/logapi/test_logging.py +++ b/neutron/tests/functional/services/logapi/test_logging.py @@ -63,8 +63,6 @@ class LoggingExtensionTestFramework(test_firewall.BaseFirewallTestCase): def initialize_ovs_fw_log(self): mock.patch('ryu.base.app_manager.AppManager.get_instance').start() - mock.patch( - 'neutron.agent.ovsdb.impl_vsctl.OvsdbVsctl.transaction').start() agent_api = ovs_ext_api.OVSAgentExtensionAPI( ovs_bridge.OVSAgentBridge(self.tester.bridge.br_name), ovs_bridge.OVSAgentBridge('br-tun')) diff --git a/neutron/tests/unit/agent/common/test_ovs_lib.py b/neutron/tests/unit/agent/common/test_ovs_lib.py index 371cc850d99..b14a20a13de 100644 --- a/neutron/tests/unit/agent/common/test_ovs_lib.py +++ b/neutron/tests/unit/agent/common/test_ovs_lib.py @@ -15,7 +15,6 @@ import collections import mock -from neutron_lib import constants from neutron_lib import exceptions from oslo_serialization import jsonutils from oslo_utils import uuidutils @@ -24,23 +23,9 @@ import testtools from neutron.agent.common import ovs_lib from neutron.agent.common import utils -from neutron.conf.agent import common as config from neutron.plugins.ml2.drivers.openvswitch.agent.common \ import constants as p_const from neutron.tests import base -from neutron.tests import tools - - -OVS_LINUX_KERN_VERS_WITHOUT_VXLAN = "3.12.0" - -# some test data for get_vif_port_to_ofport_map that exhibited bug 1444269 -OVSLIST_WITH_UNSET_PORT = ( - '{"data":[["patch-tun",["map",[]],1],["tap2ab72a72-44",["map",[["attached-' - 'mac","fa:16:3e:b0:f8:38"],["iface-id","2ab72a72-4407-4ef3-806a-b2172f3e4d' - 'c7"],["iface-status","active"]]],2],["tap6b108774-15",["map",[["attached-' - 'mac","fa:16:3e:02:f5:91"],["iface-id","6b108774-1559-45e9-a7c3-b714f11722' - 'cf"],["iface-status","active"]]],["set",[]]]],"headings":["name","externa' - 'l_ids","ofport"]}') class OFCTLParamListMatcher(object): @@ -84,16 +69,6 @@ class StringSetMatcher(object): return '' % (self.set, sep) -def vsctl_only(f): - # NOTE(ivasilevskaya) as long as some tests rely heavily on mocking - # direct vsctl commands, need to ensure that ovsdb_interface = 'vsctl' - # TODO(ivasilevskaya) introduce alternative tests for native interface? - def wrapper(*args, **kwargs): - config.cfg.CONF.set_override("ovsdb_interface", "vsctl", group="OVS") - return f(*args, **kwargs) - return wrapper - - class OVS_Lib_Test(base.BaseTestCase): """A test suite to exercise the OVS libraries shared by Neutron agents. @@ -101,33 +76,16 @@ class OVS_Lib_Test(base.BaseTestCase): can run on any system. That does, however, limit their scope. """ - @vsctl_only def setUp(self): super(OVS_Lib_Test, self).setUp() self.BR_NAME = "br-int" + # Don't attempt to connect to ovsdb + mock.patch('neutron.agent.ovsdb.impl_idl.api_factory').start() self.br = ovs_lib.OVSBridge(self.BR_NAME) self.execute = mock.patch.object( utils, "execute", spec=utils.execute).start() - @property - def TO(self): - return "--timeout=%s" % self.br.vsctl_timeout - - def _vsctl_args(self, *args): - cmd = ['ovs-vsctl', self.TO, '--oneline', '--format=json', '--'] - cmd += args - return cmd - - def _vsctl_mock(self, *args): - cmd = self._vsctl_args(*args) - return mock.call(cmd, run_as_root=True, log_fail_as_error=False) - - def _verify_vsctl_mock(self, *args): - cmd = self._vsctl_args(*args) - self.execute.assert_called_once_with(cmd, run_as_root=True, - log_fail_as_error=False) - def test_vifport(self): """Create and stringify vif port, confirm no exceptions.""" @@ -147,9 +105,6 @@ class OVS_Lib_Test(base.BaseTestCase): # test __str__ str(port) - def _build_timeout_opt(self, exp_timeout): - return "--timeout=%d" % exp_timeout if exp_timeout else self.TO - def test_add_flow(self): ofport = "99" vid = 4000 @@ -276,32 +231,6 @@ class OVS_Lib_Test(base.BaseTestCase): process_input="hard_timeout=0,idle_timeout=0,priority=1," "cookie=1234,actions=normal") - def _test_get_port_ofport(self, ofport, expected_result): - pname = "tap99" - self.br.vsctl_timeout = 0 # Don't waste precious time retrying - self.execute.return_value = self._encode_ovs_json( - ['ofport'], [[ofport]]) - self.assertEqual(self.br.get_port_ofport(pname), expected_result) - self._verify_vsctl_mock("--columns=ofport", "list", "Interface", pname) - - def test_get_port_ofport_succeeds_for_valid_ofport(self): - self._test_get_port_ofport(6, 6) - - def test_get_port_ofport_returns_invalid_ofport_for_non_int(self): - self._test_get_port_ofport([], ovs_lib.INVALID_OFPORT) - - def test_get_port_ofport_returns_invalid_for_invalid(self): - self._test_get_port_ofport(ovs_lib.INVALID_OFPORT, - ovs_lib.INVALID_OFPORT) - - def test_get_port_mac(self): - pname = "tap99" - self.br.vsctl_timeout = 0 # Don't waste precious time retrying - self.execute.return_value = self._encode_ovs_json( - ['mac_in_use'], [['00:01:02:03:04:05']]) - expected_result = '00:01:02:03:04:05' - self.assertEqual(self.br.get_port_mac(pname), expected_result) - def test_default_datapath(self): # verify kernel datapath is default expected = p_const.OVS_DATAPATH_SYSTEM @@ -444,37 +373,6 @@ class OVS_Lib_Test(base.BaseTestCase): self.br.mod_flow, **params) - def test_ofctl_of_version_use_highest(self): - self.br.add_flow(in_port=1, actions="drop") - self.execute.assert_has_calls([ - mock.call(['ovs-ofctl', 'add-flows', '-O', p_const.OPENFLOW10, - mock.ANY, '-'], process_input=mock.ANY, - run_as_root=mock.ANY) - ]) - self.br.use_at_least_protocol(p_const.OPENFLOW12) - self.execute.reset_mock() - self.br.add_flow(in_port=1, actions="drop") - self.execute.assert_has_calls([ - mock.call(['ovs-ofctl', 'add-flows', '-O', p_const.OPENFLOW12, - mock.ANY, '-'], process_input=mock.ANY, - run_as_root=mock.ANY), - ]) - - def test_ofctl_of_version_keep_highest(self): - self.br.use_at_least_protocol(p_const.OPENFLOW13) - self.br.use_at_least_protocol(p_const.OPENFLOW12) - self.execute.reset_mock() - self.br.add_flow(in_port=1, actions="drop") - self.execute.assert_has_calls([ - mock.call(['ovs-ofctl', 'add-flows', '-O', p_const.OPENFLOW13, - mock.ANY, '-'], process_input=mock.ANY, - run_as_root=mock.ANY), - ]) - - def test_ofctl_of_version_use_unknown(self): - with testtools.ExpectedException(Exception): - self.br.use_at_least_protocol("OpenFlow42") - def test_run_ofctl_retry_on_socket_error(self): err = RuntimeError('failed to connect to socket') self.execute.side_effect = [err] * 5 @@ -490,134 +388,6 @@ class OVS_Lib_Test(base.BaseTestCase): self.assertEqual(0, sleep.call_count) self.assertEqual(1, self.execute.call_count) - def test_add_tunnel_port(self): - pname = "tap99" - local_ip = "1.1.1.1" - remote_ip = "9.9.9.9" - ofport = 6 - command = ["--may-exist", "add-port", - self.BR_NAME, pname] - command.extend(["--", "set", "Interface", pname]) - command.extend(["type=gre", "options:df_default=true", - "options:remote_ip=" + remote_ip, - "options:local_ip=" + local_ip, - "options:in_key=flow", - "options:out_key=flow"]) - # Each element is a tuple of (expected mock call, return_value) - expected_calls_and_values = [ - (self._vsctl_mock(*command), None), - (self._vsctl_mock("--columns=ofport", "list", "Interface", pname), - self._encode_ovs_json(['ofport'], [[ofport]])), - ] - tools.setup_mock_calls(self.execute, expected_calls_and_values) - - self.assertEqual( - self.br.add_tunnel_port(pname, remote_ip, local_ip), - ofport) - - tools.verify_mock_calls(self.execute, expected_calls_and_values) - - def test_add_vxlan_fragmented_tunnel_port(self): - pname = "tap99" - local_ip = "1.1.1.1" - remote_ip = "9.9.9.9" - ofport = 6 - vxlan_udp_port = "9999" - dont_fragment = False - command = ["--may-exist", "add-port", self.BR_NAME, pname] - command.extend(["--", "set", "Interface", pname]) - command.extend(["type=" + constants.TYPE_VXLAN, - "options:dst_port=" + vxlan_udp_port, - "options:df_default=false", - "options:remote_ip=" + remote_ip, - "options:local_ip=" + local_ip, - "options:in_key=flow", - "options:out_key=flow"]) - # Each element is a tuple of (expected mock call, return_value) - expected_calls_and_values = [ - (self._vsctl_mock(*command), None), - (self._vsctl_mock("--columns=ofport", "list", "Interface", pname), - self._encode_ovs_json(['ofport'], [[ofport]])), - ] - tools.setup_mock_calls(self.execute, expected_calls_and_values) - - self.assertEqual( - self.br.add_tunnel_port(pname, remote_ip, local_ip, - constants.TYPE_VXLAN, vxlan_udp_port, - dont_fragment), - ofport) - - tools.verify_mock_calls(self.execute, expected_calls_and_values) - - def test_add_vxlan_csum_tunnel_port(self): - pname = "tap99" - local_ip = "1.1.1.1" - remote_ip = "9.9.9.9" - ofport = 6 - vxlan_udp_port = "9999" - dont_fragment = True - tunnel_csum = True - command = ["--may-exist", "add-port", self.BR_NAME, pname] - command.extend(["--", "set", "Interface", pname]) - command.extend(["type=" + constants.TYPE_VXLAN, - "options:dst_port=" + vxlan_udp_port, - "options:df_default=true", - "options:remote_ip=" + remote_ip, - "options:local_ip=" + local_ip, - "options:in_key=flow", - "options:out_key=flow", - "options:csum=true"]) - # Each element is a tuple of (expected mock call, return_value) - expected_calls_and_values = [ - (self._vsctl_mock(*command), None), - (self._vsctl_mock("--columns=ofport", "list", "Interface", pname), - self._encode_ovs_json(['ofport'], [[ofport]])), - ] - tools.setup_mock_calls(self.execute, expected_calls_and_values) - - self.assertEqual( - self.br.add_tunnel_port(pname, remote_ip, local_ip, - constants.TYPE_VXLAN, vxlan_udp_port, - dont_fragment, tunnel_csum), - ofport) - - tools.verify_mock_calls(self.execute, expected_calls_and_values) - - def test_add_vxlan_tos_tunnel_port(self): - pname = "tap99" - local_ip = "1.1.1.1" - remote_ip = "9.9.9.9" - ofport = 6 - vxlan_udp_port = "9999" - dont_fragment = True - tunnel_csum = False - tos = 8 - command = ["--may-exist", "add-port", self.BR_NAME, pname] - command.extend(["--", "set", "Interface", pname]) - command.extend(["type=" + constants.TYPE_VXLAN, - "options:dst_port=" + vxlan_udp_port, - "options:df_default=true", - "options:remote_ip=" + remote_ip, - "options:local_ip=" + local_ip, - "options:in_key=flow", - "options:out_key=flow", - "options:tos=" + str(tos)]) - # Each element is a tuple of (expected mock call, return_value) - expected_calls_and_values = [ - (self._vsctl_mock(*command), None), - (self._vsctl_mock("--columns=ofport", "list", "Interface", pname), - self._encode_ovs_json(['ofport'], [[ofport]])), - ] - tools.setup_mock_calls(self.execute, expected_calls_and_values) - - self.assertEqual( - self.br.add_tunnel_port(pname, remote_ip, local_ip, - constants.TYPE_VXLAN, vxlan_udp_port, - dont_fragment, tunnel_csum, tos), - ofport) - - tools.verify_mock_calls(self.execute, expected_calls_and_values) - def _encode_ovs_json(self, headings, data): # See man ovs-vsctl(8) for the encoding details. r = {"data": [], @@ -637,12 +407,6 @@ class OVS_Lib_Test(base.BaseTestCase): type(cell)) return jsonutils.dumps(r) - def test_get_vif_port_to_ofport_map(self): - self.execute.return_value = OVSLIST_WITH_UNSET_PORT - results = self.br.get_vif_port_to_ofport_map() - expected = {'2ab72a72-4407-4ef3-806a-b2172f3e4dc7': 2, 'patch-tun': 1} - self.assertEqual(expected, results) - def test_get_vif_ports(self): pname = "tap99" ofport = 6 @@ -665,120 +429,6 @@ class OVS_Lib_Test(base.BaseTestCase): columns=['name', 'external_ids', 'ofport'], if_exists=True) - def test_get_vif_port_set(self): - id_key = 'iface-id' - headings = ['name', 'external_ids', 'ofport'] - data = [ - # A vif port on this bridge: - ['tap99', {id_key: 'tap99id', 'attached-mac': 'tap99mac'}, 1], - # A vif port on this bridge not yet configured - ['tap98', {id_key: 'tap98id', 'attached-mac': 'tap98mac'}, []], - # Another vif port on this bridge not yet configured - ['tap97', {id_key: 'tap97id', 'attached-mac': 'tap97mac'}, - ['set', []]], - - # Non-vif port on this bridge: - ['bogus', {}, 2], - ] - - # Each element is a tuple of (expected mock call, return_value) - expected_calls_and_values = [ - (self._vsctl_mock("list-ports", self.BR_NAME), 'tap99\\ntun22'), - (self._vsctl_mock("--if-exists", - "--columns=name,external_ids,ofport", - "list", "Interface", 'tap99', 'tun22'), - self._encode_ovs_json(headings, data)), - ] - tools.setup_mock_calls(self.execute, expected_calls_and_values) - - port_set = self.br.get_vif_port_set() - self.assertEqual(set(['tap99id']), port_set) - tools.verify_mock_calls(self.execute, expected_calls_and_values) - - def test_get_vif_ports_list_ports_error(self): - expected_calls_and_values = [ - (self._vsctl_mock("list-ports", self.BR_NAME), RuntimeError()), - ] - tools.setup_mock_calls(self.execute, expected_calls_and_values) - self.assertRaises(RuntimeError, self.br.get_vif_ports) - tools.verify_mock_calls(self.execute, expected_calls_and_values) - - def test_get_vif_port_set_list_ports_error(self): - expected_calls_and_values = [ - (self._vsctl_mock("list-ports", self.BR_NAME), RuntimeError()), - ] - tools.setup_mock_calls(self.execute, expected_calls_and_values) - self.assertRaises(RuntimeError, self.br.get_vif_port_set) - tools.verify_mock_calls(self.execute, expected_calls_and_values) - - def test_get_vif_port_set_list_interface_error(self): - expected_calls_and_values = [ - (self._vsctl_mock("list-ports", self.BR_NAME), 'tap99\n'), - (self._vsctl_mock("--if-exists", - "--columns=name,external_ids,ofport", - "list", "Interface", "tap99"), RuntimeError()), - ] - tools.setup_mock_calls(self.execute, expected_calls_and_values) - self.assertRaises(RuntimeError, self.br.get_vif_port_set) - tools.verify_mock_calls(self.execute, expected_calls_and_values) - - def test_get_port_tag_dict(self): - headings = ['name', 'tag'] - data = [ - ['int-br-eth2', set()], - ['patch-tun', set()], - ['qr-76d9e6b6-21', 1], - ['tapce5318ff-78', 1], - ['tape1400310-e6', 1], - ] - - # Each element is a tuple of (expected mock call, return_value) - expected_calls_and_values = [ - (self._vsctl_mock("list-ports", self.BR_NAME), - '\\n'.join((iface for iface, tag in data))), - (self._vsctl_mock("--columns=name,tag", "list", "Port"), - self._encode_ovs_json(headings, data)), - ] - tools.setup_mock_calls(self.execute, expected_calls_and_values) - - port_tags = self.br.get_port_tag_dict() - self.assertEqual( - port_tags, - {u'int-br-eth2': [], - u'patch-tun': [], - u'qr-76d9e6b6-21': 1, - u'tapce5318ff-78': 1, - u'tape1400310-e6': 1} - ) - - def test_clear_db_attribute(self): - pname = "tap77" - self.br.clear_db_attribute("Port", pname, "tag") - self._verify_vsctl_mock("clear", "Port", pname, "tag") - - def _test_iface_to_br(self, exp_timeout=None): - iface = 'tap0' - br = 'br-int' - if exp_timeout: - self.br.vsctl_timeout = exp_timeout - self.execute.return_value = 'br-int' - self.assertEqual(self.br.get_bridge_for_iface(iface), br) - self._verify_vsctl_mock("iface-to-br", iface) - - def test_iface_to_br(self): - self._test_iface_to_br() - - def test_iface_to_br_non_default_timeout(self): - new_timeout = 5 - self._test_iface_to_br(new_timeout) - - def test_iface_to_br_handles_ovs_vsctl_exception(self): - iface = 'tap0' - self.execute.side_effect = Exception - - self.assertIsNone(self.br.get_bridge_for_iface(iface)) - self._verify_vsctl_mock("iface-to-br", iface) - def test_delete_all_ports(self): with mock.patch.object(self.br, 'get_port_name_list', return_value=['port1']) as get_port: @@ -802,21 +452,6 @@ class OVS_Lib_Test(base.BaseTestCase): mock.call('tap5678') ]) - def test_delete_neutron_ports_list_error(self): - expected_calls_and_values = [ - (self._vsctl_mock("list-ports", self.BR_NAME), RuntimeError()), - ] - tools.setup_mock_calls(self.execute, expected_calls_and_values) - self.assertRaises(RuntimeError, self.br.delete_ports, all_ports=False) - tools.verify_mock_calls(self.execute, expected_calls_and_values) - - def test_get_bridges_not_default_timeout(self): - bridges = ['br-int', 'br-ex'] - self.br.vsctl_timeout = 5 - self.execute.return_value = 'br-int\\nbr-ex\n' - self.assertEqual(self.br.get_bridges(), bridges) - self._verify_vsctl_mock("list-br") - def test_get_local_port_mac_succeeds(self): with mock.patch('neutron.agent.linux.ip_lib.IpLinkCommand', return_value=mock.Mock(address='foo')): @@ -875,88 +510,6 @@ class OVS_Lib_Test(base.BaseTestCase): [mock.call('Interface', columns=['name', 'external_ids', 'ofport'], if_exists=True)]) - def _test_get_vif_port_by_id(self, iface_id, data, br_name=None, - extra_calls_and_values=None): - headings = ['external_ids', 'name', 'ofport'] - - # Each element is a tuple of (expected mock call, return_value) - expected_calls_and_values = [ - (self._vsctl_mock("--columns=external_ids,name,ofport", "find", - "Interface", - 'external_ids:iface-id=%s' % iface_id, - 'external_ids:attached-mac!=""'), - self._encode_ovs_json(headings, data))] - if data: - if not br_name: - br_name = self.BR_NAME - - # Only the last information list in 'data' is used, so if more - # than one vif is described in data, the rest must be declared - # in the argument 'expected_calls_and_values'. - if extra_calls_and_values: - expected_calls_and_values.extend(extra_calls_and_values) - - expected_calls_and_values.append( - (self._vsctl_mock("iface-to-br", - data[-1][headings.index('name')]), br_name)) - tools.setup_mock_calls(self.execute, expected_calls_and_values) - vif_port = self.br.get_vif_port_by_id(iface_id) - - tools.verify_mock_calls(self.execute, expected_calls_and_values) - return vif_port - - def _assert_vif_port(self, vif_port, ofport=None, mac=None): - if not ofport or ofport == -1 or not mac: - self.assertIsNone(vif_port, "Got %s" % vif_port) - return - self.assertEqual('tap99id', vif_port.vif_id) - self.assertEqual(mac, vif_port.vif_mac) - self.assertEqual('tap99', vif_port.port_name) - self.assertEqual(ofport, vif_port.ofport) - - def _test_get_vif_port_by_id_with_data(self, ofport=None, mac=None): - external_ids = [["iface-id", "tap99id"], - ["iface-status", "active"], - ["attached-mac", mac]] - data = [[["map", external_ids], "tap99", - ofport if ofport else ["set", []]]] - vif_port = self._test_get_vif_port_by_id('tap99id', data) - self._assert_vif_port(vif_port, ofport, mac) - - def test_get_vif_by_port_id_with_ofport(self): - self._test_get_vif_port_by_id_with_data( - ofport=1, mac="aa:bb:cc:dd:ee:ff") - - def test_get_vif_by_port_id_without_ofport(self): - self._test_get_vif_port_by_id_with_data(mac="aa:bb:cc:dd:ee:ff") - - def test_get_vif_by_port_id_with_invalid_ofport(self): - self._test_get_vif_port_by_id_with_data( - ofport=-1, mac="aa:bb:cc:dd:ee:ff") - - def test_get_vif_by_port_id_with_no_data(self): - self.assertIsNone(self._test_get_vif_port_by_id('whatever', [])) - - def test_get_vif_by_port_id_different_bridge(self): - external_ids = [["iface-id", "tap99id"], - ["iface-status", "active"]] - data = [[["map", external_ids], "tap99", 1]] - self.assertIsNone(self._test_get_vif_port_by_id('tap99id', data, - "br-ext")) - - def test_get_vif_by_port_id_multiple_vifs(self): - external_ids = [["iface-id", "tap99id"], - ["iface-status", "active"], - ["attached-mac", "de:ad:be:ef:13:37"]] - data = [[["map", external_ids], "dummytap", 1], - [["map", external_ids], "tap99", 1337]] - extra_calls_and_values = [ - (self._vsctl_mock("iface-to-br", "dummytap"), "br-ext")] - - vif_port = self._test_get_vif_port_by_id( - 'tap99id', data, extra_calls_and_values=extra_calls_and_values) - self._assert_vif_port(vif_port, ofport=1337, mac="de:ad:be:ef:13:37") - def test_get_port_ofport_retry(self): with mock.patch.object( self.br, 'db_get_val', @@ -965,7 +518,7 @@ class OVS_Lib_Test(base.BaseTestCase): def test_get_port_ofport_retry_fails(self): # reduce timeout for faster execution - self.br.vsctl_timeout = 1 + self.br.ovsdb_timeout = 1 # after 7 calls the retry will timeout and raise with mock.patch.object( self.br, 'db_get_val', @@ -985,7 +538,7 @@ class OVS_Lib_Test(base.BaseTestCase): def test_get_port_external_ids_retry_fails(self): # reduce timeout for faster execution - self.br.vsctl_timeout = 1 + self.br.ovsdb_timeout = 1 # after 7 calls the retry will timeout and raise with mock.patch.object( self.br, 'db_get_val', @@ -1141,35 +694,3 @@ class TestDeferredOVSBridge(base.BaseTestCase): def test_getattr_unallowed_attr_failure(self): with ovs_lib.DeferredOVSBridge(self.br) as deferred_br: self.assertRaises(AttributeError, getattr, deferred_br, 'failure') - - @vsctl_only - def test_default_cookie(self): - self.br = ovs_lib.OVSBridge("br-tun") - uuid_stamp1 = self.br.default_cookie - self.assertEqual(uuid_stamp1, self.br.default_cookie) - - @vsctl_only - def test_cookie_passed_to_addmod(self): - self.br = ovs_lib.OVSBridge("br-tun") - stamp = str(self.br.default_cookie) - expected_calls = [ - mock.call('add-flows', ['-'], - 'hard_timeout=0,idle_timeout=0,priority=1,' - 'cookie=' + stamp + ',actions=drop'), - mock.call('mod-flows', ['-'], - 'cookie=' + stamp + ',actions=drop') - ] - with mock.patch.object(self.br, 'run_ofctl') as f: - with ovs_lib.DeferredOVSBridge(self.br) as deferred_br: - deferred_br.add_flow(actions='drop') - deferred_br.mod_flow(actions='drop') - f.assert_has_calls(expected_calls) - - @vsctl_only - def test_add_flow_with_bundle(self): - br = ovs_lib.OVSBridge("foo") - deferred = br.deferred(use_bundle=True) - with mock.patch.object(utils, "execute", spec=utils.execute) as mexec: - deferred.add_flow(in_port=1, actions='drop') - deferred.apply_flows() - self.assertIn('--bundle', mexec.call_args[0][0]) diff --git a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py index 0685ae82def..d1476e89d8a 100644 --- a/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py +++ b/neutron/tests/unit/agent/linux/openvswitch_firewall/test_firewall.py @@ -734,7 +734,7 @@ class TestCookieContext(base.BaseTestCase): def setUp(self): super(TestCookieContext, self).setUp() # Don't attempt to connect to ovsdb - mock.patch('neutron.agent.ovsdb.api.from_config').start() + mock.patch('neutron.agent.ovsdb.impl_idl.api_factory').start() # Don't trigger iptables -> ovsfw migration mock.patch( 'neutron.agent.linux.openvswitch_firewall.iptables.Helper').start() diff --git a/neutron/tests/unit/cmd/test_ovs_cleanup.py b/neutron/tests/unit/cmd/test_ovs_cleanup.py index 3e2557521d6..7038ae8396a 100644 --- a/neutron/tests/unit/cmd/test_ovs_cleanup.py +++ b/neutron/tests/unit/cmd/test_ovs_cleanup.py @@ -13,47 +13,27 @@ # License for the specific language governing permissions and limitations # under the License. -import itertools - import mock -from oslo_utils import uuidutils -from neutron.agent.common import ovs_lib -from neutron.agent.linux import ip_lib from neutron.cmd import ovs_cleanup as util from neutron.tests import base class TestOVSCleanup(base.BaseTestCase): - def test_collect_neutron_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('neutron.agent.common.ovs_lib.OVSBridge') as ovs: - ovs.return_value.get_vif_ports.side_effect = ports - bridges = ['br-int', 'br-ex'] - ret = util.collect_neutron_ports(bridges) - self.assertEqual(ret, portnames) + def test_clean_ovs_bridges(self): + conf = mock.Mock() + conf.ovs_all_ports = True + conf.ovs_integration_bridge = 'br-int' + conf.external_network_bridge = 'br-ex' + bridges = [conf.ovs_integration_bridge, conf.external_network_bridge] + with mock.patch('neutron.agent.common.ovs_lib.BaseOVS') as ovs_cls: + ovs_base = mock.Mock() + ovs_base.get_bridges.return_value = bridges + ovs_cls.return_value = ovs_base - @mock.patch.object(ip_lib, 'IPDevice') - def test_delete_neutron_ports(self, mock_ip): - ports = ['tap1234', 'tap5678', 'tap09ab'] - port_found = [True, False, True] - - mock_ip.return_value.exists.side_effect = port_found - util.delete_neutron_ports(ports) - mock_ip.assert_has_calls( - [mock.call('tap1234'), - mock.call().exists(), - mock.call().link.delete(), - mock.call('tap5678'), - mock.call().exists(), - mock.call('tap09ab'), - mock.call().exists(), - mock.call().link.delete()]) + util.do_main(conf) + ovs_base.ovsdb.ovs_cleanup.assert_has_calls( + [mock.call(conf.ovs_integration_bridge, True), + mock.call(conf.external_network_bridge, True)], + any_order=True) diff --git a/releasenotes/notes/removed-ovsdb_interface-ovs-vsctl-timeout-a618ec8e27552202.yaml b/releasenotes/notes/removed-ovsdb_interface-ovs-vsctl-timeout-a618ec8e27552202.yaml new file mode 100644 index 00000000000..d63c97878c3 --- /dev/null +++ b/releasenotes/notes/removed-ovsdb_interface-ovs-vsctl-timeout-a618ec8e27552202.yaml @@ -0,0 +1,7 @@ +--- +upgrade: + - | + The deprecated ``ovsdb_interface`` configuration option has been removed, + the default ``native`` driver is now always used. In addition, the + deprecated ``ovs_vsctl_timeout`` option, which was renamed to + ``ovsdb_timeout`` in Queens, has also been removed.