diff --git a/meson.build b/meson.build index 66cda8b5a..53d17623a 100644 --- a/meson.build +++ b/meson.build @@ -71,7 +71,12 @@ test_env = [ ] if get_option('unit_testing') - subdir('tests/ctests') + test('ctests', + find_program('tests/ctests/run.sh'), + workdir: join_paths(meson.current_source_dir(), 'tests/ctests'), + args: [join_paths(meson.current_build_dir(), '_ctestsbuild'), + '-Dparent_current_source_dir=' + meson.current_source_dir(), + '-Dparent_current_build_dir=' + meson.current_build_dir()]) endif #FIXME: exclude doc/env/ diff --git a/netplan_cli/cli/ovs.py b/netplan_cli/cli/ovs.py index 0ba0482b6..04c836c6b 100644 --- a/netplan_cli/cli/ovs.py +++ b/netplan_cli/cli/ovs.py @@ -20,10 +20,9 @@ import subprocess import re -from .utils import systemctl_is_active - -OPENVSWITCH_OVS_VSCTL = '/usr/bin/ovs-vsctl' -OPENVSWITCH_OVSDB_SERVER_UNIT = 'ovsdb-server.service' +OPENVSWITCH_OVS_VSCTL = ( + '/snap/bin/ovs-vsctl' if os.path.exists('/snap/bin/ovs-vsctl') else + '/usr/bin/ovs-vsctl') # Defaults for non-optional settings, as defined here: # http://www.openvswitch.org/ovs-vswitchd.conf.db.5.pdf DEFAULTS = { @@ -43,6 +42,19 @@ class OvsDbServerNotRunning(Exception): pass +def _assert_ovsdb_server_connection(): + """Invoke OPENVSWITCH_OVS_VSCTL, raise OvsDbServerNotRunning on error.""" + try: + # The `get-manager` command is a light weight database operation which + # we use as indication of the client being able to connect. + subprocess.check_call( + [OPENVSWITCH_OVS_VSCTL, '--timeout', '5', 'get-manager']) + except (subprocess.CalledProcessError, OSError): + raise OvsDbServerNotRunning('{} is unable to connect to database, ' + 'ensure ovsdb-server is running' + .format(OPENVSWITCH_OVS_VSCTL)) + + def _del_col(type, iface, column, value): """Cleanup values from a column (i.e. "column=value")""" default = DEFAULTS.get(column) @@ -125,8 +137,7 @@ def apply_ovs_cleanup(config_manager, ovs_old, ovs_current): # pragma: nocover Also filter for individual settings tagged netplan/[/. */ +#include #include #include @@ -29,6 +30,48 @@ #include "util.h" #include "util-internal.h" +#ifndef PREFIX +#define PREFIX "/usr/" +#endif + +#ifndef BINDIR +#define BINDIR PREFIX"bin" +#endif + +#ifndef SNAPBINDIR +#define SNAPBINDIR "/snap/bin" +#endif + +#define OPENVSWITCH_OVS_VSCTL "ovs-vsctl" + +static char netplan_openvswitch_ovs_vsctl_path[ + MAX(sizeof BINDIR + 1 + sizeof OPENVSWITCH_OVS_VSCTL, + sizeof SNAPBINDIR + 1 + sizeof OPENVSWITCH_OVS_VSCTL)]; + +void +netplan_openvswitch_vsctl__(void) +{ + if (g_file_test(SNAPBINDIR"/"OPENVSWITCH_OVS_VSCTL, G_FILE_TEST_EXISTS)) { + snprintf(netplan_openvswitch_ovs_vsctl_path, + sizeof netplan_openvswitch_ovs_vsctl_path, + SNAPBINDIR"/"OPENVSWITCH_OVS_VSCTL); + } else { + snprintf(netplan_openvswitch_ovs_vsctl_path, + sizeof netplan_openvswitch_ovs_vsctl_path, + BINDIR"/"OPENVSWITCH_OVS_VSCTL); + } + +} + +const char * +netplan_openvswitch_ovs_vsctl(void) +{ + if (!*netplan_openvswitch_ovs_vsctl_path) { + netplan_openvswitch_vsctl__(); + } + return (const char *) &netplan_openvswitch_ovs_vsctl_path; +} + static gboolean write_ovs_systemd_unit(const char* id, const GString* cmds, const char* rootdir, gboolean physical, gboolean cleanup, const char* dependency, GError** error) { @@ -51,7 +94,8 @@ write_ovs_systemd_unit(const char* id, const GString* cmds, const char* rootdir, g_string_append_printf(s, "After=netplan-ovs-cleanup.service\n"); } else { /* The netplan-ovs-cleanup unit shall not run on systems where Open vSwitch is not installed. */ - g_string_append(s, "ConditionFileIsExecutable=" OPENVSWITCH_OVS_VSCTL "\n"); + g_string_append_printf(s, "ConditionFileIsExecutable=%s\n", + netplan_openvswitch_ovs_vsctl()); } g_string_append(s, "Before=network.target\nWants=network.target\n"); if (dependency) { @@ -127,7 +171,8 @@ write_ovs_tag_setting(const gchar* id, const char* type, const char* col, const if (key) g_string_append_printf(s, "/%s", key); g_string_append_printf(s, "=%s", clean_value); - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set %s %s %s", type, id, s->str); + append_systemd_cmd(cmds, "%s set %s %s %s", + netplan_openvswitch_ovs_vsctl(), type, id, s->str); g_string_free(s, TRUE); } @@ -142,8 +187,9 @@ write_ovs_additional_data(GHashTable *data, const char* type, const gchar* id, G while (g_hash_table_iter_next(&iter, (gpointer) &key, (gpointer) &value)) { /* XXX: we need to check what happens when an invalid key=value pair gets supplied here. We might want to handle this somehow. */ - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set %s %s %s:%s=%s", - type, id, setting, key, value); + append_systemd_cmd(cmds, "%s set %s %s %s:%s=%s", + netplan_openvswitch_ovs_vsctl(), type, id, setting, + key, value); write_ovs_tag_setting(id, type, setting, key, value, cmds); } } @@ -175,8 +221,9 @@ write_ovs_bond_interfaces(const NetplanState* np_state, const NetplanNetDefiniti return NULL; } - s = g_string_new(OPENVSWITCH_OVS_VSCTL " --may-exist add-bond"); - g_string_append_printf(s, " %s %s", def->bridge, def->id); + s = g_string_new(NULL); + g_string_printf(s, "%s --may-exist add-bond %s %s", + netplan_openvswitch_ovs_vsctl(), def->bridge, def->id); g_hash_table_iter_init(&iter, np_state->netdefs); while (g_hash_table_iter_next(&iter, (gpointer) &key, (gpointer) &tmp_nd)) { @@ -202,8 +249,8 @@ static void write_ovs_tag_netplan(const gchar* id, const char* type, GString* cmds) { /* Mark this bridge/port/interface as created by netplan */ - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set %s %s external-ids:netplan=true", - type, id); + append_systemd_cmd(cmds, "%s set %s %s external-ids:netplan=true", + netplan_openvswitch_ovs_vsctl(), type, id); } static gboolean @@ -216,7 +263,8 @@ write_ovs_bond_mode(const NetplanNetDefinition* def, GString* cmds, GError** err !strcmp(def->bond_params.mode, "balance-tcp") || !strcmp(def->bond_params.mode, "balance-slb")) { value = def->bond_params.mode; - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set Port %s bond_mode=%s", def->id, value); + append_systemd_cmd(cmds, "%s set Port %s bond_mode=%s", + netplan_openvswitch_ovs_vsctl(), def->id, value); write_ovs_tag_setting(def->id, "Port", "bond_mode", NULL, value, cmds); } else { g_set_error(error, NETPLAN_BACKEND_ERROR, NETPLAN_ERROR_VALIDATION, "%s: bond mode '%s' not supported by Open vSwitch\n", @@ -233,7 +281,8 @@ write_ovs_bridge_interfaces(const NetplanState* np_state, const NetplanNetDefini GHashTableIter iter; gchar* key; - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " --may-exist add-br %s", def->id); + append_systemd_cmd(cmds, "%s --may-exist add-br %s", + netplan_openvswitch_ovs_vsctl(), def->id); g_hash_table_iter_init(&iter, np_state->netdefs); while (g_hash_table_iter_next(&iter, (gpointer) &key, (gpointer) &tmp_nd)) { @@ -243,8 +292,9 @@ write_ovs_bridge_interfaces(const NetplanState* np_state, const NetplanNetDefini GString * patch_ports = g_string_new(""); if (tmp_nd->type == NETPLAN_DEF_TYPE_PORT) setup_patch_port(patch_ports, tmp_nd); - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " --may-exist add-port %s %s%s", - def->id, tmp_nd->id, patch_ports->str); + append_systemd_cmd(cmds, "%s --may-exist add-port %s %s%s", + netplan_openvswitch_ovs_vsctl(), def->id, + tmp_nd->id, patch_ports->str); g_string_free(patch_ports, TRUE); } } @@ -259,7 +309,8 @@ write_ovs_protocols(const NetplanOVSSettings* ovs_settings, const gchar* bridge, for (unsigned i = 1; i < ovs_settings->protocols->len; ++i) g_string_append_printf(s, ",%s", g_array_index(ovs_settings->protocols, char*, i)); - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set Bridge %s protocols=%s", bridge, s->str); + append_systemd_cmd(cmds, "%s set Bridge %s protocols=%s", + netplan_openvswitch_ovs_vsctl(), bridge, s->str); write_ovs_tag_setting(bridge, "Bridge", "protocols", NULL, s->str, cmds); g_string_free(s, TRUE); } @@ -301,7 +352,8 @@ write_ovs_bridge_controller_targets(const NetplanOVSSettings* settings, const Ne } g_string_erase(s, s->len-1, 1); - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set-controller %s %s", bridge, s->str); + append_systemd_cmd(cmds, "%s set-controller %s %s", + netplan_openvswitch_ovs_vsctl(), bridge, s->str); write_ovs_tag_setting(bridge, "Bridge", "global", "set-controller", s->str, cmds); cleanup: @@ -340,7 +392,9 @@ netplan_netdef_write_ovs(const NetplanState* np_state, const NetplanNetDefinitio write_ovs_tag_netplan(def->id, type, cmds); /* Set LACP mode, default to "off" */ value = def->ovs_settings.lacp? def->ovs_settings.lacp : "off"; - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set Port %s lacp=%s", def->id, value); + append_systemd_cmd(cmds, "%s set Port %s lacp=%s", + netplan_openvswitch_ovs_vsctl(), + def->id, value); write_ovs_tag_setting(def->id, type, "lacp", NULL, value, cmds); if (def->bond_params.mode && !write_ovs_bond_mode(def, cmds, error)) return FALSE; @@ -351,15 +405,23 @@ netplan_netdef_write_ovs(const NetplanState* np_state, const NetplanNetDefinitio write_ovs_tag_netplan(def->id, type, cmds); /* Set fail-mode, default to "standalone" */ value = def->ovs_settings.fail_mode? def->ovs_settings.fail_mode : "standalone"; - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set-fail-mode %s %s", def->id, value); + append_systemd_cmd(cmds, "%s set-fail-mode %s %s", + netplan_openvswitch_ovs_vsctl(), + def->id, value); write_ovs_tag_setting(def->id, type, "global", "set-fail-mode", value, cmds); /* Enable/disable mcast-snooping */ value = def->ovs_settings.mcast_snooping? "true" : "false"; - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set Bridge %s mcast_snooping_enable=%s", def->id, value); + append_systemd_cmd(cmds, + "%s set Bridge %s mcast_snooping_enable=%s", + netplan_openvswitch_ovs_vsctl(), + def->id, value); write_ovs_tag_setting(def->id, type, "mcast_snooping_enable", NULL, value, cmds); /* Enable/disable rstp */ value = def->ovs_settings.rstp? "true" : "false"; - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set Bridge %s rstp_enable=%s", def->id, value); + append_systemd_cmd(cmds, + "%s set Bridge %s rstp_enable=%s", + netplan_openvswitch_ovs_vsctl(), + def->id, value); write_ovs_tag_setting(def->id, type, "rstp_enable", NULL, value, cmds); /* Set protocols */ if (def->ovs_settings.protocols && def->ovs_settings.protocols->len > 0) @@ -374,7 +436,11 @@ netplan_netdef_write_ovs(const NetplanState* np_state, const NetplanNetDefinitio /* Set controller connection mode, only applicable if at least one controller target address was set */ if (def->ovs_settings.controller.connection_mode) { value = def->ovs_settings.controller.connection_mode; - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set Controller %s connection-mode=%s", def->id, value); + append_systemd_cmd(cmds, + "%s set Controller %s " + "connection-mode=%s", + netplan_openvswitch_ovs_vsctl(), + def->id, value); write_ovs_tag_setting(def->id, "Controller", "connection-mode", NULL, value, cmds); } } @@ -400,7 +466,9 @@ netplan_netdef_write_ovs(const NetplanState* np_state, const NetplanNetDefinitio g_assert(def->vlan_link); dependency = def->vlan_link->id; /* Create a fake VLAN bridge */ - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " --may-exist add-br %s %s %i", def->id, def->vlan_link->id, def->vlan_id) + append_systemd_cmd(cmds, "%s --may-exist add-br %s %s %i", + netplan_openvswitch_ovs_vsctl(), + def->id, def->vlan_link->id, def->vlan_id) write_ovs_tag_netplan(def->id, type, cmds); break; @@ -477,7 +545,8 @@ netplan_state_finish_ovs_write(const NetplanState* np_state, const char* rootdir settings->ssl.client_key, settings->ssl.client_certificate, settings->ssl.ca_certificate); - append_systemd_cmd(cmds, OPENVSWITCH_OVS_VSCTL " set-ssl %s", value->str); + append_systemd_cmd(cmds, "%s set-ssl %s", + netplan_openvswitch_ovs_vsctl(), value->str); write_ovs_tag_setting(".", "open_vswitch", "global", "set-ssl", value->str, cmds); g_string_free(value, TRUE); } diff --git a/src/openvswitch.h b/src/openvswitch.h index 67aff9add..5982b11f8 100644 --- a/src/openvswitch.h +++ b/src/openvswitch.h @@ -29,3 +29,6 @@ netplan_netdef_write_ovs( NETPLAN_INTERNAL gboolean netplan_ovs_cleanup(const char* rootdir); + +NETPLAN_INTERNAL const char * +netplan_openvswitch_ovs_vsctl(void); diff --git a/src/util-internal.h b/src/util-internal.h index fe41c776e..1d590dd1f 100644 --- a/src/util-internal.h +++ b/src/util-internal.h @@ -61,8 +61,6 @@ wifi_get_freq5(int channel); NETPLAN_ABI gchar* systemd_escape(char* string); -#define OPENVSWITCH_OVS_VSCTL "/usr/bin/ovs-vsctl" - void mark_data_as_dirty(NetplanParser* npp, const void* data_ptr); @@ -179,3 +177,7 @@ _netplan_netdef_pertype_iter_next(struct netdef_pertype_iter* it); NETPLAN_INTERNAL void _netplan_netdef_pertype_iter_free(struct netdef_pertype_iter* it); + +#ifndef MAX +#define MAX(X, Y) ((X) > (Y) ? (X) : (Y)) +#endif diff --git a/src/validation.c b/src/validation.c index 4e372135f..baaa381c7 100644 --- a/src/validation.c +++ b/src/validation.c @@ -31,6 +31,7 @@ #include "error.h" #include "util-internal.h" #include "validation.h" +#include "openvswitch.h" /* Check coherence for address types */ @@ -401,7 +402,9 @@ validate_netdef_grammar(const NetplanParser* npp, NetplanNetDefinition* nd, GErr if (nd->backend == NETPLAN_BACKEND_OVS) { // LCOV_EXCL_START - if (!g_file_test(OPENVSWITCH_OVS_VSCTL, G_FILE_TEST_EXISTS)) { + if (!g_file_test(netplan_openvswitch_ovs_vsctl(), + G_FILE_TEST_EXISTS)) + { /* Tested via integration test */ return yaml_error(npp, NULL, error, "%s: The 'ovs-vsctl' tool is required to setup OpenVSwitch interfaces.", nd->id); } diff --git a/tests/ctests/build.sh b/tests/ctests/build.sh new file mode 100755 index 000000000..093e5bcf7 --- /dev/null +++ b/tests/ctests/build.sh @@ -0,0 +1,9 @@ +#!/bin/sh + +BUILDDIR="$1"; shift +meson setup ${BUILDDIR} $* +meson compile -C ${BUILDDIR} --verbose + +if [ "$(basename $0)" = "run.sh" ]; then + meson test -C ${BUILDDIR} +fi diff --git a/tests/ctests/meson.build b/tests/ctests/meson.build index 8a41e9a30..ab503153f 100644 --- a/tests/ctests/meson.build +++ b/tests/ctests/meson.build @@ -1,3 +1,10 @@ +project('ctests', 'c') + +add_project_arguments( + '-DSBINDIR="' + join_paths(get_option('prefix'), get_option('sbindir')) + '"', + '-D_GNU_SOURCE', + language: 'c') + tests = { 'test_netplan_parser': false, 'test_netplan_state': false, @@ -10,15 +17,25 @@ tests = { 'test_netplan_openvswitch': false, } +glib = dependency('glib-2.0') +gio = dependency('gio-2.0') +yaml = dependency('yaml-0.1') +uuid = dependency('uuid') cmocka = dependency('cmocka', required: true) +netplan = meson.get_compiler('c').find_library( + 'netplan', + dirs: [join_paths(get_option('parent_current_build_dir'), 'src')]) foreach name, should_fail: tests exe = executable(name, '@0@.c'.format(name), - include_directories: [inc, inc_internal], - dependencies: [cmocka, glib, gio, yaml, uuid], + include_directories: [ + join_paths(get_option('parent_current_source_dir'), 'include'), + join_paths(get_option('parent_current_source_dir'), 'src'), + ], + dependencies: [cmocka, glib, gio, yaml, uuid, netplan], c_args: [ - '-DFIXTURESDIR="' + meson.project_source_root() + '/tests/ctests/fixtures"', + '-DFIXTURESDIR="' + meson.project_source_root() + '/fixtures"', '-Wno-deprecated-declarations', '-D_GNU_SOURCE', ], diff --git a/tests/ctests/meson_options.txt b/tests/ctests/meson_options.txt new file mode 100644 index 000000000..da1127f6a --- /dev/null +++ b/tests/ctests/meson_options.txt @@ -0,0 +1,2 @@ +option('parent_current_source_dir', type: 'string', value: '') +option('parent_current_build_dir', type: 'string', value: '') diff --git a/tests/ctests/run.sh b/tests/ctests/run.sh new file mode 120000 index 000000000..c07a74de4 --- /dev/null +++ b/tests/ctests/run.sh @@ -0,0 +1 @@ +build.sh \ No newline at end of file diff --git a/tools/run_asan.sh b/tools/run_asan.sh index 73ed35e38..7541ca3ad 100755 --- a/tools/run_asan.sh +++ b/tools/run_asan.sh @@ -5,11 +5,19 @@ set -x BUILDDIR="_leakcheckbuild" CLEANBUILDDIR="_cleanbuild" +CTESTSBUILDDIR="_ctestsbuild" CC=gcc meson setup ${BUILDDIR} -Db_sanitize=address,undefined meson compile -C ${BUILDDIR} --verbose +tests/ctests/build.sh \ + ${CTESTSBUILDDIR} \ + tests/ctests \ + -Dparent_current_source_dir=$(realpath .) \ + -Dparent_current_build_dir=$(realpath ${BUILDDIR}) \ + -Db_sanitize=address,undefined + meson setup ${CLEANBUILDDIR} meson compile -C ${CLEANBUILDDIR} --verbose @@ -18,10 +26,11 @@ ${CC} tools/keyfile_to_yaml.c -o tools/keyfile_to_yaml \ -Iinclude -L${BUILDDIR}/src \ -fsanitize=address,undefined -g -TESTS=$(find ${BUILDDIR}/tests/ctests/ -executable -type f) +TESTS=$(find ${CTESTSBUILDDIR} -executable -type f) for test in ${TESTS} do - ./${test} + # https://github.com/google/sanitizers/issues/1017 + ASAN_OPTIONS=detect_odr_violation=0 ./${test} done mkdir -p ${BUILDDIR}/fakeroot/{etc/netplan,run}