[ovs-dev] ovsdb-client: Make "wait" command logging more sensible.

Message ID 20180806213634.19565-1-blp@ovn.org
State Accepted
Headers show
Series
  • [ovs-dev] ovsdb-client: Make "wait" command logging more sensible.
Related show

Commit Message

Ben Pfaff Aug. 6, 2018, 9:36 p.m.
The "wait" command in ovsdb-client (which was introduced as part of the
clustering support) fairly often logs things that are normal for it but
in other circumstances might be cause for concern, for example messages
about being unable to connect to a remote.  Until now, it has tried to
suppress some of those itself by raising log levels.  Unfortunately, in
some cases this had the opposite effect because it overrode any settings on
the command line, such as an attempt in ovsdb-cluster.at to suppress all
logging related to the timeval module.  This commit drops the special
log levels from the "wait" command and puts equivalents into the tests
themselves.

Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 ovsdb/ovsdb-client.c   | 4 ----
 tests/ovsdb-cluster.at | 6 +++---
 tests/ovsdb-server.at  | 2 +-
 tests/ovsdb-tool.at    | 2 +-
 tests/ovsdb.at         | 6 ++++++
 5 files changed, 11 insertions(+), 9 deletions(-)

Comments

Justin Pettit Aug. 17, 2018, 11:53 p.m. | #1
> On Aug 6, 2018, at 2:36 PM, Ben Pfaff <blp@ovn.org> wrote:
> 
> The "wait" command in ovsdb-client (which was introduced as part of the
> clustering support) fairly often logs things that are normal for it but
> in other circumstances might be cause for concern, for example messages
> about being unable to connect to a remote.  Until now, it has tried to
> suppress some of those itself by raising log levels.  Unfortunately, in
> some cases this had the opposite effect because it overrode any settings on
> the command line, such as an attempt in ovsdb-cluster.at to suppress all
> logging related to the timeval module.  This commit drops the special
> log levels from the "wait" command and puts equivalents into the tests
> themselves.
> 
> Signed-off-by: Ben Pfaff <blp@ovn.org>

Acked-by: Justin Pettit <jpettit@ovn.org>

--Justin
Ben Pfaff Aug. 18, 2018, 12:44 a.m. | #2
On Fri, Aug 17, 2018 at 04:53:51PM -0700, Justin Pettit wrote:
> 
> > On Aug 6, 2018, at 2:36 PM, Ben Pfaff <blp@ovn.org> wrote:
> > 
> > The "wait" command in ovsdb-client (which was introduced as part of the
> > clustering support) fairly often logs things that are normal for it but
> > in other circumstances might be cause for concern, for example messages
> > about being unable to connect to a remote.  Until now, it has tried to
> > suppress some of those itself by raising log levels.  Unfortunately, in
> > some cases this had the opposite effect because it overrode any settings on
> > the command line, such as an attempt in ovsdb-cluster.at to suppress all
> > logging related to the timeval module.  This commit drops the special
> > log levels from the "wait" command and puts equivalents into the tests
> > themselves.
> > 
> > Signed-off-by: Ben Pfaff <blp@ovn.org>
> 
> Acked-by: Justin Pettit <jpettit@ovn.org>

Thanks, applied to master.

Patch

diff --git a/ovsdb/ovsdb-client.c b/ovsdb/ovsdb-client.c
index cdb021f391ac..288732bd85f1 100644
--- a/ovsdb/ovsdb-client.c
+++ b/ovsdb/ovsdb-client.c
@@ -2328,10 +2328,6 @@  do_wait(struct jsonrpc *rpc_unused OVS_UNUSED,
         const char *database_unused OVS_UNUSED,
         int argc, char *argv[])
 {
-    vlog_set_levels(NULL, VLF_CONSOLE, VLL_WARN);
-    vlog_set_levels_from_string_assert("reconnect:err");
-    vlog_set_levels_from_string_assert("jsonrpc:err");
-
     const char *database = argv[argc - 2];
     const char *state = argv[argc - 1];
 
diff --git a/tests/ovsdb-cluster.at b/tests/ovsdb-cluster.at
index 01650df4c531..acc5cdde8b09 100644
--- a/tests/ovsdb-cluster.at
+++ b/tests/ovsdb-cluster.at
@@ -18,7 +18,7 @@  ovsdb_check_cluster () {
         AT_CHECK([ovsdb-server -vraft -vconsole:off -vsyslog:off --detach --no-chdir --log-file=s$i.log --pidfile=s$i.pid --unixctl=s$i --remote=punix:s$i.ovsdb s$i.db])
     done
     for i in `seq $n`; do
-        AT_CHECK([ovsdb-client --timeout=30 wait unix:s$i.ovsdb $schema connected])
+        AT_CHECK([ovsdb_client_wait unix:s$i.ovsdb $schema connected])
     done
 
     for txn
@@ -100,14 +100,14 @@  ovsdb|WARN|schema: changed 2 columns in 'OVN_Southbound' database from ephemeral
     connect_server() {
         local i=$1
         printf "\ns$i: waiting to connect to storage\n"
-        AT_CHECK([ovsdb-client --timeout=30 -vtimeval:off -vfile -vsyslog:off -vvlog:off --log-file=connect$i.log wait unix:s$i.ovsdb $schema connected])
+        AT_CHECK([ovsdb_client_wait --log-file=connect$i.log unix:s$i.ovsdb $schema connected])
     }
     remove_server() {
         local i=$1
         printf "\ns$i: removing from cluster\n"
         AT_CHECK([ovs-appctl --timeout=30 -t "`pwd`"/s$i cluster/leave OVN_Southbound])
         printf "\ns$i: waiting for removal to complete\n"
-        AT_CHECK([ovsdb-client --timeout=30 -vtimeval:off -vfile -vsyslog:off -vvlog:off --log-file=remove$i.log wait unix:s$i.ovsdb $schema removed])
+        AT_CHECK([ovsdb_client_wait --log-file=remove$i.log unix:s$i.ovsdb $schema removed])
         stop_server $i
     }
     add_server() {
diff --git a/tests/ovsdb-server.at b/tests/ovsdb-server.at
index edffae1bc075..add815c71f1f 100644
--- a/tests/ovsdb-server.at
+++ b/tests/ovsdb-server.at
@@ -668,7 +668,7 @@  ovsdb_check_online_compaction() {
               fi])
     dnl Start ovsdb-server.
     AT_CHECK([ovsdb-server -vvlog:off -vconsole:off --detach --no-chdir --pidfile --remote=punix:socket --log-file db], [0])
-    AT_CHECK([ovsdb-client wait unix:socket ordinals connected])
+    AT_CHECK([ovsdb_client_wait unix:socket ordinals connected])
     AT_CAPTURE_FILE([ovsdb-server.log])
     dnl Do a bunch of random transactions that put crap in the database log.
     AT_CHECK(
diff --git a/tests/ovsdb-tool.at b/tests/ovsdb-tool.at
index 359d1f063ee1..69c5d6afa801 100644
--- a/tests/ovsdb-tool.at
+++ b/tests/ovsdb-tool.at
@@ -452,7 +452,7 @@  ovsdb-tool create-cluster db2 db1 unix:s1.raft
 
 # Dump the data.
 AT_CHECK([ovsdb-server -vconsole:off -vfile -vvlog:off --detach --no-chdir --pidfile --log-file --remote=punix:db.sock db2])
-AT_CHECK([ovsdb-client wait ordinals connected])
+AT_CHECK([ovsdb_client_wait ordinals connected])
 AT_CHECK([ovsdb-client dump > dump2])
 OVS_APP_EXIT_AND_WAIT([ovsdb-server])
 
diff --git a/tests/ovsdb.at b/tests/ovsdb.at
index afe0a54a4380..0c9856d016cc 100644
--- a/tests/ovsdb.at
+++ b/tests/ovsdb.at
@@ -129,6 +129,12 @@  m4_define([OVSDB_CHECK_NEGATIVE_CPY],
    OVSDB_CHECK_NEGATIVE_PY([$1 - Python2], [$2], [$3], [$4], [$5])
    OVSDB_CHECK_NEGATIVE_PY3([$1 - Python3], [$2], [$3], [$4], [$5])])
 
+OVS_START_SHELL_HELPERS
+ovsdb_client_wait() {
+    ovsdb-client -vconsole:warn -vreconnect:err -vjsonrpc:err -vtimeval:off -vfile -vsyslog:off -vvlog:off --timeout=30 wait "$@"
+}
+OVS_END_SHELL_HELPERS
+
 m4_include([tests/ovsdb-log.at])
 m4_include([tests/ovsdb-types.at])
 m4_include([tests/ovsdb-data.at])