From bdb7bb987c7cadda00ce33fd672bacaca5af86f0 Mon Sep 17 00:00:00 2001 From: Terry Wilson Date: Tue, 23 Dec 2014 13:49:15 -0700 Subject: [PATCH] Add OVSDB abstract API Abstract all existing run_vsctl calls to an abstract OVSDB API. This will allow the future addition of a native OVSDB protocol implementation of the API without breaking backward compatibility. Partially-Implements: blueprint vsctl-to-ovsdb Change-Id: I2c1b5bc9cb6b9860676bb5e3eaca0402d7e99b01 --- neutron/agent/linux/interface.py | 6 +- neutron/agent/linux/ovs_lib.py | 318 ++++++--------- neutron/agent/linux/ovsdb_vsctl.py | 285 ++++++++++++++ neutron/agent/ovsdb.py | 313 +++++++++++++++ .../openvswitch/agent/ovs_neutron_agent.py | 6 +- .../tests/functional/agent/test_ovs_lib.py | 31 +- .../tests/unit/agent/linux/test_ovs_lib.py | 367 ++++++------------ neutron/tests/unit/ibm/test_sdnve_agent.py | 4 +- .../unit/ofagent/test_ofa_neutron_agent.py | 2 +- .../openvswitch/test_ovs_neutron_agent.py | 20 +- .../tests/unit/openvswitch/test_ovs_tunnel.py | 2 +- neutron/tests/unit/test_linux_interface.py | 14 +- 12 files changed, 860 insertions(+), 508 deletions(-) create mode 100644 neutron/agent/linux/ovsdb_vsctl.py create mode 100644 neutron/agent/ovsdb.py diff --git a/neutron/agent/linux/interface.py b/neutron/agent/linux/interface.py index 0d3ee1457..eff2cec8c 100644 --- a/neutron/agent/linux/interface.py +++ b/neutron/agent/linux/interface.py @@ -207,9 +207,9 @@ class OVSInterfaceDriver(LinuxInterfaceDriver): def _ovs_add_port(self, bridge, device_name, port_id, mac_address, internal=True): - attrs = [('external-ids:iface-id', port_id), - ('external-ids:iface-status', 'active'), - ('external-ids:attached-mac', mac_address)] + attrs = [('external_ids', {'iface-id': port_id, + 'iface-status': 'active', + 'attached-mac': mac_address})] if internal: attrs.insert(0, ('type', 'internal')) diff --git a/neutron/agent/linux/ovs_lib.py b/neutron/agent/linux/ovs_lib.py index 33801f2e2..b73907a6d 100644 --- a/neutron/agent/linux/ovs_lib.py +++ b/neutron/agent/linux/ovs_lib.py @@ -13,18 +13,18 @@ # License for the specific language governing permissions and limitations # under the License. +import collections import itertools -import numbers import operator from oslo.config import cfg -from oslo.serialization import jsonutils from oslo.utils import excutils import retrying import six from neutron.agent.linux import ip_lib from neutron.agent.linux import utils +from neutron.agent import ovsdb from neutron.common import exceptions from neutron.i18n import _LE, _LI, _LW from neutron.openstack.common import log as logging @@ -34,7 +34,8 @@ from neutron.plugins.common import constants DEFAULT_OVS_VSCTL_TIMEOUT = 10 # Special return value for an invalid OVS ofport -INVALID_OFPORT = '-1' +INVALID_OFPORT = -1 +UNASSIGNED_OFPORT = [] OPTS = [ cfg.IntOpt('ovs_vsctl_timeout', @@ -95,90 +96,42 @@ class BaseOVS(object): def __init__(self, root_helper): self.root_helper = root_helper self.vsctl_timeout = cfg.CONF.ovs_vsctl_timeout - - def run_vsctl(self, args, check_error=False): - full_args = ["ovs-vsctl", "--timeout=%d" % self.vsctl_timeout] + args - try: - return utils.execute(full_args, root_helper=self.root_helper) - except Exception as e: - with excutils.save_and_reraise_exception() as ctxt: - LOG.error(_LE("Unable to execute %(cmd)s. " - "Exception: %(exception)s"), - {'cmd': full_args, 'exception': e}) - if not check_error: - ctxt.reraise = False + self.ovsdb = ovsdb.API.get(self) def add_bridge(self, bridge_name): - self.run_vsctl(["--", "--may-exist", "add-br", bridge_name]) + self.ovsdb.add_br(bridge_name).execute() return OVSBridge(bridge_name, self.root_helper) def delete_bridge(self, bridge_name): - self.run_vsctl(["--", "--if-exists", "del-br", bridge_name]) + self.ovsdb.del_br(bridge_name).execute() def bridge_exists(self, bridge_name): - try: - self.run_vsctl(['br-exists', bridge_name], check_error=True) - except RuntimeError as e: - with excutils.save_and_reraise_exception() as ctxt: - if 'Exit code: 2\n' in str(e): - ctxt.reraise = False - return False - return True - - def get_bridge_name_for_port_name(self, port_name): - try: - return self.run_vsctl( - ['port-to-br', port_name], check_error=True).strip() - except RuntimeError as e: - with excutils.save_and_reraise_exception() as ctxt: - if 'Exit code: 1\n' in str(e): - ctxt.reraise = False + return self.ovsdb.br_exists(bridge_name).execute() def port_exists(self, port_name): - return bool(self.get_bridge_name_for_port_name(port_name)) + cmd = self.ovsdb.db_get('Port', port_name, 'name') + return bool(cmd.execute(check_error=False, log_errors=False)) def get_bridge_for_iface(self, iface): - res = self.run_vsctl(['iface-to-br', iface]) - if res: - return res.strip() + return self.ovsdb.iface_to_br(iface).execute() def get_bridges(self): - res = self.run_vsctl(['list-br'], check_error=True) - return res.strip().split('\n') if res else [] + return self.ovsdb.list_br().execute(check_error=True) def get_bridge_external_bridge_id(self, bridge): - res = self.run_vsctl(['br-get-external-id', bridge, 'bridge-id']) - if res: - return res.strip() + return self.ovsdb.br_get_external_id(bridge, 'bridge-id').execute() - def set_db_attribute(self, table_name, record, column, value): - args = ["set", table_name, record, "%s=%s" % (column, value)] - self.run_vsctl(args) + def set_db_attribute(self, table_name, record, column, value, + check_error=False): + self.ovsdb.db_set(table_name, record, (column, value)).execute( + check_error=check_error) def clear_db_attribute(self, table_name, record, column): - args = ["clear", table_name, record, column] - self.run_vsctl(args) - - def _db_get_map(self, table, record, column, check_error=False): - def db_str_to_map(full_str): - entries = full_str.strip("{}").split(", ") - ret = {} - for e in entries: - key, sep, value = e.partition('=') - if sep != "": - ret[key] = value.strip('"') - return ret - - output = self.db_get_val(table, record, column, - check_error=check_error) - if output: - return db_str_to_map(output) - return {} + self.ovsdb.db_clear(table_name, record, column).execute() def db_get_val(self, table, record, column, check_error=False): - output = self.run_vsctl(["get", table, record, column], check_error) - if output: - return output.rstrip("\n\r") + return self.ovsdb.db_get(table, record, column).execute( + check_error=check_error) class OVSBridge(BaseOVS): @@ -186,64 +139,56 @@ class OVSBridge(BaseOVS): super(OVSBridge, self).__init__(root_helper) self.br_name = br_name - def set_controller(self, controller_names): - vsctl_command = ['--', 'set-controller', self.br_name] - vsctl_command.extend(controller_names) - self.run_vsctl(vsctl_command, check_error=True) + def set_controller(self, controllers): + self.ovsdb.set_controller(self.br_name, + controllers).execute(check_error=True) def del_controller(self): - self.run_vsctl(['--', 'del-controller', self.br_name], - check_error=True) + self.ovsdb.del_controller(self.br_name).execute(check_error=True) def get_controller(self): - res = self.run_vsctl(['--', 'get-controller', self.br_name], - check_error=True) - if res: - return res.strip().split('\n') - return res + return self.ovsdb.get_controller(self.br_name).execute( + check_error=True) def set_secure_mode(self): - self.run_vsctl(['--', 'set-fail-mode', self.br_name, 'secure'], - check_error=True) + self.ovsdb.set_fail_mode(self.br_name, 'secure').execute( + check_error=True) def set_protocols(self, protocols): - self.run_vsctl(['--', 'set', 'bridge', self.br_name, - "protocols=%s" % protocols], - check_error=True) + self.set_db_attribute('bridge', self.br_name, 'protocols', protocols, + check_error=True) def create(self): - self.add_bridge(self.br_name) + self.ovsdb.add_br(self.br_name).execute() def destroy(self): self.delete_bridge(self.br_name) def reset_bridge(self): - self.destroy() - self.create() - - def add_port(self, port_name, *interface_options): - args = ["--", "--may-exist", "add-port", self.br_name, port_name] - if interface_options: - args += ['--', 'set', 'Interface', port_name] - args += ['%s=%s' % kv for kv in interface_options] - self.run_vsctl(args) - ofport = self.get_port_ofport(port_name) - if ofport == INVALID_OFPORT: - self.delete_port(port_name) - return ofport + with self.ovsdb.transaction() as txn: + txn.add(self.ovsdb.del_br(self.br_name)) + txn.add(self.ovsdb.add_br(self.br_name)) + + def add_port(self, port_name, *interface_attr_tuples): + with self.ovsdb.transaction() as txn: + txn.add(self.ovsdb.add_port(self.br_name, port_name)) + if interface_attr_tuples: + txn.add(self.ovsdb.db_set('Interface', port_name, + *interface_attr_tuples)) + return self.get_port_ofport(port_name) def replace_port(self, port_name, *interface_attr_tuples): """Replace existing port or create it, and configure port interface.""" - cmd = ['--', '--if-exists', 'del-port', port_name, - '--', 'add-port', self.br_name, port_name] - if interface_attr_tuples: - cmd += ['--', 'set', 'Interface', port_name] - cmd += ['%s=%s' % kv for kv in interface_attr_tuples] - self.run_vsctl(cmd) + with self.ovsdb.transaction() as txn: + txn.add(self.ovsdb.del_port(port_name)) + txn.add(self.ovsdb.add_port(self.br_name, port_name, + may_exist=False)) + if interface_attr_tuples: + txn.add(self.ovsdb.db_set('Interface', port_name, + *interface_attr_tuples)) def delete_port(self, port_name): - self.run_vsctl(["--", "--if-exists", "del-port", self.br_name, - port_name]) + self.ovsdb.del_port(port_name, self.br_name).execute() def run_ofctl(self, cmd, args, process_input=None): full_args = ["ovs-ofctl", cmd, self.br_name] + args @@ -279,7 +224,7 @@ class OVSBridge(BaseOVS): def get_datapath_id(self): return self.db_get_val('Bridge', - self.br_name, 'datapath_id').strip('"') + self.br_name, 'datapath_id') def do_action_flows(self, action, kwargs_list): flow_strs = [_build_flow_expr_str(kw, action) for kw in kwargs_list] @@ -310,34 +255,34 @@ class OVSBridge(BaseOVS): tunnel_type=constants.TYPE_GRE, vxlan_udp_port=constants.VXLAN_UDP_PORT, dont_fragment=True): - options = [('type', tunnel_type)] - if (tunnel_type == constants.TYPE_VXLAN and - vxlan_udp_port != constants.VXLAN_UDP_PORT): - # Only set the VXLAN UDP port if it's not the default - options.append(("options:dst_port", vxlan_udp_port)) - options.extend([ - ("options:df_default", str(bool(dont_fragment)).lower()), - ("options:remote_ip", remote_ip), - ("options:local_ip", local_ip), - ("options:in_key", "flow"), - ("options:out_key", "flow")]) - return self.add_port(port_name, *options) + attrs = [('type', tunnel_type)] + # TODO(twilson) This is an OrderedDict solely to make a test happy + options = collections.OrderedDict() + vxlan_uses_custom_udp_port = ( + tunnel_type == constants.TYPE_VXLAN and + vxlan_udp_port != constants.VXLAN_UDP_PORT + ) + if vxlan_uses_custom_udp_port: + options['dst_port'] = vxlan_udp_port + options['df_default'] = str(dont_fragment).lower() + options['remote_ip'] = remote_ip + options['local_ip'] = local_ip + options['in_key'] = 'flow' + options['out_key'] = 'flow' + attrs.append(('options', options)) + + return self.add_port(port_name, *attrs) def add_patch_port(self, local_name, remote_name): - options = [ - ('type', 'patch'), - ('options:peer', remote_name) - ] - return self.add_port(local_name, *options) + attrs = [('type', 'patch'), + ('options', {'peer': remote_name})] + return self.add_port(local_name, *attrs) def get_port_name_list(self): - res = self.run_vsctl(["list-ports", self.br_name], check_error=True) - if res: - return res.strip().split("\n") - return [] + return self.ovsdb.list_ports(self.br_name).execute(check_error=True) def get_port_stats(self, port_name): - return self._db_get_map("Interface", port_name, "statistics") + return self.db_get_val("Interface", port_name, "statistics") def get_xapi_iface_id(self, xs_vif_uuid): args = ["xe", "vif-param-get", "param-name=other-config", @@ -355,8 +300,8 @@ class OVSBridge(BaseOVS): edge_ports = [] port_names = self.get_port_name_list() for name in port_names: - external_ids = self._db_get_map("Interface", name, "external_ids", - check_error=True) + external_ids = self.db_get_val("Interface", name, "external_ids", + check_error=True) ofport = self.db_get_val("Interface", name, "ofport", check_error=True) if "iface-id" in external_ids and "attached-mac" in external_ids: @@ -376,31 +321,25 @@ class OVSBridge(BaseOVS): def get_vif_port_set(self): edge_ports = set() - args = ['--format=json', '--', '--columns=external_ids,ofport', - '--if-exists', 'list', 'Interface'] - args += self.get_port_name_list() - result = self.run_vsctl(args, check_error=True) - if not result: - return edge_ports - for row in jsonutils.loads(result)['data']: - external_ids = dict(row[0][1]) - # Do not consider VIFs which aren't yet ready - # This can happen when ofport values are either [] or ["set", []] - # We will therefore consider only integer values for ofport - ofport = row[1] - if not isinstance(ofport, numbers.Integral): - LOG.warn(_LW("Found not yet ready openvswitch port: %s"), row) - elif ofport < 1: - LOG.warn(_LW("Found failed openvswitch port: %s"), row) - elif 'attached-mac' in external_ids: - if "iface-id" in external_ids: + port_names = self.get_port_name_list() + cmd = self.ovsdb.db_list( + 'Interface', port_names, + columns=['name', 'external_ids', 'ofport'], if_exists=True) + results = cmd.execute(check_error=True) + for result in results: + if result['ofport'] == UNASSIGNED_OFPORT: + LOG.warn(_LW("Found not yet ready openvswitch port: %s"), + result['name']) + elif result['ofport'] == INVALID_OFPORT: + LOG.warn(_LW("Found failed openvswitch port: %s"), + result['name']) + elif 'attached-mac' in result['external_ids']: + external_ids = result['external_ids'] + if 'iface-id' in external_ids: edge_ports.add(external_ids['iface-id']) - elif "xs-vif-uuid" in external_ids: - # if this is a xenserver and iface-id is not - # automatically synced to OVS from XAPI, we grab it - # from XAPI directly + elif 'xs-vif-uuid' in external_ids: iface_id = self.get_xapi_iface_id( - external_ids["xs-vif-uuid"]) + external_ids['xs-vif-uuid']) edge_ports.add(iface_id) return edge_ports @@ -419,61 +358,28 @@ class OVSBridge(BaseOVS): in the "Interface" table queried by the get_vif_port_set() method. """ - args = ['--format=json', '--', '--columns=name,tag', '--if-exists', - 'list', 'Port'] - args += self.get_port_name_list() - result = self.run_vsctl(args, check_error=True) - port_tag_dict = {} - if not result: - return port_tag_dict - for name, tag in jsonutils.loads(result)['data']: - # 'tag' can be [u'set', []] or an integer - if isinstance(tag, list): - tag = tag[1] - port_tag_dict[name] = tag - return port_tag_dict + port_names = self.get_port_name_list() + cmd = self.ovsdb.db_list('Port', port_names, columns=['name', 'tag']) + results = cmd.execute(check_error=True) + return {p['name']: p['tag'] for p in results} def get_vif_port_by_id(self, port_id): - args = ['--format=json', '--', '--columns=external_ids,name,ofport', - 'find', 'Interface', - 'external_ids:iface-id="%s"' % port_id] - result = self.run_vsctl(args) - if not result: - return - json_result = jsonutils.loads(result) - try: - # Retrieve the indexes of the columns we're looking for - headings = json_result['headings'] - ext_ids_idx = headings.index('external_ids') - name_idx = headings.index('name') - ofport_idx = headings.index('ofport') - # If data attribute is missing or empty the line below will raise - # an exeception which will be captured in this block. - # We won't deal with the possibility of ovs-vsctl return multiple - # rows since the interface identifier is unique - for data in json_result['data']: - port_name = data[name_idx] - switch = self.get_bridge_for_iface(port_name) - if switch != self.br_name: - continue - ofport = data[ofport_idx] - # ofport must be integer otherwise return None - if not isinstance(ofport, int) or ofport == -1: - LOG.warn(_LW("ofport: %(ofport)s for VIF: %(vif)s is not a" - " positive integer"), {'ofport': ofport, - 'vif': port_id}) - return - # Find VIF's mac address in external ids - ext_id_dict = dict((item[0], item[1]) for item in - data[ext_ids_idx][1]) - vif_mac = ext_id_dict['attached-mac'] - return VifPort(port_name, ofport, port_id, vif_mac, self) - LOG.info(_LI("Port %(port_id)s not present in bridge %(br_name)s"), - {'port_id': port_id, 'br_name': self.br_name}) - except Exception as error: - LOG.warn(_LW("Unable to parse interface details. Exception: %s"), - error) - return + ports = self.ovsdb.db_find( + 'Interface', ('external_ids', '=', {'iface-id': port_id}), + ('external_ids', '!=', {'attached-mac': ''}), + columns=['external_ids', 'name', 'ofport']).execute() + for port in ports: + if self.br_name != self.get_bridge_for_iface(port['name']): + continue + if port['ofport'] in [UNASSIGNED_OFPORT, INVALID_OFPORT]: + LOG.warn(_LW("ofport: %(ofport)s for VIF: %(vif)s is not a" + " positive integer"), + {'ofport': port['ofport'], 'vif': port_id}) + continue + mac = port['external_ids'].get('attached-mac') + return VifPort(port['name'], port['ofport'], port_id, mac, self) + LOG.info(_LI("Port %(port_id)s not present in bridge %(br_name)s"), + {'port_id': port_id, 'br_name': self.br_name}) def delete_ports(self, all_ports=False): if all_ports: diff --git a/neutron/agent/linux/ovsdb_vsctl.py b/neutron/agent/linux/ovsdb_vsctl.py new file mode 100644 index 000000000..d19dc53c0 --- /dev/null +++ b/neutron/agent/linux/ovsdb_vsctl.py @@ -0,0 +1,285 @@ +# 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.serialization import jsonutils +from oslo.utils import excutils + +from neutron.agent.linux import utils +from neutron.agent import ovsdb +from neutron.i18n import _LE +from neutron.openstack.common import log as logging + +LOG = logging.getLogger(__name__) + + +class Transaction(ovsdb.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, + root_helper=self.context.root_helper, + log_fail_as_error=False).rstrip() + except Exception: + with excutils.save_and_reraise_exception() as ctxt: + if self.log_errors: + LOG.exception(_LE("Unable to execute %(cmd)s."), + {'cmd': full_args}) + if not self.check_error: + ctxt.reraise = False + + +class BaseCommand(ovsdb.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): + # This shouldn't happen, but if it does and we check_errors + # log and raise. + with excutils.save_and_reraise_exception(): + LOG.exception(_LE("Could not parse: %s"), raw_result) + + headings = json['headings'] + data = json['data'] + results = [] + for record in data: + obj = {} + for pos, heading in enumerate(headings): + obj[heading] = _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 = self._result[0].values()[0] + + +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): + def transaction(self, check_error=False, log_errors=True, **kwargs): + return Transaction(self.context, check_error, log_errors, **kwargs) + + def add_br(self, name, may_exist=True): + opts = ['--may-exist'] if may_exist else None + return BaseCommand(self.context, 'add-br', opts, [name]) + + 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_set(self, table, record, *col_values): + args = [table, record] + args += _set_colval_args(*col_values) + return BaseCommand(self.context, 'set', 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 _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, _py_to_val(v)) for k, v in val.items()] + elif (isinstance(val, collections.Sequence) + and not isinstance(val, basestring)): + args.append("%s%s%s" % (col, op, ",".join(map(_py_to_val, val)))) + else: + args.append("%s%s%s" % (col, op, _py_to_val(val))) + return args + + +def _val_to_py(val): + """Convert a json ovsdb return value to native python object""" + if isinstance(val, collections.Sequence) and len(val) == 2: + if val[0] == "uuid": + return uuid.UUID(val[1]) + elif val[0] == "set": + return [_val_to_py(x) for x in val[1]] + 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: + return pyval diff --git a/neutron/agent/ovsdb.py b/neutron/agent/ovsdb.py new file mode 100644 index 000000000..288112ca4 --- /dev/null +++ b/neutron/agent/ovsdb.py @@ -0,0 +1,313 @@ +# 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 abc + +from oslo.config import cfg +from oslo.utils import importutils +import six + +interface_map = { + 'vsctl': 'neutron.agent.linux.ovsdb_vsctl.OvsdbVsctl', +} + +OPTS = [ + cfg.StrOpt('ovsdb_interface', + choices=interface_map.keys(), + default='vsctl', + help=_('The interface for interacting with the OVSDB')), +] +cfg.CONF.register_opts(OPTS, 'OVS') + + +@six.add_metaclass(abc.ABCMeta) +class Command(object): + """An OSVDB command that can be executed in a transaction + + :attr result: The result of executing the command in a transaction + """ + + @abc.abstractmethod + def execute(self, **transaction_options): + """Immediately execute an OVSDB command + + This implicitly creates a transaction with the passed options and then + executes it, returning the value of the executed transaction + + :param transaction_options: Options to pass to the transaction + """ + + +@six.add_metaclass(abc.ABCMeta) +class Transaction(object): + @abc.abstractmethod + def commit(self): + """Commit the transaction to OVSDB""" + + @abc.abstractmethod + def add(self, command): + """Append an OVSDB operation to the transaction""" + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_val, tb): + if exc_type is None: + self.result = self.commit() + + +@six.add_metaclass(abc.ABCMeta) +class API(object): + def __init__(self, context): + self.context = context + + @staticmethod + def get(context, iface_name=None): + """Return the configured OVSDB API implementation""" + iface = importutils.import_class( + interface_map[iface_name or cfg.CONF.OVS.ovsdb_interface]) + return iface(context) + + @abc.abstractmethod + def transaction(self, check_error=False, log_errors=True, **kwargs): + """Create a transaction + + :param check_error: Allow the transaction to raise an exception? + :type check_error: bool + :param log_errors: Log an error if the transaction fails? + :type log_errors: bool + :returns: A new transaction + :rtype: :class:`Transaction` + """ + + @abc.abstractmethod + def add_br(self, name, may_exist=True): + """Create an command to add an OVS bridge + + :param name: The name of the bridge + :type name: string + :param may_exist: Do not fail if bridge already exists + :type may_exist: bool + :returns: :class:`Command` with no result + """ + + @abc.abstractmethod + def del_br(self, name, if_exists=True): + """Create a command to delete an OVS bridge + + :param name: The name of the bridge + :type name: string + :param if_exists: Do not fail if the bridge does not exist + :type if_exists: bool + :returns: :class:`Command` with no result + """ + + @abc.abstractmethod + def br_exists(self, name): + """Create a command to check if an OVS bridge exists + + :param name: The name of the bridge + :type name: string + :returns: :class:`Command` with bool result + """ + + @abc.abstractmethod + def port_to_br(self, name): + """Create a command to return the name of the bridge with the port + + :param name: The name of the OVS port + :type name: string + :returns: :class:`Command` with bridge name result + """ + + @abc.abstractmethod + def iface_to_br(self, name): + """Create a command to return the name of the bridge with the interface + + :param name: The name of the OVS interface + :type name: string + :returns: :class:`Command` with bridge name result + """ + + @abc.abstractmethod + def list_br(self): + """Create a command to return the current list of OVS bridge names + + :returns: :class:`Command` with list of bridge names result + """ + + @abc.abstractmethod + def br_get_external_id(self, name, field): + """Create a command to return a field from the Bridge's external_ids + + :param name: The name of the OVS Bridge + :type name: string + :param field: The external_ids field to return + :type field: string + :returns: :class:`Command` with field value result + """ + + @abc.abstractmethod + def db_set(self, table, record, *col_values): + """Create a command to set fields in a record + + :param table: The OVS table containing the record to be modified + :type table: string + :param record: The record id (name/uuid) to be modified + :type table: string + :param col_values: The columns and their associated values + :type col_values: Tuples of (column, value). Values may be atomic + values or unnested sequences/mappings + :returns: :class:`Command` with no result + """ + # TODO(twilson) Consider handling kwargs for arguments where order + # doesn't matter. Though that would break the assert_called_once_with + # unit tests + + @abc.abstractmethod + def db_clear(self, table, record, column): + """Create a command to clear a field's value in a record + + :param table: The OVS table containing the record to be modified + :type table: string + :param record: The record id (name/uuid) to be modified + :type record: string + :param column: The column whose value should be cleared + :type column: string + :returns: :class:`Command` with no result + """ + + @abc.abstractmethod + def db_get(self, table, record, column): + """Create a command to return a field's value in a record + + :param table: The OVS table containing the record to be queried + :type table: string + :param record: The record id (name/uuid) to be queried + :type record: string + :param column: The column whose value should be returned + :type column: string + :returns: :class:`Command` with the field's value result + """ + + @abc.abstractmethod + def db_list(self, table, records=None, columns=None, if_exists=False): + """Create a command to return a list of OVSDB records + + :param table: The OVS table to query + :type table: string + :param records: The records to return values from + :type records: list of record ids (names/uuids) + :param columns: Limit results to only columns, None means all columns + :type columns: list of column names or None + :param if_exists: Do not fail if the bridge does not exist + :type if_exists: bool + :returns: :class:`Command` with [{'column', value}, ...] result + """ + + @abc.abstractmethod + def db_find(self, table, *conditions, **kwargs): + """Create a command to return find OVSDB records matching conditions + + :param table: The OVS table to query + :type table: string + :param conditions:The conditions to satisfy the query + :type conditions: 3-tuples containing (column, operation, match) + Examples: + atomic: ('tag', '=', 7) + map: ('external_ids' '=', {'iface-id': 'xxx'}) + field exists? + ('external_ids', '!=', {'iface-id', ''}) + set contains?: + ('protocols', '{>=}', 'OpenFlow13') + See the ovs-vsctl man page for more operations + :param columns: Limit results to only columns, None means all columns + :type columns: list of column names or None + :returns: :class:`Command` with [{'column', value}, ...] result + """ + + @abc.abstractmethod + def set_controller(self, bridge, controllers): + """Create a command to set an OVS bridge's OpenFlow controllers + + :param bridge: The name of the bridge + :type bridge: string + :param controllers: The controller strings + :type controllers: list of strings, see ovs-vsctl manpage for format + :returns: :class:`Command` with no result + """ + + @abc.abstractmethod + def del_controller(self, bridge): + """Create a command to clear an OVS bridge's OpenFlow controllers + + :param bridge: The name of the bridge + :type bridge: string + :returns: :class:`Command` with no result + """ + + @abc.abstractmethod + def get_controller(self, bridge): + """Create a command to return an OVS bridge's OpenFlow controllers + + :param bridge: The name of the bridge + :type bridge: string + :returns: :class:`Command` with list of controller strings result + """ + + @abc.abstractmethod + def set_fail_mode(self, bridge, mode): + """Create a command to set an OVS bridge's failure mode + + :param bridge: The name of the bridge + :type bridge: string + :param mode: The failure mode + :type mode: "secure" or "standalone" + :returns: :class:`Command` with no result + """ + + @abc.abstractmethod + def add_port(self, bridge, port, may_exist=True): + """Create a command to add a port to an OVS bridge + + :param bridge: The name of the bridge + :type bridge: string + :param port: The name of the port + :type port: string + :param may_exist: Do not fail if bridge already exists + :type may_exist: bool + :returns: :class:`Command` with no result + """ + + @abc.abstractmethod + def del_port(self, port, bridge=None, if_exists=True): + """Create a command to delete a port an OVS port + + :param port: The name of the port + :type port: string + :param bridge: Only delete port if it is attached to this bridge + :type bridge: string + :param if_exists: Do not fail if the bridge does not exist + :type if_exists: bool + :returns: :class:`Command` with no result + """ + + @abc.abstractmethod + def list_ports(self, bridge): + """Create a command to list the names of porsts on a bridge + + :param bridge: The name of the bridge + :type bridge: string + :returns: :class:`Command` with list of port names result + """ diff --git a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py index a00100d61..e4639eb02 100644 --- a/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py +++ b/neutron/plugins/openvswitch/agent/ovs_neutron_agent.py @@ -53,7 +53,7 @@ LOG = logging.getLogger(__name__) cfg.CONF.import_group('AGENT', 'neutron.plugins.openvswitch.common.config') # A placeholder for dead vlans. -DEAD_VLAN_TAG = str(q_const.MAX_VLAN_TAG + 1) +DEAD_VLAN_TAG = q_const.MAX_VLAN_TAG + 1 class DeviceListRetrievalError(exceptions.NeutronException): @@ -650,9 +650,9 @@ class OVSNeutronAgent(sg_rpc.SecurityGroupAgentRpcCallbackMixin, # Do not bind a port if it's already bound cur_tag = self.int_br.db_get_val("Port", port.port_name, "tag") - if cur_tag != str(lvm.vlan): + if cur_tag != lvm.vlan: self.int_br.set_db_attribute("Port", port.port_name, "tag", - str(lvm.vlan)) + lvm.vlan) if port.ofport != -1: self.int_br.delete_flows(in_port=port.ofport) diff --git a/neutron/tests/functional/agent/test_ovs_lib.py b/neutron/tests/functional/agent/test_ovs_lib.py index c817fab4d..09c60ca62 100644 --- a/neutron/tests/functional/agent/test_ovs_lib.py +++ b/neutron/tests/functional/agent/test_ovs_lib.py @@ -52,8 +52,6 @@ class OVSBridgeTestCase(base.BaseOVSLinuxTestCase): self.assertTrue(int(ofport)) self.assertTrue(int(self.br.get_port_ofport(port_name))) self.assertTrue(self.br.port_exists(port_name)) - self.assertEqual(self.br.br_name, - self.br.get_bridge_name_for_port_name(port_name)) self.assertEqual(self.br.br_name, self.br.get_bridge_for_iface(port_name)) self.br.delete_port(port_name) @@ -68,22 +66,17 @@ class OVSBridgeTestCase(base.BaseOVSLinuxTestCase): self.br.replace_port(port_name, ('type', 'internal'), ('external_ids:test', 'test')) self.assertTrue(self.br.port_exists(port_name)) - self.assertEqual('test', self.br._db_get_map('Interface', port_name, - 'external_ids')['test']) + self.assertEqual('test', self.br.db_get_val('Interface', port_name, + 'external_ids')['test']) def test_attribute_lifecycle(self): (port_name, ofport) = self.create_ovs_port() - # TODO(twilson) The existing set/get/clear functions are horribly - # limited by not understanding what types should actually be returned - # In the OVSDB Abstract Interface, they should understand/deal with - # real python types - tag = '42' + tag = 42 self.ovs.set_db_attribute('Port', port_name, 'tag', tag) self.assertEqual(tag, self.ovs.db_get_val('Port', port_name, 'tag')) - self.assertEqual(int(tag), self.br.get_port_tag_dict()[port_name]) + self.assertEqual(tag, self.br.get_port_tag_dict()[port_name]) self.ovs.clear_db_attribute('Port', port_name, 'tag') - # ick, ick, ick - self.assertEqual(self.ovs.db_get_val('Port', port_name, 'tag'), '[]') + self.assertEqual(self.ovs.db_get_val('Port', port_name, 'tag'), []) self.assertEqual(self.br.get_port_tag_dict()[port_name], []) def test_get_bridge_external_bridge_id(self): @@ -98,10 +91,7 @@ class OVSBridgeTestCase(base.BaseOVSLinuxTestCase): self.br.set_controller(controllers) self.assertSetEqual(controllers, set(self.br.get_controller())) self.br.del_controller() - # TODO(twilson) I would prefer this test against [], but currently - # get_controller returns '' when there are no controllers. This will - # change with the OVSDB Abstract Interface work. - self.assertEqual(0, len(self.br.get_controller())) + self.assertEqual([], self.br.get_controller()) def test_set_fail_mode(self): self.br.set_secure_mode() @@ -110,13 +100,10 @@ class OVSBridgeTestCase(base.BaseOVSLinuxTestCase): 'secure') def test_set_protocols(self): - # TODO(twilson) set_protocols is technically broken as it should - # allow one to set an array of allowed protocols self.br.set_protocols('OpenFlow10') - # and again, ick, this has to change self.assertEqual( self.br.db_get_val('Bridge', self.br.br_name, 'protocols'), - '["OpenFlow10"]') + "OpenFlow10") def test_get_datapath_id(self): brdev = self.ip.device(self.br.br_name) @@ -135,7 +122,7 @@ class OVSBridgeTestCase(base.BaseOVSLinuxTestCase): attrs['local_ip']) self.assertEqual(self.ovs.db_get_val('Interface', port_name, 'type'), 'gre') - options = self.ovs._db_get_map('Interface', port_name, 'options') + options = self.ovs.db_get_val('Interface', port_name, 'options') for attr, val in attrs.items(): self.assertEqual(val, options[attr]) @@ -145,7 +132,7 @@ class OVSBridgeTestCase(base.BaseOVSLinuxTestCase): self.br.add_patch_port(local, peer) self.assertEqual(self.ovs.db_get_val('Interface', local, 'type'), 'patch') - options = self.ovs._db_get_map('Interface', local, 'options') + options = self.ovs.db_get_val('Interface', local, 'options') self.assertEqual(peer, options['peer']) def test_get_port_name_list(self): diff --git a/neutron/tests/unit/agent/linux/test_ovs_lib.py b/neutron/tests/unit/agent/linux/test_ovs_lib.py index 158d2bdcf..465389825 100644 --- a/neutron/tests/unit/agent/linux/test_ovs_lib.py +++ b/neutron/tests/unit/agent/linux/test_ovs_lib.py @@ -14,7 +14,6 @@ import collections import mock -from oslo.config import cfg from oslo.serialization import jsonutils import testtools @@ -30,79 +29,6 @@ from neutron.tests import tools OVS_LINUX_KERN_VERS_WITHOUT_VXLAN = "3.12.0" -class TestBaseOVS(base.BaseTestCase): - - def setUp(self): - super(TestBaseOVS, self).setUp() - self.root_helper = 'sudo' - self.ovs = ovs_lib.BaseOVS(self.root_helper) - self.br_name = 'bridge1' - - def test_add_bridge(self): - with mock.patch.object(self.ovs, 'run_vsctl') as mock_vsctl: - bridge = self.ovs.add_bridge(self.br_name) - - mock_vsctl.assert_called_with(["--", "--may-exist", - "add-br", self.br_name]) - self.assertEqual(bridge.br_name, self.br_name) - self.assertEqual(bridge.root_helper, self.ovs.root_helper) - - def test_delete_bridge(self): - with mock.patch.object(self.ovs, 'run_vsctl') as mock_vsctl: - self.ovs.delete_bridge(self.br_name) - mock_vsctl.assert_called_with(["--", "--if-exists", "del-br", - self.br_name]) - - def test_bridge_exists_returns_true(self): - with mock.patch.object(self.ovs, 'run_vsctl') as mock_vsctl: - self.assertTrue(self.ovs.bridge_exists(self.br_name)) - mock_vsctl.assert_called_with(['br-exists', self.br_name], - check_error=True) - - def test_bridge_exists_returns_false_for_exit_code_2(self): - with mock.patch.object(self.ovs, 'run_vsctl', - side_effect=RuntimeError('Exit code: 2\n')): - self.assertFalse(self.ovs.bridge_exists('bridge1')) - - def test_bridge_exists_raises_unknown_exception(self): - with mock.patch.object(self.ovs, 'run_vsctl', - side_effect=RuntimeError()): - with testtools.ExpectedException(RuntimeError): - self.ovs.bridge_exists('bridge1') - - def test_get_bridge_name_for_port_name_returns_bridge_for_valid_port(self): - port_name = 'bar' - with mock.patch.object(self.ovs, 'run_vsctl', - return_value=self.br_name) as mock_vsctl: - bridge = self.ovs.get_bridge_name_for_port_name(port_name) - self.assertEqual(bridge, self.br_name) - mock_vsctl.assert_called_with(['port-to-br', port_name], - check_error=True) - - def test_get_bridge_name_for_port_name_returns_none_for_exit_code_1(self): - with mock.patch.object(self.ovs, 'run_vsctl', - side_effect=RuntimeError('Exit code: 1\n')): - self.assertFalse(self.ovs.get_bridge_name_for_port_name('bridge1')) - - def test_get_bridge_name_for_port_name_raises_unknown_exception(self): - with mock.patch.object(self.ovs, 'run_vsctl', - side_effect=RuntimeError()): - with testtools.ExpectedException(RuntimeError): - self.ovs.get_bridge_name_for_port_name('bridge1') - - def _test_port_exists(self, br_name, result): - with mock.patch.object(self.ovs, - 'get_bridge_name_for_port_name', - return_value=br_name): - self.assertEqual(self.ovs.port_exists('bar'), result) - - def test_port_exists_returns_true_for_bridge_name(self): - self._test_port_exists(self.br_name, True) - - def test_port_exists_returns_false_for_none(self): - self._test_port_exists(None, False) - - class OFCTLParamListMatcher(object): def _parse(self, params): @@ -131,13 +57,31 @@ class OVS_Lib_Test(base.BaseTestCase): def setUp(self): super(OVS_Lib_Test, self).setUp() self.BR_NAME = "br-int" - self.TO = "--timeout=10" self.root_helper = 'sudo' self.br = ovs_lib.OVSBridge(self.BR_NAME, self.root_helper) 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, root_helper=self.root_helper, + log_fail_as_error=False) + + def _verify_vsctl_mock(self, *args): + cmd = self._vsctl_args(*args) + self.execute.assert_called_once_with(cmd, root_helper=self.root_helper, + log_fail_as_error=False) + def test_vifport(self): """Create and stringify vif port, confirm no exceptions.""" @@ -160,39 +104,30 @@ class OVS_Lib_Test(base.BaseTestCase): def test_set_controller(self): controller_names = ['tcp:127.0.0.1:6633', 'tcp:172.17.16.10:5555'] self.br.set_controller(controller_names) - self.execute.assert_called_once_with( - ['ovs-vsctl', self.TO, '--', 'set-controller', self.BR_NAME, - 'tcp:127.0.0.1:6633', 'tcp:172.17.16.10:5555'], - root_helper=self.root_helper) + self._verify_vsctl_mock('set-controller', self.BR_NAME, + 'tcp:127.0.0.1:6633', 'tcp:172.17.16.10:5555') def test_del_controller(self): self.br.del_controller() - self.execute.assert_called_once_with( - ['ovs-vsctl', self.TO, '--', 'del-controller', self.BR_NAME], - root_helper=self.root_helper) + self._verify_vsctl_mock('del-controller', self.BR_NAME) def test_get_controller(self): - self.execute.return_value = 'tcp:127.0.0.1:6633\ntcp:172.17.16.10:5555' + self.execute.return_value = ( + 'tcp:127.0.0.1:6633\\ntcp:172.17.16.10:5555') names = self.br.get_controller() self.assertEqual(names, ['tcp:127.0.0.1:6633', 'tcp:172.17.16.10:5555']) - self.execute.assert_called_once_with( - ['ovs-vsctl', self.TO, '--', 'get-controller', self.BR_NAME], - root_helper=self.root_helper) + self._verify_vsctl_mock('get-controller', self.BR_NAME) def test_set_secure_mode(self): self.br.set_secure_mode() - self.execute.assert_called_once_with( - ['ovs-vsctl', self.TO, '--', 'set-fail-mode', self.BR_NAME, - 'secure'], root_helper=self.root_helper) + self._verify_vsctl_mock('set-fail-mode', self.BR_NAME, 'secure') def test_set_protocols(self): protocols = 'OpenFlow13' self.br.set_protocols(protocols) - self.execute.assert_called_once_with( - ['ovs-vsctl', self.TO, '--', 'set', 'bridge', self.BR_NAME, - "protocols=%s" % protocols], - root_helper=self.root_helper) + self._verify_vsctl_mock('set', 'bridge', self.BR_NAME, + "protocols=%s" % protocols) def test_create(self): self.br.add_bridge(self.BR_NAME) @@ -216,32 +151,23 @@ class OVS_Lib_Test(base.BaseTestCase): def test_replace_port(self): pname = "tap5" self.br.replace_port(pname) - self.execute.assert_called_once_with( - ["ovs-vsctl", self.TO, - "--", "--if-exists", "del-port", pname, - "--", "add-port", self.BR_NAME, pname], - root_helper=self.root_helper) + self._verify_vsctl_mock("--if-exists", "del-port", pname, + "--", "add-port", self.BR_NAME, pname) def test_replace_port_with_attrs(self): pname = "tap5" self.br.replace_port(pname, ('type', 'internal'), - ('external-ids:iface-status', 'active')) - self.execute.assert_called_once_with( - ["ovs-vsctl", self.TO, - "--", "--if-exists", "del-port", pname, - "--", "add-port", self.BR_NAME, pname, - "--", "set", "Interface", pname, - "type=internal", "external-ids:iface-status=active"], - root_helper=self.root_helper) + ('external_ids:iface-status', 'active')) + self._verify_vsctl_mock("--if-exists", "del-port", pname, + "--", "add-port", self.BR_NAME, pname, + "--", "set", "Interface", pname, + "type=internal", + "external_ids:iface-status=active") def _test_delete_port(self, exp_timeout=None): - exp_timeout_str = self._build_timeout_opt(exp_timeout) pname = "tap5" self.br.delete_port(pname) - self.execute.assert_called_once_with( - ["ovs-vsctl", exp_timeout_str, "--", "--if-exists", - "del-port", self.BR_NAME, pname], - root_helper=self.root_helper) + self._verify_vsctl_mock("--if-exists", "del-port", self.BR_NAME, pname) def test_delete_port(self): self._test_delete_port() @@ -363,27 +289,19 @@ class OVS_Lib_Test(base.BaseTestCase): "actions=normal", root_helper=self.root_helper) - def _set_timeout(self, val): - self.TO = '--timeout=%d' % val - self.br.vsctl_timeout = val - def _test_get_port_ofport(self, ofport, expected_result): pname = "tap99" - self._set_timeout(0) # Don't waste precious time retrying - self.execute.return_value = ofport + 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.execute.assert_called_with( - ["ovs-vsctl", self.TO, "get", "Interface", pname, "ofport"], - root_helper=self.root_helper) + 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") + 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_ofport_for_none(self): - self._test_get_port_ofport(None, ovs_lib.INVALID_OFPORT) + 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, @@ -391,12 +309,11 @@ class OVS_Lib_Test(base.BaseTestCase): def test_get_datapath_id(self): datapath_id = '"0000b67f4fbcc149"' - self.execute.return_value = datapath_id - self.assertEqual(self.br.get_datapath_id(), datapath_id.strip('"')) - self.execute.assert_called_once_with( - ["ovs-vsctl", self.TO, "get", - "Bridge", self.BR_NAME, "datapath_id"], - root_helper=self.root_helper) + self.execute.return_value = self._encode_ovs_json(['datapath_id'], + [[datapath_id]]) + self.assertEqual(self.br.get_datapath_id(), datapath_id) + self._verify_vsctl_mock("--columns=datapath_id", "list", "Bridge", + self.BR_NAME) def test_count_flows(self): self.execute.return_value = 'ignore\nflow-1\n' @@ -482,8 +399,8 @@ class OVS_Lib_Test(base.BaseTestCase): pname = "tap99" local_ip = "1.1.1.1" remote_ip = "9.9.9.9" - ofport = "6" - command = ["ovs-vsctl", self.TO, '--', "--may-exist", "add-port", + ofport = 6 + command = ["--may-exist", "add-port", self.BR_NAME, pname] command.extend(["--", "set", "Interface", pname]) command.extend(["type=gre", "options:df_default=true", @@ -493,11 +410,9 @@ class OVS_Lib_Test(base.BaseTestCase): "options:out_key=flow"]) # Each element is a tuple of (expected mock call, return_value) expected_calls_and_values = [ - (mock.call(command, root_helper=self.root_helper), None), - (mock.call(["ovs-vsctl", self.TO, "get", - "Interface", pname, "ofport"], - root_helper=self.root_helper), - ofport), + (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) @@ -511,11 +426,10 @@ class OVS_Lib_Test(base.BaseTestCase): pname = "tap99" local_ip = "1.1.1.1" remote_ip = "9.9.9.9" - ofport = "6" + ofport = 6 vxlan_udp_port = "9999" dont_fragment = False - command = ["ovs-vsctl", self.TO, '--', "--may-exist", "add-port", - self.BR_NAME, pname] + 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, @@ -526,11 +440,9 @@ class OVS_Lib_Test(base.BaseTestCase): "options:out_key=flow"]) # Each element is a tuple of (expected mock call, return_value) expected_calls_and_values = [ - (mock.call(command, root_helper=self.root_helper), None), - (mock.call(["ovs-vsctl", self.TO, "get", - "Interface", pname, "ofport"], - root_helper=self.root_helper), - ofport), + (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) @@ -545,20 +457,16 @@ class OVS_Lib_Test(base.BaseTestCase): def test_add_patch_port(self): pname = "tap99" peer = "bar10" - ofport = "6" + ofport = 6 # Each element is a tuple of (expected mock call, return_value) - command = ["ovs-vsctl", self.TO, "--", "--may-exist", "add-port", - self.BR_NAME, pname] + command = ["--may-exist", "add-port", self.BR_NAME, pname] command.extend(["--", "set", "Interface", pname]) command.extend(["type=patch", "options:peer=" + peer]) expected_calls_and_values = [ - (mock.call(command, root_helper=self.root_helper), - None), - (mock.call(["ovs-vsctl", self.TO, "get", - "Interface", pname, "ofport"], - root_helper=self.root_helper), - ofport) + (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) @@ -567,30 +475,24 @@ class OVS_Lib_Test(base.BaseTestCase): def _test_get_vif_ports(self, is_xen=False): pname = "tap99" - ofport = "6" + ofport = 6 + ofport_data = self._encode_ovs_json(['ofport'], [[ofport]]) vif_id = uuidutils.generate_uuid() mac = "ca:fe:de:ad:be:ef" - - if is_xen: - external_ids = ('{xs-vif-uuid="%s", attached-mac="%s"}' - % (vif_id, mac)) - else: - external_ids = ('{iface-id="%s", attached-mac="%s"}' - % (vif_id, mac)) + id_field = 'xs-vif-uuid' if is_xen else 'iface-id' + external_ids = ('{"data":[[["map",[["attached-mac","%(mac)s"],' + '["%(id_field)s","%(vif)s"],' + '["iface-status","active"]]]]],' + '"headings":["external_ids"]}' % { + 'mac': mac, 'vif': vif_id, 'id_field': id_field}) # Each element is a tuple of (expected mock call, return_value) expected_calls_and_values = [ - (mock.call(["ovs-vsctl", self.TO, "list-ports", self.BR_NAME], - root_helper=self.root_helper), - "%s\n" % pname), - (mock.call(["ovs-vsctl", self.TO, "get", - "Interface", pname, "external_ids"], - root_helper=self.root_helper), - external_ids), - (mock.call(["ovs-vsctl", self.TO, "get", - "Interface", pname, "ofport"], - root_helper=self.root_helper), - ofport), + (self._vsctl_mock("list-ports", self.BR_NAME), "%s\n" % pname), + (self._vsctl_mock("--columns=external_ids", "list", + "Interface", pname), external_ids), + (self._vsctl_mock("--columns=ofport", "list", "Interface", pname), + ofport_data), ] if is_xen: expected_calls_and_values.append( @@ -635,29 +537,26 @@ class OVS_Lib_Test(base.BaseTestCase): else: id_key = 'iface-id' - headings = ['external_ids', 'ofport'] + headings = ['name', 'external_ids', 'ofport'] data = [ # A vif port on this bridge: - [{id_key: 'tap99id', 'attached-mac': 'tap99mac'}, 1], + ['tap99', {id_key: 'tap99id', 'attached-mac': 'tap99mac'}, 1], # A vif port on this bridge not yet configured - [{id_key: 'tap98id', 'attached-mac': 'tap98mac'}, []], + ['tap98', {id_key: 'tap98id', 'attached-mac': 'tap98mac'}, []], # Another vif port on this bridge not yet configured - [{id_key: 'tap97id', 'attached-mac': 'tap97mac'}, + ['tap97', {id_key: 'tap97id', 'attached-mac': 'tap97mac'}, ['set', []]], # Non-vif port on this bridge: - [{}, 2], + ['bogus', {}, 2], ] # Each element is a tuple of (expected mock call, return_value) expected_calls_and_values = [ - (mock.call(["ovs-vsctl", self.TO, "list-ports", self.BR_NAME], - root_helper=self.root_helper), - 'tap99\ntun22'), - (mock.call(["ovs-vsctl", self.TO, "--format=json", - "--", "--columns=external_ids,ofport", '--if-exists', - "list", "Interface", 'tap99', 'tun22'], - root_helper=self.root_helper), + (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) @@ -687,9 +586,7 @@ class OVS_Lib_Test(base.BaseTestCase): def test_get_vif_ports_list_ports_error(self): expected_calls_and_values = [ - (mock.call(["ovs-vsctl", self.TO, "list-ports", self.BR_NAME], - root_helper=self.root_helper), - RuntimeError()), + (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) @@ -697,9 +594,7 @@ class OVS_Lib_Test(base.BaseTestCase): def test_get_vif_port_set_list_ports_error(self): expected_calls_and_values = [ - (mock.call(["ovs-vsctl", self.TO, "list-ports", self.BR_NAME], - root_helper=self.root_helper), - RuntimeError()), + (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) @@ -707,14 +602,10 @@ class OVS_Lib_Test(base.BaseTestCase): def test_get_vif_port_set_list_interface_error(self): expected_calls_and_values = [ - (mock.call(["ovs-vsctl", self.TO, "list-ports", self.BR_NAME], - root_helper=self.root_helper), - 'tap99\n'), - (mock.call(["ovs-vsctl", self.TO, "--format=json", - "--", "--columns=external_ids,ofport", '--if-exists', - "list", "Interface", "tap99"], - root_helper=self.root_helper), - RuntimeError()), + (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) @@ -732,13 +623,9 @@ class OVS_Lib_Test(base.BaseTestCase): # Each element is a tuple of (expected mock call, return_value) expected_calls_and_values = [ - (mock.call(["ovs-vsctl", self.TO, "list-ports", self.BR_NAME], - root_helper=self.root_helper), - '\n'.join((iface for iface, tag in data))), - (mock.call(["ovs-vsctl", self.TO, "--format=json", - "--", "--columns=name,tag", - "list", "Port"], - root_helper=self.root_helper), + (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) @@ -756,40 +643,30 @@ class OVS_Lib_Test(base.BaseTestCase): def test_clear_db_attribute(self): pname = "tap77" self.br.clear_db_attribute("Port", pname, "tag") - self.execute.assert_called_once_with( - ["ovs-vsctl", self.TO, "clear", "Port", pname, "tag"], - root_helper=self.root_helper) + self._verify_vsctl_mock("clear", "Port", pname, "tag") def _test_iface_to_br(self, exp_timeout=None): iface = 'tap0' br = 'br-int' - root_helper = 'sudo' if exp_timeout: self.br.vsctl_timeout = exp_timeout self.execute.return_value = 'br-int' - exp_timeout_str = self._build_timeout_opt(exp_timeout) self.assertEqual(self.br.get_bridge_for_iface(iface), br) - self.execute.assert_called_once_with( - ["ovs-vsctl", exp_timeout_str, "iface-to-br", iface], - root_helper=root_helper) + 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 - cfg.CONF.set_override('ovs_vsctl_timeout', new_timeout) self._test_iface_to_br(new_timeout) def test_iface_to_br_handles_ovs_vsctl_exception(self): iface = 'tap0' - root_helper = 'sudo' self.execute.side_effect = Exception self.assertIsNone(self.br.get_bridge_for_iface(iface)) - self.execute.assert_called_once_with( - ["ovs-vsctl", self.TO, "iface-to-br", iface], - root_helper=root_helper) + self._verify_vsctl_mock("iface-to-br", iface) def test_delete_all_ports(self): with mock.patch.object(self.br, 'get_port_name_list', @@ -816,9 +693,7 @@ class OVS_Lib_Test(base.BaseTestCase): def test_delete_neutron_ports_list_error(self): expected_calls_and_values = [ - (mock.call(["ovs-vsctl", self.TO, "list-ports", self.BR_NAME], - root_helper=self.root_helper), - RuntimeError()), + (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) @@ -826,23 +701,17 @@ class OVS_Lib_Test(base.BaseTestCase): def _test_get_bridges(self, exp_timeout=None): bridges = ['br-int', 'br-ex'] - root_helper = 'sudo' if exp_timeout: self.br.vsctl_timeout = exp_timeout - self.execute.return_value = 'br-int\nbr-ex\n' - timeout_str = self._build_timeout_opt(exp_timeout) + self.execute.return_value = 'br-int\\nbr-ex\n' self.assertEqual(self.br.get_bridges(), bridges) - self.execute.assert_called_once_with( - ["ovs-vsctl", timeout_str, "list-br"], - root_helper=root_helper) + self._verify_vsctl_mock("list-br") def test_get_bridges(self): self._test_get_bridges() def test_get_bridges_not_default_timeout(self): - new_timeout = 5 - cfg.CONF.set_override('ovs_vsctl_timeout', new_timeout) - self._test_get_bridges(new_timeout) + self._test_get_bridges(5) def test_get_local_port_mac_succeeds(self): with mock.patch('neutron.agent.linux.ip_lib.IpLinkCommand', @@ -861,11 +730,10 @@ class OVS_Lib_Test(base.BaseTestCase): # Each element is a tuple of (expected mock call, return_value) expected_calls_and_values = [ - (mock.call(["ovs-vsctl", self.TO, "--format=json", - "--", "--columns=external_ids,name,ofport", - "find", "Interface", - 'external_ids:iface-id="%s"' % iface_id], - root_helper=self.root_helper), + (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: @@ -878,10 +746,8 @@ class OVS_Lib_Test(base.BaseTestCase): expected_calls_and_values.extend(extra_calls_and_values) expected_calls_and_values.append( - (mock.call(["ovs-vsctl", self.TO, - "iface-to-br", data[-1][headings.index('name')]], - root_helper=self.root_helper), - br_name)) + (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) @@ -890,7 +756,7 @@ class OVS_Lib_Test(base.BaseTestCase): def _assert_vif_port(self, vif_port, ofport=None, mac=None): if not ofport or ofport == -1 or not mac: - self.assertIsNone(vif_port) + self.assertIsNone(vif_port, "Got %s" % vif_port) return self.assertEqual('tap99id', vif_port.vif_id) self.assertEqual(mac, vif_port.vif_mac) @@ -899,11 +765,10 @@ class OVS_Lib_Test(base.BaseTestCase): def _test_get_vif_port_by_id_with_data(self, ofport=None, mac=None): external_ids = [["iface-id", "tap99id"], - ["iface-status", "active"]] - if mac: - external_ids.append(["attached-mac", mac]) + ["iface-status", "active"], + ["attached-mac", mac]] data = [[["map", external_ids], "tap99", - ofport if ofport else '["set",[]]']] + ofport if ofport else ["set", []]]] vif_port = self._test_get_vif_port_by_id('tap99id', data) self._assert_vif_port(vif_port, ofport, mac) @@ -918,9 +783,6 @@ class OVS_Lib_Test(base.BaseTestCase): 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_mac(self): - self._test_get_vif_port_by_id_with_data(ofport=1) - def test_get_vif_by_port_id_with_no_data(self): self.assertIsNone(self._test_get_vif_port_by_id('whatever', [])) @@ -938,13 +800,10 @@ class OVS_Lib_Test(base.BaseTestCase): data = [[["map", external_ids], "dummytap", 1], [["map", external_ids], "tap99", 1337]] extra_calls_and_values = [ - (mock.call(["ovs-vsctl", self.TO, - "iface-to-br", "dummytap"], - root_helper=self.root_helper), - "br-ext")] + (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) + '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") diff --git a/neutron/tests/unit/ibm/test_sdnve_agent.py b/neutron/tests/unit/ibm/test_sdnve_agent.py index c701bc4a9..83fcb9ab4 100644 --- a/neutron/tests/unit/ibm/test_sdnve_agent.py +++ b/neutron/tests/unit/ibm/test_sdnve_agent.py @@ -104,7 +104,7 @@ class TestSdnveNeutronAgent(base.BaseTestCase): def test_get_info(self): with mock.patch.object(self.agent.int_br, - 'run_vsctl') as run_vsctl_func: + 'set_controller') as set_controller_func: kwargs = {} self.agent.info_update('dummy', **kwargs) - self.assertFalse(run_vsctl_func.called) + self.assertFalse(set_controller_func.called) diff --git a/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py b/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py index 81af685fe..24cc2893c 100644 --- a/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py +++ b/neutron/tests/unit/ofagent/test_ofa_neutron_agent.py @@ -736,7 +736,7 @@ class TestOFANeutronAgent(ofa_test_base.OFAAgentTestBase): def test__setup_tunnel_port_error_negative(self): with contextlib.nested( mock.patch.object(self.agent.int_br, 'add_tunnel_port', - return_value='-1'), + return_value=ovs_lib.INVALID_OFPORT), mock.patch.object(self.mod_agent.LOG, 'error') ) as (add_tunnel_port_fn, log_error_fn): ofport = self.agent._setup_tunnel_port( diff --git a/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py b/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py index 473479eac..baa71b1a6 100644 --- a/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py +++ b/neutron/tests/unit/openvswitch/test_ovs_neutron_agent.py @@ -151,7 +151,7 @@ class TestOvsNeutronAgent(base.BaseTestCase): mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.' 'set_db_attribute', return_value=True), mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.' - 'db_get_val', return_value=str(old_local_vlan)), + 'db_get_val', return_value=old_local_vlan), mock.patch.object(self.agent.int_br, 'delete_flows') ) as (set_ovs_db_func, get_ovs_db_func, delete_flows_func): self.agent.port_bound(port, net_uuid, 'local', None, None, @@ -159,7 +159,7 @@ class TestOvsNeutronAgent(base.BaseTestCase): get_ovs_db_func.assert_called_once_with("Port", mock.ANY, "tag") if new_local_vlan != old_local_vlan: set_ovs_db_func.assert_called_once_with( - "Port", mock.ANY, "tag", str(new_local_vlan)) + "Port", mock.ANY, "tag", new_local_vlan) if ofport != -1: delete_flows_func.assert_called_once_with(in_port=port.ofport) else: @@ -213,7 +213,7 @@ class TestOvsNeutronAgent(base.BaseTestCase): with contextlib.nested( mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.' 'db_get_val', - return_value=str(self._old_local_vlan)), + return_value=self._old_local_vlan), mock.patch.object( self.agent.dvr_agent.plugin_rpc, 'get_subnet_for_dvr', return_value={'gateway_ip': '1.1.1.1', @@ -255,7 +255,7 @@ class TestOvsNeutronAgent(base.BaseTestCase): with contextlib.nested( mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.' 'db_get_val', - return_value=str(self._old_local_vlan)), + return_value=self._old_local_vlan), mock.patch.object(self.agent.dvr_agent.plugin_rpc, 'get_subnet_for_dvr', return_value={ @@ -380,7 +380,7 @@ class TestOvsNeutronAgent(base.BaseTestCase): with contextlib.nested( mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.' 'db_get_val', - return_value=str(self._old_local_vlan)), + return_value=self._old_local_vlan), mock.patch.object( self.agent.dvr_agent.plugin_rpc, 'get_subnet_for_dvr', return_value={'gateway_ip': '1.1.1.1', @@ -428,7 +428,7 @@ class TestOvsNeutronAgent(base.BaseTestCase): with contextlib.nested( mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.' 'db_get_val', - return_value=str(self._old_local_vlan)), + return_value=self._old_local_vlan), mock.patch.object( self.agent.dvr_agent.plugin_rpc, 'get_subnet_for_dvr', return_value={'gateway_ip': gateway_ip, @@ -525,7 +525,7 @@ class TestOvsNeutronAgent(base.BaseTestCase): with contextlib.nested( mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.' 'db_get_val', - return_value=str(self._old_local_vlan)), + return_value=self._old_local_vlan), mock.patch.object( self.agent.dvr_agent.plugin_rpc, 'get_subnet_for_dvr', return_value={'gateway_ip': gateway_ip, @@ -616,7 +616,7 @@ class TestOvsNeutronAgent(base.BaseTestCase): with contextlib.nested( mock.patch('neutron.agent.linux.ovs_lib.OVSBridge.' 'db_get_val', - return_value=str(self._old_local_vlan)), + return_value=self._old_local_vlan), mock.patch.object( self.agent.dvr_agent.plugin_rpc, 'get_subnet_for_dvr', return_value={'gateway_ip': '1.1.1.1', @@ -757,7 +757,7 @@ class TestOvsNeutronAgent(base.BaseTestCase): self.assertFalse(add_flow_func.called) else: set_ovs_db_func.assert_called_once_with( - "Port", mock.ANY, "tag", str(ovs_neutron_agent.DEAD_VLAN_TAG)) + "Port", mock.ANY, "tag", ovs_neutron_agent.DEAD_VLAN_TAG) add_flow_func.assert_called_once_with( priority=2, in_port=port.ofport, actions="drop") @@ -1504,7 +1504,7 @@ class TestOvsNeutronAgent(base.BaseTestCase): def test_setup_tunnel_port_error_negative_df_disabled(self): with contextlib.nested( mock.patch.object(self.agent.tun_br, 'add_tunnel_port', - return_value='-1'), + return_value=ovs_lib.INVALID_OFPORT), mock.patch.object(ovs_neutron_agent.LOG, 'error') ) as (add_tunnel_port_fn, log_error_fn): self.agent.dont_fragment = False diff --git a/neutron/tests/unit/openvswitch/test_ovs_tunnel.py b/neutron/tests/unit/openvswitch/test_ovs_tunnel.py index e729db5ed..ec6a1d9f0 100644 --- a/neutron/tests/unit/openvswitch/test_ovs_tunnel.py +++ b/neutron/tests/unit/openvswitch/test_ovs_tunnel.py @@ -441,7 +441,7 @@ class TunnelTest(base.BaseTestCase): self.mock_int_bridge_expected += [ mock.call.db_get_val('Port', VIF_PORT.port_name, 'tag'), mock.call.set_db_attribute('Port', VIF_PORT.port_name, - 'tag', str(LVM.vlan)), + 'tag', LVM.vlan), mock.call.delete_flows(in_port=VIF_PORT.ofport) ] diff --git a/neutron/tests/unit/test_linux_interface.py b/neutron/tests/unit/test_linux_interface.py index 78b098e88..e1947a803 100644 --- a/neutron/tests/unit/test_linux_interface.py +++ b/neutron/tests/unit/test_linux_interface.py @@ -224,9 +224,10 @@ class TestOVSInterfaceDriver(TestBase): replace.assert_called_once_with( 'tap0', ('type', 'internal'), - ('external-ids:iface-id', 'port-1234'), - ('external-ids:iface-status', 'active'), - ('external-ids:attached-mac', 'aa:bb:cc:dd:ee:ff')) + ('external_ids', { + 'iface-id': 'port-1234', + 'iface-status': 'active', + 'attached-mac': 'aa:bb:cc:dd:ee:ff'})) expected = [mock.call('sudo'), mock.call().device('tap0'), @@ -305,9 +306,10 @@ class TestOVSInterfaceDriverWithVeth(TestOVSInterfaceDriver): prefix=prefix) replace.assert_called_once_with( 'tap0', - ('external-ids:iface-id', 'port-1234'), - ('external-ids:iface-status', 'active'), - ('external-ids:attached-mac', 'aa:bb:cc:dd:ee:ff')) + ('external_ids', { + 'iface-id': 'port-1234', + 'iface-status': 'active', + 'attached-mac': 'aa:bb:cc:dd:ee:ff'})) ns_dev.assert_has_calls( [mock.call.link.set_address('aa:bb:cc:dd:ee:ff')]) -- 2.45.2