From 9551e80befc0f8ecc5ee8a7e025147358c2753ee Mon Sep 17 00:00:00 2001 From: Ilya Maximets Date: Tue, 14 Aug 2018 10:53:16 +0300 Subject: [PATCH] tests: Use environment variable for default timeout. Introduce new 'OVS_CTL_TIMEOUT' environment variable that, if set, will be used as a default timeout for OVS control utilities. Setting it in 'atlocal.in' will cover all the hangs inside the testsuite, even when utils called in a subshell. Signed-off-by: Ilya Maximets Signed-off-by: Ben Pfaff --- NEWS | 2 ++ lib/util.c | 18 ++++++++++++++++++ lib/util.h | 2 ++ ovn/utilities/ovn-nbctl.c | 8 ++------ ovn/utilities/ovn-sbctl.c | 4 +--- ovsdb/ovsdb-client.c | 6 +++--- python/ovs/fatal_signal.py | 8 +++++++- tests/appctl.py | 3 +-- tests/atlocal.in | 4 ++++ tests/test-ovsdb.c | 6 +++--- tests/test-ovsdb.py | 4 +++- utilities/ovs-appctl.c | 6 +++--- utilities/ovs-dpctl.c | 7 ++++--- utilities/ovs-ofctl.c | 6 +++--- utilities/ovs-vsctl.c | 4 +--- vtep/vtep-ctl.c | 4 +--- 16 files changed, 58 insertions(+), 34 deletions(-) diff --git a/NEWS b/NEWS index ae21340e90..5a659647a7 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,8 @@ Post-v2.10.0 --------------------- - The environment variable OVS_SYSLOG_METHOD, if set, is now used as the default syslog method. + - The environment variable OVS_CTL_TIMEOUT, if set, is now used + as the default timeout for control utilities. v2.10.0 - xx xxx xxxx diff --git a/lib/util.c b/lib/util.c index 082bc7bb08..66544bff38 100644 --- a/lib/util.c +++ b/lib/util.c @@ -629,6 +629,24 @@ get_boot_time(void) return boot_time; } +/* This is a wrapper for setting timeout in control utils. + * The value of OVS_CTL_TIMEOUT environment variable will be used by + * default if 'secs' is not specified. */ +void +ctl_timeout_setup(unsigned int secs) +{ + if (!secs) { + char *env = getenv("OVS_CTL_TIMEOUT"); + + if (env && env[0]) { + str_to_uint(env, 10, &secs); + } + } + if (secs) { + time_alarm(secs); + } +} + /* Returns a pointer to a string describing the program version. The * caller must not modify or free the returned string. */ diff --git a/lib/util.h b/lib/util.h index fd414489c7..6e29751bf1 100644 --- a/lib/util.h +++ b/lib/util.h @@ -136,6 +136,8 @@ const char *get_subprogram_name(void); unsigned int get_page_size(void); long long int get_boot_time(void); +void ctl_timeout_setup(unsigned int secs); + void ovs_print_version(uint8_t min_ofp, uint8_t max_ofp); OVS_NO_RETURN void out_of_memory(void); diff --git a/ovn/utilities/ovn-nbctl.c b/ovn/utilities/ovn-nbctl.c index 972ba127fc..4f7e7241ad 100644 --- a/ovn/utilities/ovn-nbctl.c +++ b/ovn/utilities/ovn-nbctl.c @@ -199,9 +199,7 @@ main(int argc, char *argv[]) VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG, "Called as %s", args); - if (timeout) { - time_alarm(timeout); - } + ctl_timeout_setup(timeout); error = run_prerequisites(commands, n_commands, idl); if (error) { @@ -5381,9 +5379,7 @@ nbctl_client(const char *socket_name, svec_add(&args, argv[i]); } - if (timeout) { - time_alarm(timeout); - } + ctl_timeout_setup(timeout); struct jsonrpc *client; int error = unixctl_client_create(socket_name, &client); diff --git a/ovn/utilities/ovn-sbctl.c b/ovn/utilities/ovn-sbctl.c index 6ec4c80dd1..d0af83fa03 100644 --- a/ovn/utilities/ovn-sbctl.c +++ b/ovn/utilities/ovn-sbctl.c @@ -120,9 +120,7 @@ main(int argc, char *argv[]) VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG, "Called as %s", args); - if (timeout) { - time_alarm(timeout); - } + ctl_timeout_setup(timeout); /* Initialize IDL. */ idl = the_idl = ovsdb_idl_create(db, &sbrec_idl_class, false, true); diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c index 7f02188148..18cc135884 100644 --- a/ovsdb/ovsdb-client.c +++ b/ovsdb/ovsdb-client.c @@ -328,11 +328,11 @@ parse_options(int argc, char *argv[]) {NULL, 0, NULL, 0}, }; char *short_options = ovs_cmdl_long_options_to_short_options(long_options); + unsigned int timeout = 0; table_style.format = TF_TABLE; for (;;) { - unsigned int timeout = 0; int c; c = getopt_long(argc, argv, short_options, long_options, NULL); @@ -368,8 +368,6 @@ parse_options(int argc, char *argv[]) case 't': if (!str_to_uint(optarg, 10, &timeout) || !timeout) { ovs_fatal(0, "value %s on -t or --timeout is invalid", optarg); - } else { - time_alarm(timeout); } break; @@ -393,6 +391,8 @@ parse_options(int argc, char *argv[]) } } free(short_options); + + ctl_timeout_setup(timeout); } static void diff --git a/python/ovs/fatal_signal.py b/python/ovs/fatal_signal.py index eb4b663f4f..cb2e99e87d 100644 --- a/python/ovs/fatal_signal.py +++ b/python/ovs/fatal_signal.py @@ -156,8 +156,14 @@ def _init(): def signal_alarm(timeout): + if not timeout: + env_timeout = os.environ.get('OVS_CTL_TIMEOUT') + if env_timeout: + timeout = int(env_timeout) + if not timeout: + return + if sys.platform == "win32": - import os import time import threading diff --git a/tests/appctl.py b/tests/appctl.py index e4f0696651..b85b364fac 100644 --- a/tests/appctl.py +++ b/tests/appctl.py @@ -51,8 +51,7 @@ def main(): help="wait at most SECS seconds for a response") args = parser.parse_args() - if args.timeout: - signal_alarm(int(args.timeout)) + signal_alarm(int(args.timeout) if args.timeout else None) ovs.vlog.Vlog.init() target = args.target diff --git a/tests/atlocal.in b/tests/atlocal.in index a86de8bc1b..abfc1acf36 100644 --- a/tests/atlocal.in +++ b/tests/atlocal.in @@ -216,3 +216,7 @@ unset OVN_SB_DB # Prevent logging to syslog during tests. OVS_SYSLOG_METHOD=null export OVS_SYSLOG_METHOD + +# Set default timeout for control utils +OVS_CTL_TIMEOUT=30 +export OVS_CTL_TIMEOUT diff --git a/tests/test-ovsdb.c b/tests/test-ovsdb.c index 4b216fe74b..f3f0e49671 100644 --- a/tests/test-ovsdb.c +++ b/tests/test-ovsdb.c @@ -94,9 +94,9 @@ parse_options(int argc, char *argv[], struct test_ovsdb_pvt_context *pvt) {NULL, 0, NULL, 0}, }; char *short_options = ovs_cmdl_long_options_to_short_options(long_options); + unsigned int timeout = 0; for (;;) { - unsigned int timeout = 0; int c; c = getopt_long(argc, argv, short_options, long_options, NULL); @@ -108,8 +108,6 @@ parse_options(int argc, char *argv[], struct test_ovsdb_pvt_context *pvt) case 't': if (!str_to_uint(optarg, 10, &timeout) || !timeout) { ovs_fatal(0, "value %s on -t or --timeout is invalid", optarg); - } else { - time_alarm(timeout); } break; @@ -140,6 +138,8 @@ parse_options(int argc, char *argv[], struct test_ovsdb_pvt_context *pvt) } } free(short_options); + + ctl_timeout_setup(timeout); } static void diff --git a/tests/test-ovsdb.py b/tests/test-ovsdb.py index ed5d21b0c1..14491a2e91 100644 --- a/tests/test-ovsdb.py +++ b/tests/test-ovsdb.py @@ -822,6 +822,7 @@ def main(argv): sys.stderr.write("%s: %s\n" % (ovs.util.PROGRAM_NAME, geo.msg)) sys.exit(1) + timeout = None for key, value in options: if key in ['-h', '--help']: usage() @@ -833,10 +834,11 @@ def main(argv): except TypeError: raise error.Error("value %s on -t or --timeout is not at " "least 1" % value) - signal_alarm(timeout) else: sys.exit(0) + signal_alarm(timeout) + if not args: sys.stderr.write("%s: missing command argument " "(use --help for help)\n" % ovs.util.PROGRAM_NAME) diff --git a/utilities/ovs-appctl.c b/utilities/ovs-appctl.c index d6408ea9d7..ba0c172e6d 100644 --- a/utilities/ovs-appctl.c +++ b/utilities/ovs-appctl.c @@ -128,12 +128,12 @@ parse_command_line(int argc, char *argv[]) char *short_options = xasprintf("+%s", short_options_); const char *target; int e_options; + unsigned int timeout = 0; target = NULL; e_options = 0; for (;;) { int option; - unsigned int timeout = 0; option = getopt_long(argc, argv, short_options, long_options, NULL); if (option == -1) { @@ -168,8 +168,6 @@ parse_command_line(int argc, char *argv[]) case 'T': if (!str_to_uint(optarg, 10, &timeout) || !timeout) { ovs_fatal(0, "value %s on -T or --timeout is invalid", optarg); - } else { - time_alarm(timeout); } break; @@ -189,6 +187,8 @@ parse_command_line(int argc, char *argv[]) free(short_options_); free(short_options); + ctl_timeout_setup(timeout); + if (optind >= argc) { ovs_fatal(0, "at least one non-option argument is required " "(use --help for help)"); diff --git a/utilities/ovs-dpctl.c b/utilities/ovs-dpctl.c index 441f804404..7d99607f4c 100644 --- a/utilities/ovs-dpctl.c +++ b/utilities/ovs-dpctl.c @@ -100,8 +100,9 @@ parse_options(int argc, char *argv[]) char *short_options = ovs_cmdl_long_options_to_short_options(long_options); bool set_names = false; + unsigned int timeout = 0; + for (;;) { - unsigned int timeout = 0; int c; c = getopt_long(argc, argv, short_options, long_options, NULL); @@ -143,8 +144,6 @@ parse_options(int argc, char *argv[]) case 't': if (!str_to_uint(optarg, 10, &timeout) || !timeout) { ovs_fatal(0, "value %s on -t or --timeout is invalid", optarg); - } else { - time_alarm(timeout); } break; @@ -170,6 +169,8 @@ parse_options(int argc, char *argv[]) } free(short_options); + ctl_timeout_setup(timeout); + if (!set_names) { dpctl_p.names = dpctl_p.verbosity > 0; } diff --git a/utilities/ovs-ofctl.c b/utilities/ovs-ofctl.c index 708e6b26ef..c018bd48fc 100644 --- a/utilities/ovs-ofctl.c +++ b/utilities/ovs-ofctl.c @@ -253,6 +253,7 @@ parse_options(int argc, char *argv[]) char *short_options = ovs_cmdl_long_options_to_short_options(long_options); uint32_t versions; enum ofputil_protocol version_protocols; + unsigned int timeout = 0; /* For now, ovs-ofctl only enables OpenFlow 1.0 by default. This is * because ovs-ofctl implements command such as "add-flow" as raw OpenFlow @@ -272,7 +273,6 @@ parse_options(int argc, char *argv[]) set_allowed_ofp_versions("OpenFlow10"); for (;;) { - unsigned int timeout = 0; int c; c = getopt_long(argc, argv, short_options, long_options, NULL); @@ -284,8 +284,6 @@ parse_options(int argc, char *argv[]) case 't': if (!str_to_uint(optarg, 10, &timeout) || !timeout) { ovs_fatal(0, "value %s on -t or --timeout is invalid", optarg); - } else { - time_alarm(timeout); } break; @@ -391,6 +389,8 @@ parse_options(int argc, char *argv[]) } } + ctl_timeout_setup(timeout); + if (n_criteria) { /* Always do a final sort pass based on priority. */ add_sort_criterion(SORT_DESC, "priority"); diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c index 523775c6bc..a369051863 100644 --- a/utilities/ovs-vsctl.c +++ b/utilities/ovs-vsctl.c @@ -156,9 +156,7 @@ main(int argc, char *argv[]) VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG, "Called as %s", args); - if (timeout) { - time_alarm(timeout); - } + ctl_timeout_setup(timeout); /* Initialize IDL. */ idl = the_idl = ovsdb_idl_create(db, &ovsrec_idl_class, false, retry); diff --git a/vtep/vtep-ctl.c b/vtep/vtep-ctl.c index fb3c81e2be..ab552457d9 100644 --- a/vtep/vtep-ctl.c +++ b/vtep/vtep-ctl.c @@ -119,9 +119,7 @@ main(int argc, char *argv[]) VLOG(ctl_might_write_to_db(commands, n_commands) ? VLL_INFO : VLL_DBG, "Called as %s", args); - if (timeout) { - time_alarm(timeout); - } + ctl_timeout_setup(timeout); /* Initialize IDL. */ idl = the_idl = ovsdb_idl_create(db, &vteprec_idl_class, false, false);