ovn-sbctl: Add daemon support.

Also rewrite the manpage and convert it to XML for consistency with ovn-nbctl, and add tests. Signed-off-by: Ben Pfaff <blp@ovn.org>
ovn-org · Apr 1, 2021 · e72c5ec · e72c5ec
1 parent a999642
commit e72c5ec
Show file tree

Hide file tree

Showing 10 changed files with 783 additions and 916 deletions.
diff --git a/NEWS b/NEWS
@@ -7,7 +7,9 @@ Post-v21.03.0
     (This may take testing and tuning to be effective.)  This version of OVN
     requires DDLog 0.36.
   - Introduce ovn-controller incremetal processing engine statistics
-  - ovn-nbctl daemon mode is no longer considered experimental.
+  - Utilities:
+    * ovn-nbctl daemon mode is no longer considered experimental.
+    * ovn-sbctl now also supports daemon mode.
 
 OVN v21.03.0 - 12 Mar 2021
 -------------------------

diff --git a/manpages.mk b/manpages.mk
@@ -10,20 +10,3 @@ lib/common-syn.man:
 lib/common.man:
 lib/ovs.tmac:
 
-utilities/ovn-sbctl.8: \
-	utilities/ovn-sbctl.8.in \
-	lib/common.man \
-	lib/db-ctl-base.man \
-	lib/ovs.tmac \
-	lib/ssl-bootstrap.man \
-	lib/ssl.man \
-	lib/table.man \
-	lib/vlog.man
-utilities/ovn-sbctl.8.in:
-lib/common.man:
-lib/db-ctl-base.man:
-lib/ovs.tmac:
-lib/ssl-bootstrap.man:
-lib/ssl.man:
-lib/table.man:
-lib/vlog.man:
diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at
@@ -1,9 +1,14 @@
 AT_BANNER([ovn-sbctl])
 
+OVS_START_SHELL_HELPERS
 # OVN_SBCTL_TEST_START
 m4_define([OVN_SBCTL_TEST_START],
-  [dnl Create databases (ovn-nb, ovn-sb).
-   AT_KEYWORDS([ovn])
+  [AT_KEYWORDS([ovn])
+   AT_CAPTURE_FILE([ovsdb-server.log])
+   AT_CAPTURE_FILE([ovn-northd.log])
+   ovn_sbctl_test_start $1])
+ovn_sbctl_test_start() {
+   dnl Create databases (ovn-nb, ovn-sb).
    for daemon in ovn-nb ovn-sb; do
       AT_CHECK([ovsdb-tool create $daemon.db $abs_top_srcdir/${daemon}.ovsschema])
    done
@@ -15,27 +20,54 @@ m4_define([OVN_SBCTL_TEST_START],
    AT_CHECK([[sed < stderr '
 /vlog|INFO|opened log file/d
 /ovsdb_server|INFO|ovsdb-server (Open vSwitch)/d']])
-   AT_CAPTURE_FILE([ovsdb-server.log])
 
    dnl Start ovn-northd.
    AT_CHECK([ovn-northd --detach --no-chdir --pidfile --log-file --ovnnb-db=unix:$OVS_RUNDIR/ovnnb_db.sock --ovnsb-db=unix:$OVS_RUNDIR/ovnsb_db.sock], [0], [], [stderr])
    on_exit "kill `cat ovn-northd.pid`"
    AT_CHECK([[sed < stderr '
 /vlog|INFO|opened log file/d']])
-   AT_CAPTURE_FILE([ovn-northd.log])
-])
+
+   AS_CASE([$1],
+     [daemon],
+       [export OVN_SB_DAEMON=$(ovn-sbctl --pidfile --detach --no-chdir --log-file -vsocket_util:off)
+        on_exit "kill `cat ovn-sbctl.pid`"],
+     [direct], [],
+     [*], [AT_FAIL_IF(:)])
+}
 
 # OVN_SBCTL_TEST_STOP
-m4_define([OVN_SBCTL_TEST_STOP],
-  [AT_CHECK([check_logs "$1"])
-   OVS_APP_EXIT_AND_WAIT([ovn-northd])
-   OVS_APP_EXIT_AND_WAIT_BY_TARGET([$OVS_RUNDIR/ovnnb_db.ctl], [$OVS_RUNDIR/ovnnb_db.pid])
-   OVS_APP_EXIT_AND_WAIT_BY_TARGET([$OVS_RUNDIR/ovnsb_db.ctl], [$OVS_RUNDIR/ovnsb_db.pid])])
+m4_define([OVN_SBCTL_TEST_STOP], [ovn_sbctl_test_stop])
+ovn_sbctl_test_stop() {
+  AT_CHECK([check_logs "$1"])
+  OVS_APP_EXIT_AND_WAIT([ovn-northd])
+  OVS_APP_EXIT_AND_WAIT_BY_TARGET([$OVS_RUNDIR/ovnnb_db.ctl], [$OVS_RUNDIR/ovnnb_db.pid])
+  OVS_APP_EXIT_AND_WAIT_BY_TARGET([$OVS_RUNDIR/ovnsb_db.ctl], [$OVS_RUNDIR/ovnsb_db.pid])
+}
+OVS_END_SHELL_HELPERS
+
+# OVN_SBCTL_TEST(NAME, TITLE, COMMANDS)
+m4_define([OVN_SBCTL_TEST],
+   [OVS_START_SHELL_HELPERS
+    $1() {
+      $3
+    }
+    OVS_END_SHELL_HELPERS
+
+    AT_SETUP([ovn-sbctl - $2 - direct])
+    OVN_SBCTL_TEST_START direct
+    $1
+    OVN_SBCTL_TEST_STOP
+    AT_CLEANUP
+
+    AT_SETUP([ovn-sbctl - $2 - daemon])
+    OVN_SBCTL_TEST_START daemon
+    $1
+    OVN_SBCTL_TEST_STOP
+    AT_CLEANUP])
 
 dnl ---------------------------------------------------------------------
 
-AT_SETUP([ovn-sbctl - chassis commands])
-OVN_SBCTL_TEST_START
+OVN_SBCTL_TEST([ovn_sbctl_chassis_commands], [ovn-sbctl - chassis commands], [
 ovn_init_db ovn-sb
 
 AT_CHECK([ovn-sbctl chassis-add ch0 geneve 1.2.3.4])
@@ -61,16 +93,14 @@ AT_CHECK([ovn-sbctl -f csv -d bare --no-headings --columns ip,type list encap |
 1.2.3.5,vxlan
 ])
 
-OVN_SBCTL_TEST_STOP
 as ovn-sb
 OVS_APP_EXIT_AND_WAIT([ovsdb-server])
-AT_CLEANUP
+as
+])
 
 dnl ---------------------------------------------------------------------
 
-AT_SETUP([ovn-sbctl])
-OVN_SBCTL_TEST_START
-
+OVN_SBCTL_TEST([ovn_sbctl_commands], [ovn-sbctl], [
 AT_CHECK([ovn-nbctl ls-add br-test])
 AT_CHECK([ovn-nbctl lsp-add br-test vif0])
 AT_CHECK([ovn-nbctl lsp-set-addresses vif0 f0:ab:cd:ef:01:02])
@@ -131,20 +161,14 @@ mac                 : [[]]
 type                : vtep
 options             : {vtep_logical_switch=l0, vtep_physical_switch=p0}
 ])
-
-OVN_SBCTL_TEST_STOP
-AT_CLEANUP
+])
 
 dnl ---------------------------------------------------------------------
 
-AT_SETUP([ovn-sbctl - connection])
-OVN_SBCTL_TEST_START
-
+OVN_SBCTL_TEST([ovn_sbctl_connection], [ovn-sbctl - connection], [
 AT_CHECK([ovn-sbctl --inactivity-probe=30000 set-connection ptcp:6641:127.0.0.1 punix:$OVS_RUNDIR/ovnsb_db.sock])
 AT_CHECK([ovn-sbctl list connection | grep inactivity_probe], [0], [dnl
 inactivity_probe    : 30000
 inactivity_probe    : 30000
 ])
-
-OVN_SBCTL_TEST_STOP
-AT_CLEANUP
+])
diff --git a/utilities/automake.mk b/utilities/automake.mk
@@ -14,7 +14,6 @@ man_MANS += \
     utilities/ovn-appctl.8
 
 MAN_ROOTS += \
-    utilities/ovn-sbctl.8.in \
     utilities/ovn-detrace.1.in
 
 # Docker drivers
@@ -30,6 +29,7 @@ EXTRA_DIST += \
     utilities/ovn-docker-overlay-driver.in \
     utilities/ovn-docker-underlay-driver.in \
     utilities/ovn-nbctl.8.xml \
+    utilities/ovn-sbctl.8.xml \
     utilities/ovn-ic-nbctl.8.xml \
     utilities/ovn-ic-sbctl.8.xml \
     utilities/ovn-appctl.8.xml \
@@ -79,7 +79,10 @@ utilities_ovn_nbctl_LDADD = lib/libovn.la $(OVSDB_LIBDIR)/libovsdb.la $(OVS_LIBD
 
 # ovn-sbctl
 bin_PROGRAMS += utilities/ovn-sbctl
-utilities_ovn_sbctl_SOURCES = utilities/ovn-sbctl.c
+utilities_ovn_sbctl_SOURCES = \
+    utilities/ovn-dbctl.c \
+    utilities/ovn-dbctl.h \
+    utilities/ovn-sbctl.c
 utilities_ovn_sbctl_LDADD = lib/libovn.la $(OVSDB_LIBDIR)/libovsdb.la $(OVS_LIBDIR)/libopenvswitch.la
 
 # ovn-ic-nbctl

diff --git a/utilities/ovn-dbctl.c b/utilities/ovn-dbctl.c
@@ -327,7 +327,8 @@ enum {
 };
 
 static char * OVS_WARN_UNUSED_RESULT
-handle_main_loop_option(int opt, const char *arg, bool *handled)
+handle_main_loop_option(const struct ovn_dbctl_options *dbctl_options,
+                        int opt, const char *arg, bool *handled)
 {
     ovs_assert(handled);
     *handled = true;
@@ -338,11 +339,16 @@ handle_main_loop_option(int opt, const char *arg, bool *handled)
         break;
 
     case OPT_NO_WAIT:
+        if (!dbctl_options->allow_wait) {
+            return xstrdup("--no-wait not supported");
+        }
         wait_type = NBCTL_WAIT_NONE;
         break;
 
     case OPT_WAIT:
-        if (!strcmp(arg, "none")) {
+        if (!dbctl_options->allow_wait) {
+            return xstrdup("--wait not supported");
+        } else if (!strcmp(arg, "none")) {
             wait_type = NBCTL_WAIT_NONE;
         } else if (!strcmp(arg, "sb")) {
             wait_type = NBCTL_WAIT_SB;
@@ -355,6 +361,9 @@ handle_main_loop_option(int opt, const char *arg, bool *handled)
         break;
 
     case OPT_PRINT_WAIT_TIME:
+        if (!dbctl_options->allow_wait) {
+            return xstrdup("--print-wait-time not supported");
+        }
         print_wait_time = true;
         break;
 
@@ -486,7 +495,8 @@ apply_options_direct(const struct ovn_dbctl_options *dbctl_options,
     for (const struct ovs_cmdl_parsed_option *po = parsed_options;
          po < &parsed_options[n]; po++) {
         bool handled;
-        char *error = handle_main_loop_option(po->o->val, po->arg, &handled);
+        char *error = handle_main_loop_option(dbctl_options,
+                                              po->o->val, po->arg, &handled);
         if (error) {
             ctl_fatal("%s", error);
         }
@@ -834,7 +844,8 @@ find_option_by_value(const struct option *options, int value)
 }
 
 static char * OVS_WARN_UNUSED_RESULT
-server_parse_options(int argc, char *argv[], struct shash *local_options,
+server_parse_options(const struct ovn_dbctl_options *dbctl_options,
+                     int argc, char *argv[], struct shash *local_options,
                      int *n_options_p)
 {
     static const struct option global_long_options[] = {
@@ -865,7 +876,7 @@ server_parse_options(int argc, char *argv[], struct shash *local_options,
         }
 
         bool handled;
-        error = handle_main_loop_option(c, optarg, &handled);
+        error = handle_main_loop_option(dbctl_options, c, optarg, &handled);
         if (error) {
             goto out;
         }
@@ -967,7 +978,8 @@ server_cmd_run(struct unixctl_conn *conn, int argc, const char **argv_,
     /* Parse commands & options. */
     char *args = process_escape_args(argv);
     shash_init(&local_options);
-    error = server_parse_options(argc, argv, &local_options, &n_options);
+    error = server_parse_options(dbctl_options,
+                                 argc, argv, &local_options, &n_options);
     if (error) {
         unixctl_command_reply_error(conn, error);
         goto out;

diff --git a/utilities/ovn-dbctl.h b/utilities/ovn-dbctl.h
@@ -15,7 +15,7 @@
 #ifndef OVN_DBCTL_H
 #define OVN_DBCTL_H 1
 
-/* ovn-nbctl infrastructure code. */
+/* Common code for ovn-sbctl and ovn-nbctl. */
 
 #include <stdbool.h>
 #include "ovsdb-idl.h"
@@ -31,6 +31,7 @@ enum nbctl_wait_type {
 struct ovn_dbctl_options {
     const char *db_version;     /* Database schema version. */
     const char *default_db;     /* Default database remote. */
+    bool allow_wait;            /* Allow --wait and related options? */
 
     /* Names of important environment variables. */
     const char *options_env_var_name; /* OVN_??_OPTIONS. */

diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
@@ -5982,6 +5982,7 @@ main(int argc, char *argv[])
     struct ovn_dbctl_options dbctl_options = {
         .db_version = nbrec_get_db_version(),
         .default_db = default_nb_db(),
+        .allow_wait = true,
 
         .options_env_var_name = "OVN_NBCTL_OPTIONS",
         .daemon_env_var_name = "OVN_NB_DAEMON",