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 <[email protected]> Signed-off-by: Ben Pfaff <[email protected]>
lrouter · Aug 15, 2018 · 9551e80 · 9551e80
1 parent cbcf40a
commit 9551e80
Show file tree

Hide file tree

Showing 16 changed files with 58 additions and 34 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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);