Today the exit code refers to the execution of the change
in the database. However, when not using parameter --no-wait
(default), the ovs-vsctl also checks if OVSDB transactions
are successfully recorded and reload by ovs-vswitchd. In this
case, an error message is printed if there is a problem during
the reload, like for example the one below:
# ovs-vsctl add-port br0 gre0 -- \
set interface gre0 type=ip6gre options:key=100 \
options:remote_ip=2001::2
ovs-vsctl: Error detected while setting up 'gre0': could not \
add network device gre0 to ofproto (Address family not supported\
by protocol). See ovs-vswitchd log for details.
ovs-vsctl: The default log directory is "/var/log/openvswitch".
# echo $?
0
This patch changes to exit with specific error code '65'
(EX_DATAERR) if errors were reported during the reload.
This change may break existing scripts because ovs-vsctl will
start to fail when before it was succeeding. However, if an
error is printed, then it is likely that the change was not
functional anyway.
Reported-at: https://bugzilla.redhat.com/1731553
Signed-off-by: Flavio Leitner <[email protected]>
---
v2:
Followed Aaron's suggestion to return EX_DATAERR.
NEWS | 4 ++++
tests/ovs-vsctl.at | 19 +++++++++++++++++--
tests/ovs-vswitchd.at | 2 +-
tests/tunnel.at | 2 +-
utilities/ovs-vsctl.8.in | 2 ++
utilities/ovs-vsctl.c | 30 ++++++++++++++++++++++++------
6 files changed, 49 insertions(+), 10 deletions(-)
diff --git a/NEWS b/NEWS
index 66d5a4ea3..cb148a09f 100644
--- a/NEWS
+++ b/NEWS
@@ -28,6 +28,10 @@ Post-v3.1.0
* Added new options --[ovsdb-server|ovs-vswitchd]-umask=MODE to set umask
value when starting OVS daemons. E.g., use --ovsdb-server-umask=0002
in order to create OVSDB sockets with access mode of 0770.
+ - ovs-vsctl:
+ * Exit with error code 65 (EX_DATAERR) if errors were reported while
+ checking if OVSDB transactions are successfully recorded and reload
+ by ovs-vswitchd.
- QoS:
* Added new configuration option 'jitter' for a linux-netem QoS type.
- DPDK:
diff --git a/tests/ovs-vsctl.at b/tests/ovs-vsctl.at
index a368bff6e..b282798cc 100644
--- a/tests/ovs-vsctl.at
+++ b/tests/ovs-vsctl.at
@@ -1522,7 +1522,7 @@ cat >experr <<EOF
ovs-vsctl: Error detected while setting up 'reserved_name'. See ovs-vswitchd
log for details.
ovs-vsctl: The default log directory is "$OVS_RUNDIR".
EOF
-AT_CHECK([ovs-vsctl add-port br0 reserved_name], [0], [], [experr])
+AT_CHECK([ovs-vsctl add-port br0 reserved_name], [65], [], [experr])
# Prevent race.
OVS_WAIT_UNTIL([test `grep -- "|WARN|" ovs-vswitchd.log | wc -l` -ge 1])
# Detect the warning log message
@@ -1560,7 +1560,7 @@ cat >experr <<EOF
ovs-vsctl: Error detected while setting up 'reserved_name'. See ovs-vswitchd
log for details.
ovs-vsctl: The default log directory is "$OVS_RUNDIR".
EOF
-AT_CHECK([ovs-vsctl add-port br0 reserved_name], [0], [], [experr])
+AT_CHECK([ovs-vsctl add-port br0 reserved_name], [65], [], [experr])
# Prevent race.
OVS_WAIT_UNTIL([test `grep -- "|WARN|" ovs-vswitchd.log | wc -l` -ge 1])
# Detect the warning log message
@@ -1737,3 +1737,18 @@ AT_CHECK([ovs-vsctl --no-wait --bare --columns
_uuid,name list bridge tst1], [0]
OVS_VSCTL_CLEANUP
AT_CLEANUP
+
+AT_SETUP([ovs-vsctl -- return error if OVSDB reload issues are reported])
+OVS_VSWITCHD_START
+dnl check if ovs-vsctl returns error 65 if ovs-vswitchd fails to reload.
+
+cat >experr << EOF
+ovs-vsctl: Error detected while setting up 'gre0': gre0: bad ip6gre 'remote_ip'
+gre0: ip6gre type requires valid 'remote_ip' argument. See ovs-vswitchd log
for details.
+ovs-vsctl: The default log directory is "$OVS_RUNDIR".
+EOF
+AT_CHECK([ovs-vsctl add-port br0 gre0 -- set interface gre0 type=ip6gre
options:key=100 options:remote_ip=192.168.0.300], [65], [], [experr])
+OVS_VSWITCHD_STOP(["/is not a valid IP address/d
+/netdev_vport|WARN|gre0: bad ip6gre 'remote_ip'/d
+/netdev|WARN|gre0: could not set configuration/d"])
+AT_CLEANUP
diff --git a/tests/ovs-vswitchd.at b/tests/ovs-vswitchd.at
index 977b2eba1..80a748355 100644
--- a/tests/ovs-vswitchd.at
+++ b/tests/ovs-vswitchd.at
@@ -222,7 +222,7 @@ cat >experr <<EOF
ovs-vsctl: Error detected while setting up 'b/c'. See ovs-vswitchd log for
details.
ovs-vsctl: The default log directory is "$OVS_RUNDIR".
EOF
-AT_CHECK([ovs-vsctl add-br b/c -- set bridge b/c datapath-type=dummy], [0],
[], [experr])
+AT_CHECK([ovs-vsctl add-br b/c -- set bridge b/c datapath-type=dummy], [65],
[], [experr])
AT_CHECK([test ! -e b/c.mgmt])
OVS_VSWITCHD_STOP(['/ignoring bridge with invalid name/d'])
diff --git a/tests/tunnel.at b/tests/tunnel.at
index ddeb66bc9..d281c9e6c 100644
--- a/tests/tunnel.at
+++ b/tests/tunnel.at
@@ -1009,7 +1009,7 @@ OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1
type=vxlan \
options:remote_ip=flow ofport_request=1])
AT_CHECK([ovs-vsctl add-port br0 p2 -- set Interface p2 type=vxlan \
- options:remote_ip=flow options:exts=gbp options:key=1
ofport_request=2], [0],
+ options:remote_ip=flow options:exts=gbp options:key=1
ofport_request=2], [65],
[], [ignore])
AT_CHECK([grep 'p2: could not set configuration (File exists)'
ovs-vswitchd.log | sed "s/^.*\(p2:.*\)$/\1/"], [0],
diff --git a/utilities/ovs-vsctl.8.in b/utilities/ovs-vsctl.8.in
index 9e319aa1c..7c7e7bc29 100644
--- a/utilities/ovs-vsctl.8.in
+++ b/utilities/ovs-vsctl.8.in
@@ -892,6 +892,8 @@ Usage, syntax, or configuration file error.
.IP "2"
The \fIbridge\fR argument to \fBbr\-exists\fR specified the name of a
bridge that does not exist.
+.IP "65"
+An error has been reported post OVSDB reload.
.SH "SEE ALSO"
.
.BR ovsdb\-server (1),
diff --git a/utilities/ovs-vsctl.c b/utilities/ovs-vsctl.c
index 2f5ac1a26..d7338080b 100644
--- a/utilities/ovs-vsctl.c
+++ b/utilities/ovs-vsctl.c
@@ -25,6 +25,7 @@
#include <stdarg.h>
#include <stdlib.h>
#include <string.h>
+#include <sysexits.h>
#include <unistd.h>
#include "db-ctl-base.h"
@@ -56,6 +57,9 @@
VLOG_DEFINE_THIS_MODULE(vsctl);
+/* Post OVSDB reload error reported. */
+#define EXIT_POSTDB_ERROR EX_DATAERR
+
struct vsctl_context;
/* --db: The database server to contact. */
@@ -115,7 +119,7 @@ static void parse_options(int argc, char *argv[], struct
shash *local_options);
static void run_prerequisites(struct ctl_command[], size_t n_commands,
struct ovsdb_idl *);
static bool do_vsctl(const char *args, struct ctl_command *, size_t n,
- struct ovsdb_idl *);
+ struct ovsdb_idl *, bool *);
/* post_db_reload_check frame work is to allow ovs-vsctl to do additional
* checks after OVSDB transactions are successfully recorded and reload by
@@ -134,11 +138,13 @@ static bool do_vsctl(const char *args, struct ctl_command
*, size_t n,
* Current implementation only check for Post OVSDB reload failures on new
* interface additions with 'add-br' and 'add-port' commands.
*
+ * post_db_reload_check returns 'true' if a failure is reported.
+ *
* post_db_reload_expect_iface()
*
* keep track of interfaces to be checked post OVSDB reload. */
static void post_db_reload_check_init(void);
-static void post_db_reload_do_checks(const struct vsctl_context *);
+static bool post_db_reload_do_checks(const struct vsctl_context *);
static void post_db_reload_expect_iface(const struct ovsrec_interface *);
static struct uuid *neoteric_ifaces;
@@ -200,9 +206,15 @@ main(int argc, char *argv[])
}
if (seqno != ovsdb_idl_get_seqno(idl)) {
+ bool postdb_err;
+
seqno = ovsdb_idl_get_seqno(idl);
- if (do_vsctl(args, commands, n_commands, idl)) {
+ if (do_vsctl(args, commands, n_commands, idl, &postdb_err)) {
free(args);
+ if (postdb_err) {
+ exit(EXIT_POSTDB_ERROR);
+ }
+
exit(EXIT_SUCCESS);
}
}
@@ -2674,7 +2686,7 @@ post_db_reload_expect_iface(const struct ovsrec_interface
*iface)
neoteric_ifaces[n_neoteric_ifaces++] = iface->header_.uuid;
}
-static void
+static bool
post_db_reload_do_checks(const struct vsctl_context *vsctl_ctx)
{
bool print_error = false;
@@ -2707,6 +2719,8 @@ post_db_reload_do_checks(const struct vsctl_context
*vsctl_ctx)
if (print_error) {
ovs_error(0, "The default log directory is \"%s\".", ovs_logdir());
}
+
+ return print_error;
}
@@ -2815,7 +2829,7 @@ vsctl_parent_process_info(void)
static bool
do_vsctl(const char *args, struct ctl_command *commands, size_t n_commands,
- struct ovsdb_idl *idl)
+ struct ovsdb_idl *idl, bool *postdb_err)
{
struct ovsdb_idl_txn *txn;
const struct ovsrec_open_vswitch *ovs;
@@ -2827,6 +2841,8 @@ do_vsctl(const char *args, struct ctl_command *commands,
size_t n_commands,
int64_t next_cfg = 0;
char *ppid_info = NULL;
+ ovs_assert(postdb_err);
+ *postdb_err = false;
txn = the_idl_txn = ovsdb_idl_txn_create(idl);
if (dry_run) {
ovsdb_idl_txn_set_dry_run(txn);
@@ -2989,7 +3005,9 @@ do_vsctl(const char *args, struct ctl_command *commands,
size_t n_commands,
ovsdb_idl_run(idl);
OVSREC_OPEN_VSWITCH_FOR_EACH (ovs, idl) {
if (ovs->cur_cfg >= next_cfg) {
- post_db_reload_do_checks(&vsctl_ctx);
+ if (post_db_reload_do_checks(&vsctl_ctx)) {
+ *postdb_err = true;
+ }
goto done;
}
}
--
2.41.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
