[ovs-dev] [PATCH] ovsdb-idl.at: Wait all servers to join the cluster.

Ilya Maximets i.maximets at ovn.org
Thu Sep 3 22:54:39 UTC 2020


On 9/4/20 12:05 AM, Flavio Leitner wrote:
> On Thu, Sep 03, 2020 at 11:20:56PM +0200, Ilya Maximets wrote:
>> On 6/11/20 1:45 AM, Flavio Leitner wrote:
>>> The test 'Check Python IDL reconnects to leader - Python3
>>> (leader only)' fails sometimes when the first ovsdb-server
>>> gets killed before the others had joined the cluster.
>>>
>>> Fix the function ovsdb_cluster_start_idltest to wait them
>>> to join the cluster.
>>
>> Hi, Flavio.  Thanks for the fix and sorry for delays.
>>
>> Patch seems OK, but I'm not very comfortable with the code duplication
>> between this function and OVS_WAIT_UNTIL macro.  Have you considered
>> conversion of ovsdb_cluster_start_idltest() function into m4_define()
>> macro so we could easily use OVS_WAIT_UNTIL inside of it?
> 
> I tried, but I ran into issues and I am not familiar with m4.

Could you try following diff:

diff --git a/tests/ovsdb-idl.at b/tests/ovsdb-idl.at
index 789ae23a9..215f0a2d0 100644
--- a/tests/ovsdb-idl.at
+++ b/tests/ovsdb-idl.at
@@ -12,25 +12,6 @@ ovsdb_start_idltest () {
     on_exit 'kill `cat ovsdb-server.pid`'
 }
 
-# ovsdb_cluster_start_idltest [REMOTE] [SCHEMA]
-#
-# Creates a database using SCHEMA (default: idltest.ovsschema) and
-# starts a database cluster listening on punix:socket and REMOTE (if
-# specified).
-ovsdb_cluster_start_idltest () {
-   local n=$1
-   ovsdb-tool create-cluster s1.db $abs_srcdir/idltest.ovsschema unix:s1.raft || return $?
-   cid=`ovsdb-tool db-cid s1.db`
-   schema_name=`ovsdb-tool schema-name $abs_srcdir/idltest.ovsschema`
-   for i in `seq 2 $n`; do
-     ovsdb-tool join-cluster s$i.db $schema_name unix:s$i.raft unix:s1.raft || return $?
-   done
-   for i in `seq $n`; do
-     ovsdb-server -vraft -vconsole:warn --detach --no-chdir --log-file=s$i.log --pidfile=s$i.pid --unixctl=s$i --remote=punix:s$i.ovsdb ${2:+--remote=$2} s$i.db || return $?
-   done
-   on_exit 'kill `cat s*.pid`'
-}
-
 # ovsdb_cluster_leader [REMOTES] [DATABASE]
 #
 # Returns the leader of the DATABASE cluster.
@@ -48,6 +29,34 @@ ovsdb_cluster_leader () {
    done
 }])
 
+# OVSDB_CLUSTER_START_IDLTEST([N], [REMOTE])
+#
+# Creates a clustered database using idltest.ovsschema and starts a database
+# cluster of N servers listening on punix:socket and REMOTE (if specified).
+m4_define([OVSDB_CLUSTER_START_IDLTEST],
+  [n=$1
+   AT_CHECK([ovsdb-tool create-cluster s1.db \
+                        $abs_srcdir/idltest.ovsschema unix:s1.raft])
+   cid=$(ovsdb-tool db-cid s1.db)
+   schema_name=$(ovsdb-tool schema-name $abs_srcdir/idltest.ovsschema)
+   for i in $(seq 2 $n); do
+     AT_CHECK([ovsdb-tool join-cluster s$i.db \
+                          $schema_name unix:s$i.raft unix:s1.raft])
+   done
+   for i in $(seq $n); do
+     AT_CHECK([ovsdb-server -vraft -vconsole:warn --detach --no-chdir \
+                   --log-file=s$i.log --pidfile=s$i.pid --unixctl=s$i \
+                   --remote=punix:s$i.ovsdb ${2:+--remote=$2} s$i.db])
+   done
+   on_exit 'kill $(cat s*.pid)'
+
+   for i in $(seq $n); do
+       OVS_WAIT_UNTIL([ovs-appctl -t $(pwd)/s$i cluster/status ${schema_name} \
+                                           | grep -q 'Status: cluster member'])
+   done
+])
+
+
 # OVSDB_CHECK_IDL_C(TITLE, [PRE-IDL-TXN], TRANSACTIONS, OUTPUT, [KEYWORDS],
 #                   [FILTER])
 #
@@ -1813,7 +1822,7 @@ m4_define([OVSDB_CHECK_IDL_LEADER_ONLY_PY],
    AT_SKIP_IF([test "$IS_ARM64" = "yes"])
    AT_KEYWORDS([ovsdb server idl Python leader_only with tcp socket])
    m4_define([LPBK],[127.0.0.1])
-   AT_CHECK([ovsdb_cluster_start_idltest $2 "ptcp:0:"LPBK])
+   OVSDB_CLUSTER_START_IDLTEST([$2], ["ptcp:0:"LPBK])
    PARSE_LISTENING_PORT([s2.log], [TCP_PORT_1])
    PARSE_LISTENING_PORT([s3.log], [TCP_PORT_2])
    PARSE_LISTENING_PORT([s1.log], [TCP_PORT_3])
@@ -1836,7 +1845,7 @@ m4_define([OVSDB_CHECK_CLUSTER_IDL_C],
   [AT_SETUP([$1 - C - tcp])
    AT_KEYWORDS([ovsdb server idl positive tcp socket $5])
    m4_define([LPBK],[127.0.0.1])
-   AT_CHECK([ovsdb_cluster_start_idltest $2 "ptcp:0:"LPBK])
+   OVSDB_CLUSTER_START_IDLTEST([$2], ["ptcp:0:"LPBK])
    PARSE_LISTENING_PORT([s1.log], [TCP_PORT_1])
    PARSE_LISTENING_PORT([s2.log], [TCP_PORT_2])
    PARSE_LISTENING_PORT([s3.log], [TCP_PORT_3])
-- 

This looks like a bigger change, but, in the end, this should be easier to maintain.

Best regards, Ilya Maximets.


More information about the dev mailing list