From 1546d349b1656bc98286af47f1e2748b76743abc Mon Sep 17 00:00:00 2001 From: Rodolfo Alonso Hernandez Date: Fri, 21 Sep 2018 10:00:35 +0100 Subject: [PATCH] Add native implementation OVSDB API Added native implementation OVSDB API. Both APIs may be enabled via configuration file. The default one is the CLI vsctl. A new configuration variable, ``ovsdb_connection``, is added to define the connection string for the OVSDB backend. Added functional tests to vif_plug_ovs. This commit also includes the base functions to execute functional tests and a set of them to test the OVSDB APIs: native and ovs-vsctl. Closes-Bug: #1666917 Change-Id: I86fbf8c67572e51889eb091d7bff7f9350b52481 --- .zuul.yaml | 12 +- lower-constraints.txt | 1 + .../Debian.yaml | 3 + .../Gentoo.yaml | 3 + .../RedHat.yaml | 3 + .../Suse.yaml | 3 + .../pre.yaml | 23 +++ .../add-ovsdb-native-322fffb49c91503d.yaml | 13 ++ test-requirements.txt | 1 + vif_plug_ovs/ovs.py | 12 +- vif_plug_ovs/ovsdb/api.py | 3 +- vif_plug_ovs/ovsdb/impl_idl.py | 51 ++++++ vif_plug_ovs/tests/functional/__init__.py | 0 vif_plug_ovs/tests/functional/base.py | 140 +++++++++++++++++ .../tests/functional/ovsdb/__init__.py | 0 .../tests/functional/ovsdb/test_ovsdb_lib.py | 145 ++++++++++++++++++ 16 files changed, 402 insertions(+), 11 deletions(-) create mode 100644 playbooks/openstack-tox-functional-ovs-with-sudo/Debian.yaml create mode 100644 playbooks/openstack-tox-functional-ovs-with-sudo/Gentoo.yaml create mode 100644 playbooks/openstack-tox-functional-ovs-with-sudo/RedHat.yaml create mode 100644 playbooks/openstack-tox-functional-ovs-with-sudo/Suse.yaml create mode 100644 playbooks/openstack-tox-functional-ovs-with-sudo/pre.yaml create mode 100644 releasenotes/notes/add-ovsdb-native-322fffb49c91503d.yaml create mode 100644 vif_plug_ovs/ovsdb/impl_idl.py create mode 100644 vif_plug_ovs/tests/functional/__init__.py create mode 100644 vif_plug_ovs/tests/functional/base.py create mode 100644 vif_plug_ovs/tests/functional/ovsdb/__init__.py create mode 100644 vif_plug_ovs/tests/functional/ovsdb/test_ovsdb_lib.py diff --git a/.zuul.yaml b/.zuul.yaml index bcadbc3c..58a8d2bc 100644 --- a/.zuul.yaml +++ b/.zuul.yaml @@ -10,6 +10,14 @@ - openstack/os-vif - openstack/tempest +- job: + name: openstack-tox-functional-ovs-with-sudo + parent: openstack-tox-functional-with-sudo + required-projects: + - git.openstack.org/openstack-dev/devstack + pre-run: playbooks/openstack-tox-functional-ovs-with-sudo/pre.yaml + timeout: 600 + - project: templates: - check-requirements @@ -23,9 +31,9 @@ jobs: - kuryr-kubernetes-tempest-daemon-octavia: voting: false - - openstack-tox-functional-with-sudo + - openstack-tox-functional-ovs-with-sudo - os-vif-ovs gate: jobs: - - openstack-tox-functional-with-sudo + - openstack-tox-functional-ovs-with-sudo - os-vif-ovs diff --git a/lower-constraints.txt b/lower-constraints.txt index f8ed37c8..21229119 100644 --- a/lower-constraints.txt +++ b/lower-constraints.txt @@ -49,6 +49,7 @@ oslo.service==1.30.0 oslo.utils==3.36.0 oslo.versionedobjects==1.28.0 oslotest==1.10.0 +ovs==2.9.2 ovsdbapp==0.12.1 Paste==2.0.3 PasteDeploy==1.5.2 diff --git a/playbooks/openstack-tox-functional-ovs-with-sudo/Debian.yaml b/playbooks/openstack-tox-functional-ovs-with-sudo/Debian.yaml new file mode 100644 index 00000000..6aa54ded --- /dev/null +++ b/playbooks/openstack-tox-functional-ovs-with-sudo/Debian.yaml @@ -0,0 +1,3 @@ +--- +ovs_package: "openvswitch-switch" +ovs_service: "openvswitch-switch" diff --git a/playbooks/openstack-tox-functional-ovs-with-sudo/Gentoo.yaml b/playbooks/openstack-tox-functional-ovs-with-sudo/Gentoo.yaml new file mode 100644 index 00000000..107b077b --- /dev/null +++ b/playbooks/openstack-tox-functional-ovs-with-sudo/Gentoo.yaml @@ -0,0 +1,3 @@ +--- +ovs_package: "net-misc/openvswitch" +ovs_service: "ovs-vswitchd" diff --git a/playbooks/openstack-tox-functional-ovs-with-sudo/RedHat.yaml b/playbooks/openstack-tox-functional-ovs-with-sudo/RedHat.yaml new file mode 100644 index 00000000..19618923 --- /dev/null +++ b/playbooks/openstack-tox-functional-ovs-with-sudo/RedHat.yaml @@ -0,0 +1,3 @@ +--- +ovs_package: "openvswitch" +ovs_service: "openvswitch" diff --git a/playbooks/openstack-tox-functional-ovs-with-sudo/Suse.yaml b/playbooks/openstack-tox-functional-ovs-with-sudo/Suse.yaml new file mode 100644 index 00000000..19618923 --- /dev/null +++ b/playbooks/openstack-tox-functional-ovs-with-sudo/Suse.yaml @@ -0,0 +1,3 @@ +--- +ovs_package: "openvswitch" +ovs_service: "openvswitch" diff --git a/playbooks/openstack-tox-functional-ovs-with-sudo/pre.yaml b/playbooks/openstack-tox-functional-ovs-with-sudo/pre.yaml new file mode 100644 index 00000000..ca3bbe97 --- /dev/null +++ b/playbooks/openstack-tox-functional-ovs-with-sudo/pre.yaml @@ -0,0 +1,23 @@ +- hosts: all + name: Functional tests pre-tasks + tasks: + - name: Include OS-specific variables + include_vars: "{{ item }}" + with_first_found: + - "{{ ansible_distribution }}.yaml" + - "{{ ansible_os_family }}.yaml" + + - name: Install Open vSwitch + become: yes + package: + name: "{{ ovs_package }}" + state: present + register: ovs_installed + + - name: Start Open vSwitch + become: yes + service: + name: "{{ ovs_service }}" + state: started + enabled: yes + register: ovs_running diff --git a/releasenotes/notes/add-ovsdb-native-322fffb49c91503d.yaml b/releasenotes/notes/add-ovsdb-native-322fffb49c91503d.yaml new file mode 100644 index 00000000..3b8714cb --- /dev/null +++ b/releasenotes/notes/add-ovsdb-native-322fffb49c91503d.yaml @@ -0,0 +1,13 @@ +--- +features: + - | + Added native implementation of OVSDB API in ``vif_plug_ovs``. Both + ``vsctl`` and ``native`` APIs could be selected by setting the + configuration variable ``ovsdb_interface``. + A new configuration variable, ``ovsdb_connection``, is added. This variable + defines the connection string for the OVSDB backend. +other: + - | + Changed default value of ``ovsdb_connection`` to "tcp:127.0.0.1:6640", to + match the default value set in Neutron project. This connection string is + needed by OVSDB native interface. diff --git a/test-requirements.txt b/test-requirements.txt index 05e50c6f..da7c80a0 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -6,6 +6,7 @@ hacking>=1.1.0,<1.2.0 # Apache-2.0 coverage!=4.4,>=4.0 # Apache-2.0 python-subunit>=1.0.0 # Apache-2.0/BSD oslotest>=1.10.0 # Apache-2.0 +ovs>=2.9.2 stestr>=1.0.0 # Apache-2.0 testrepository>=0.0.18 # Apache-2.0/BSD testscenarios>=0.4 # Apache-2.0/BSD diff --git a/vif_plug_ovs/ovs.py b/vif_plug_ovs/ovs.py index c605309c..8f90510c 100644 --- a/vif_plug_ovs/ovs.py +++ b/vif_plug_ovs/ovs.py @@ -57,13 +57,11 @@ class OvsPlugin(plugin.PluginBase): 'forever.', deprecated_group="DEFAULT"), cfg.StrOpt('ovsdb_connection', - default=None, - help='The TCP socket connection for communicating with ' - 'OVS. "None" (default) is for UNIX domain socket ' - 'connection. If set to "tcp:IP:PORT" eg ' - 'tcp:127.0.0.1:6640, ovs-vsctl commands will use the ' - 'tcp:IP:PORT parameter for communicating with OVSDB over ' - 'a TCP socket.'), + default='tcp:127.0.0.1:6640', + help='The connection string for the OVSDB backend. ' + 'When executing commands using the native or vsctl ' + 'ovsdb interface drivers this config option defines ' + 'the ovsdb endpoint used.'), cfg.StrOpt('ovsdb_interface', choices=list(ovsdb_api.interface_map), default='vsctl', diff --git a/vif_plug_ovs/ovsdb/api.py b/vif_plug_ovs/ovsdb/api.py index 5f3b1983..6e30191b 100644 --- a/vif_plug_ovs/ovsdb/api.py +++ b/vif_plug_ovs/ovsdb/api.py @@ -18,8 +18,7 @@ import six interface_map = { 'vsctl': 'vif_plug_ovs.ovsdb.impl_vsctl', - # NOTE(ralonsoh): to be implemented in following patches. - # 'native': 'vif_plug_ovs.ovsdb.impl_idl', + 'native': 'vif_plug_ovs.ovsdb.impl_idl', } diff --git a/vif_plug_ovs/ovsdb/impl_idl.py b/vif_plug_ovs/ovsdb/impl_idl.py new file mode 100644 index 00000000..6dc2ffe1 --- /dev/null +++ b/vif_plug_ovs/ovsdb/impl_idl.py @@ -0,0 +1,51 @@ +# 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. + +from ovs.db import idl +from ovsdbapp.backend.ovs_idl import connection +from ovsdbapp.backend.ovs_idl import idlutils +from ovsdbapp.backend.ovs_idl import vlog +from ovsdbapp.schema.open_vswitch import impl_idl + +from vif_plug_ovs.ovsdb import api + + +def idl_factory(config): + conn = config.connection + schema_name = 'Open_vSwitch' + helper = idlutils.get_schema_helper(conn, schema_name) + helper.register_all() + return idl.Idl(conn, helper) + + +def api_factory(config): + conn = connection.Connection( + idl=idl_factory(config), + timeout=config.timeout) + return NeutronOvsdbIdl(conn) + + +class NeutronOvsdbIdl(impl_idl.OvsdbIdl, api.ImplAPI): + """IDL interface for OVS database back-end + + This class provides an OVSDB IDL (Open vSwitch Database Interface + Definition Language) interface to the OVS back-end. + """ + def __init__(self, conn): + vlog.use_python_logger() + super(NeutronOvsdbIdl, self).__init__(conn) + + def _get_table_columns(self, table): + return list(self.tables[table].columns) + + def has_table_column(self, table, column): + return column in self._get_table_columns(table) diff --git a/vif_plug_ovs/tests/functional/__init__.py b/vif_plug_ovs/tests/functional/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/vif_plug_ovs/tests/functional/base.py b/vif_plug_ovs/tests/functional/base.py new file mode 100644 index 00000000..c4b338ac --- /dev/null +++ b/vif_plug_ovs/tests/functional/base.py @@ -0,0 +1,140 @@ +# Derived from: neutron/tests/functional/base.py +# neutron/tests/base.py +# +# 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 +import functools +import inspect +import os +import six +import sys + +import eventlet +from os_vif import version as osvif_version +from oslo_config import cfg +from oslo_log import log as logging +from oslo_utils import fileutils +from oslotest import base + + +CONF = cfg.CONF +LOG = logging.getLogger(__name__) + + +def _get_test_log_path(): + return os.environ.get('OS_LOG_PATH', '/tmp') + + +# This is the directory from which infra fetches log files for functional tests +DEFAULT_LOG_DIR = os.path.join(_get_test_log_path(), 'osvif-functional-logs') + + +def wait_until_true(predicate, timeout=15, sleep=1): + """Wait until callable predicate is evaluated as True + + :param predicate: Callable deciding whether waiting should continue. + Best practice is to instantiate predicate with + ``functools.partial()``. + :param timeout: Timeout in seconds how long should function wait. + :param sleep: Polling interval for results in seconds. + :return: True if the predicate is evaluated as True within the timeout, + False in case of timeout evaluating the predicate. + """ + try: + with eventlet.Timeout(timeout): + while not predicate(): + eventlet.sleep(sleep) + except eventlet.Timeout: + return False + return True + + +class _CatchTimeoutMetaclass(abc.ABCMeta): + def __init__(cls, name, bases, dct): + super(_CatchTimeoutMetaclass, cls).__init__(name, bases, dct) + for name, method in inspect.getmembers( + # NOTE(ihrachys): we should use isroutine because it will catch + # both unbound methods (python2) and functions (python3) + cls, predicate=inspect.isroutine): + if name.startswith('test_'): + setattr(cls, name, cls._catch_timeout(method)) + + @staticmethod + def _catch_timeout(f): + @functools.wraps(f) + def func(self, *args, **kwargs): + try: + return f(self, *args, **kwargs) + except eventlet.Timeout as e: + self.fail('Execution of this test timed out: %s' % e) + return func + + +def setup_logging(): + """Sets up the logging options for a log with supplied name.""" + product_name = "vif_plug_ovs" + logging.setup(cfg.CONF, product_name) + LOG.info("Logging enabled!") + LOG.info("%(prog)s version %(version)s", + {'prog': sys.argv[0], 'version': osvif_version.__version__}) + LOG.debug("command line: %s", " ".join(sys.argv)) + + +def sanitize_log_path(path): + # Sanitize the string so that its log path is shell friendly + replace_map = {' ': '-', '(': '_', ')': '_'} + for s, r in replace_map.items(): + path = path.replace(s, r) + return path + + +@six.add_metaclass(_CatchTimeoutMetaclass) +class BaseFunctionalTestCase(base.BaseTestCase): + """Base class for functional tests. + + Test worker cannot survive eventlet's Timeout exception, which effectively + kills the whole worker, with all test cases scheduled to it. This metaclass + makes all test cases convert Timeout exceptions into unittest friendly + failure mode (self.fail). + """ + def setUp(self): + super(BaseFunctionalTestCase, self).setUp() + logging.register_options(CONF) + setup_logging() + fileutils.ensure_tree(DEFAULT_LOG_DIR, mode=0o755) + log_file = sanitize_log_path( + os.path.join(DEFAULT_LOG_DIR, "%s.txt" % self.id())) + self.flags(log_file=log_file) + privsep_helper = os.path.join( + os.getenv('VIRTUAL_ENV', os.path.dirname(sys.executable)[:-4]), + 'bin', 'privsep-helper') + self.flags( + helper_command=' '.join(['sudo', '-E', privsep_helper]), + group='vif_plug_ovs_privileged') + + def flags(self, **kw): + """Override some configuration values. + + The keyword arguments are the names of configuration options to + override and their values. + + If a group argument is supplied, the overrides are applied to + the specified configuration option group. + + All overrides are automatically cleared at the end of the current + test by the fixtures cleanup process. + """ + group = kw.pop('group', None) + for k, v in kw.items(): + CONF.set_override(k, v, group) diff --git a/vif_plug_ovs/tests/functional/ovsdb/__init__.py b/vif_plug_ovs/tests/functional/ovsdb/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/vif_plug_ovs/tests/functional/ovsdb/test_ovsdb_lib.py b/vif_plug_ovs/tests/functional/ovsdb/test_ovsdb_lib.py new file mode 100644 index 00000000..fa57c5db --- /dev/null +++ b/vif_plug_ovs/tests/functional/ovsdb/test_ovsdb_lib.py @@ -0,0 +1,145 @@ +# 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 mock +import random + +from oslo_concurrency import processutils +from oslo_config import cfg +from oslo_utils import uuidutils +from ovsdbapp.schema.open_vswitch import impl_idl +import testscenarios + +from vif_plug_ovs import constants +from vif_plug_ovs import linux_net +from vif_plug_ovs import ovs +from vif_plug_ovs.ovsdb import ovsdb_lib +from vif_plug_ovs import privsep +from vif_plug_ovs.tests.functional import base + + +CONF = cfg.CONF + + +@privsep.vif_plug.entrypoint +def run_privileged(*full_args): + return processutils.execute(*full_args)[0].rstrip() + + +class TestOVSDBLib(testscenarios.WithScenarios, base.BaseFunctionalTestCase): + + scenarios = [ + ('native', {'interface': 'native'}), + ('vsctl', {'interface': 'vsctl'}) + ] + + def setUp(self): + super(TestOVSDBLib, self).setUp() + run_privileged('ovs-vsctl', 'set-manager', 'ptcp:6640') + + # NOTE: (ralonsoh) load default configuration variables "CONFIG_OPTS" + ovs.OvsPlugin.load('ovs') + self.flags(ovsdb_interface=self.interface, group='os_vif_ovs') + + self.ovs = ovsdb_lib.BaseOVS(CONF.os_vif_ovs) + self._ovsdb = self.ovs.ovsdb + self.brname = ('br' + str(random.randint(1000, 9999)) + '-' + + self.interface) + + # Make sure exceptions pass through by calling do_post_commit directly + mock.patch.object( + impl_idl.OvsVsctlTransaction, 'post_commit', + side_effect=impl_idl.OvsVsctlTransaction.do_post_commit).start() + + def _check_interface(self, port, parameter, expected_value): + def check_value(): + return (self._ovsdb.db_get( + 'Interface', port, parameter).execute() == expected_value) + + self.assertTrue(base.wait_until_true(check_value, timeout=2, + sleep=0.5)) + + def _add_port(self, bridge, port, may_exist=True): + with self._ovsdb.transaction() as txn: + txn.add(self._ovsdb.add_port(bridge, port, may_exist=may_exist)) + txn.add(self._ovsdb.db_set('Interface', port, + ('type', 'internal'))) + self.assertIn(port, self._list_ports_in_bridge(bridge)) + + def _list_ports_in_bridge(self, bridge): + return self._ovsdb.list_ports(bridge).execute() + + def _check_bridge(self, name): + return self._ovsdb.br_exists(name).execute() + + def _add_bridge(self, name, may_exist=True, datapath_type=None): + self._ovsdb.add_br(name, may_exist=may_exist, + datapath_type=datapath_type).execute() + self.assertTrue(self._check_bridge(name)) + + def _del_bridge(self, name): + self._ovsdb.del_br(name).execute() + + def test__set_mtu_request(self): + port_name = 'port1-' + self.interface + self._add_bridge(self.brname) + self.addCleanup(self._del_bridge, self.brname) + self._add_port(self.brname, port_name) + if self.ovs._ovs_supports_mtu_requests(): + self.ovs._set_mtu_request(port_name, 1000) + self._check_interface(port_name, 'mtu', 1000) + self.ovs._set_mtu_request(port_name, 1500) + self._check_interface(port_name, 'mtu', 1500) + else: + self.skipTest('Current version of Open vSwitch does not support ' + '"mtu_request" parameter') + + def test_create_ovs_vif_port(self): + port_name = 'port2-' + self.interface + iface_id = 'iface_id' + mac = 'ca:fe:ca:fe:ca:fe' + instance_id = uuidutils.generate_uuid() + interface_type = constants.OVS_VHOSTUSER_INTERFACE_TYPE + vhost_server_path = '/fake/path' + mtu = 1500 + self._add_bridge(self.brname) + self.addCleanup(self._del_bridge, self.brname) + + self.ovs.create_ovs_vif_port(self.brname, port_name, iface_id, mac, + instance_id, mtu=mtu, + interface_type=interface_type, + vhost_server_path=vhost_server_path) + + expected_external_ids = {'iface-status': 'active', + 'iface-id': iface_id, + 'attached-mac': mac, + 'vm-uuid': instance_id} + self._check_interface(port_name, 'external_ids', expected_external_ids) + self._check_interface(port_name, 'type', interface_type) + expected_vhost_server_path = {'vhost-server-path': vhost_server_path} + self._check_interface(port_name, 'options', expected_vhost_server_path) + + @mock.patch.object(linux_net, 'delete_net_dev') + def test_delete_ovs_vif_port(self, *mock): + port_name = 'port3-' + self.interface + self._add_bridge(self.brname) + self.addCleanup(self._del_bridge, self.brname) + + self._add_port(self.brname, port_name) + self.ovs.delete_ovs_vif_port(self.brname, port_name) + self.assertNotIn(port_name, self._list_ports_in_bridge(self.brname)) + + def test_ensure_ovs_bridge(self): + bridge_name = 'bridge2-' + self.interface + self.ovs.ensure_ovs_bridge(bridge_name, constants.OVS_DATAPATH_SYSTEM) + self.assertTrue(self._check_bridge(bridge_name)) + self.addCleanup(self._del_bridge, bridge_name)