[ovs-dev] [PATCH] tests: Get rid of timeout options for control utilities.

Yifeng Sun pkusunyifeng at gmail.com
Tue Oct 15 17:05:52 UTC 2019


Looks good to me, thanks.

Reviewed-by: Yifeng Sun <pkusunyifeng at gmail.com>

On Tue, Oct 15, 2019 at 9:11 AM Ilya Maximets <i.maximets at ovn.org> wrote:
>
> 'OVS_CTL_TIMEOUT' environment variable is exported in tests/atlocal.in
> and controls timeouts for all OVS utilities in testsuite.
>
> There should be no manual tweaks for each single command.
>
> This helps with running tests under valgrind where commands could
> take really long time as you only need to change 'OVS_CTL_TIMEOUT'
> in a single place.
>
> Few manual timeouts were left in places where they make sense.
>
> Signed-off-by: Ilya Maximets <i.maximets at ovn.org>
> ---
>  tests/daemon.at         |  2 +-
>  tests/ofproto-dpif.at   | 18 +++++++++---------
>  tests/ofproto-macros.at |  2 +-
>  tests/ovs-macros.at     |  8 ++++----
>  tests/ovs-vswitchd.at   |  8 ++++----
>  tests/ovsdb-cluster.at  |  8 ++++----
>  tests/ovsdb-macros.at   |  2 +-
>  tests/pmd.at            |  4 ++--
>  tests/vtep-ctl.at       |  8 ++++----
>  9 files changed, 30 insertions(+), 30 deletions(-)
>
> diff --git a/tests/daemon.at b/tests/daemon.at
> index bdc8910f9..a7982de38 100644
> --- a/tests/daemon.at
> +++ b/tests/daemon.at
> @@ -97,7 +97,7 @@ check_process_name $child ovsdb-server
>
>  # Avoid a race between pidfile creation and notifying the parent,
>  # which can easily trigger if ovsdb-server is slow (e.g. due to valgrind).
> -OVS_WAIT_UNTIL([ovs-appctl --timeout=10 -t ovsdb-server version])
> +OVS_WAIT_UNTIL([ovs-appctl -t ovsdb-server version])
>
>  # Kill the daemon process, making it look like a segfault,
>  # and wait for a new child process to get spawned.
> diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
> index 8d9908858..49326c533 100644
> --- a/tests/ofproto-dpif.at
> +++ b/tests/ofproto-dpif.at
> @@ -10615,35 +10615,35 @@ AT_CHECK([ovs-vsctl get Interface p1 mtu], [0], [dnl
>  AT_CHECK([ovs-vsctl set Interface p1 mtu_request=1600])
>
>  # Check that the new MTU is applied
> -AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface p1 mtu=1600])
> +AT_CHECK([ovs-vsctl wait-until Interface p1 mtu=1600])
>  # The internal port 'br0' should have the same MTU value as p1, becase it's
>  # the new bridge minimum.
> -AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface br0 mtu=1600])
> +AT_CHECK([ovs-vsctl wait-until Interface br0 mtu=1600])
>
>  AT_CHECK([ovs-vsctl del-port br0 p1])
>
>  # When 'p1' is deleted, the internal port should return to the default MTU
> -AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface br0 mtu=1500])
> +AT_CHECK([ovs-vsctl wait-until Interface br0 mtu=1500])
>
>  # New port with 'mtu_request' in the same transaction.
>  AT_CHECK([ovs-vsctl add-port br0 p2 -- set int p2 type=dummy mtu_request=1600])
> -AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface p2 mtu=1600])
> -AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface br0 mtu=1600])
> +AT_CHECK([ovs-vsctl wait-until Interface p2 mtu=1600])
> +AT_CHECK([ovs-vsctl wait-until Interface br0 mtu=1600])
>
>  # Explicitly set mtu_request on the internal interface.  This should prevent
>  # the MTU from being overriden.
>  AT_CHECK([ovs-vsctl set int br0 mtu_request=1700])
> -AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface br0 mtu=1700])
> +AT_CHECK([ovs-vsctl wait-until Interface br0 mtu=1700])
>
>  # The new MTU on p2 should not affect br0.
>  AT_CHECK([ovs-vsctl set int p2 mtu_request=1400])
> -AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface p2 mtu=1400])
> -AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface br0 mtu=1700])
> +AT_CHECK([ovs-vsctl wait-until Interface p2 mtu=1400])
> +AT_CHECK([ovs-vsctl wait-until Interface br0 mtu=1700])
>
>  # Remove explicit mtu_request from br0.  Now it should track the bridge
>  # minimum again.
>  AT_CHECK([ovs-vsctl set int br0 mtu_request=[[]]])
> -AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface br0 mtu=1400])
> +AT_CHECK([ovs-vsctl wait-until Interface br0 mtu=1400])
>
>  OVS_VSWITCHD_STOP
>  AT_CLEANUP
> diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at
> index 04d4ed7e2..b2b17eed3 100644
> --- a/tests/ofproto-macros.at
> +++ b/tests/ofproto-macros.at
> @@ -249,7 +249,7 @@ add_of_br () {
>      local br=br$brnum
>      local dpid=fedcba987654321$brnum
>      local mac=aa:55:aa:55:00:0$brnum
> -    ovs-vsctl --timeout=20 \
> +    ovs-vsctl \
>          -- add-br $br \
>          -- set bridge $br datapath-type=dummy \
>                            fail-mode=secure \
> diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
> index e07c4b908..8e512f4e7 100644
> --- a/tests/ovs-macros.at
> +++ b/tests/ovs-macros.at
> @@ -155,7 +155,7 @@ kill_ovs_vswitchd () {
>      fi
>
>      # Tell the daemon to terminate gracefully
> -    ovs-appctl --timeout=10 -t ovs-vswitchd exit --cleanup 2>/dev/null
> +    ovs-appctl -t ovs-vswitchd exit --cleanup 2>/dev/null
>
>      # Nothing else to be done if there is no PID
>      test -z "$TMPPID" && return
> @@ -279,8 +279,8 @@ m4_define([OVS_APP_EXIT_AND_WAIT],
>    [AT_CHECK([test -e $OVS_RUNDIR/$1.pid])
>     TMPPID=$(cat $OVS_RUNDIR/$1.pid)
>     AT_CHECK(m4_if([$1],[ovs-vswitchd],
> -                  [ovs-appctl --timeout=10 -t $1 exit --cleanup],
> -                  [ovs-appctl --timeout=10 -t $1 exit]))
> +                  [ovs-appctl -t $1 exit --cleanup],
> +                  [ovs-appctl -t $1 exit]))
>     OVS_WAIT_WHILE([kill -0 $TMPPID 2>/dev/null])])
>
>  dnl OVS_APP_EXIT_AND_WAIT_BY_TARGET(TARGET, PIDFILE)
> @@ -290,7 +290,7 @@ dnl argument), and then wait for it to exit.
>  m4_define([OVS_APP_EXIT_AND_WAIT_BY_TARGET],
>    [AT_CHECK([test -e $2])
>     TMPPID=$(cat $2)
> -   AT_CHECK([ovs-appctl --timeout=10 --target=$1 exit])
> +   AT_CHECK([ovs-appctl --target=$1 exit])
>     OVS_WAIT_WHILE([kill -0 $TMPPID 2>/dev/null])])
>
>  dnl on_exit "COMMAND"
> diff --git a/tests/ovs-vswitchd.at b/tests/ovs-vswitchd.at
> index a30792b7d..bba4fea2b 100644
> --- a/tests/ovs-vswitchd.at
> +++ b/tests/ovs-vswitchd.at
> @@ -231,7 +231,7 @@ AT_SETUP([ovs-vswitchd - set datapath IDs])
>  OVS_VSWITCHD_START([remove bridge br0 other-config datapath-id])
>
>  # Get the default dpid and verify that it is of the expected form.
> -AT_CHECK([ovs-vsctl --timeout=10 wait-until bridge br0 datapath-id!='[[]]'])
> +AT_CHECK([ovs-vsctl wait-until bridge br0 datapath-id!='[[]]'])
>  AT_CHECK([ovs-vsctl get bridge br0 datapath-id], [0], [stdout])
>  orig_dpid=$(tr -d \" < stdout)
>  AT_CHECK([sed 's/[[0-9a-f]]/x/g' stdout], [0], ["xxxxxxxxxxxxxxxx"
> @@ -242,21 +242,21 @@ OFPT_FEATURES_REPLY: dpid:$orig_dpid
>
>  # Set a dpid with 16 hex digits.
>  AT_CHECK([ovs-vsctl set bridge br0 other-config:datapath-id=0123456789abcdef])
> -AT_CHECK([ovs-vsctl --timeout=10 wait-until bridge br0 datapath-id=0123456789abcdef])
> +AT_CHECK([ovs-vsctl wait-until bridge br0 datapath-id=0123456789abcdef])
>  AT_CHECK([ovs-ofctl show br0 | strip_xids | head -1], [0], [dnl
>  OFPT_FEATURES_REPLY: dpid:0123456789abcdef
>  ])
>
>  # Set a dpif with 0x prefix.
>  AT_CHECK([ovs-vsctl set bridge br0 other-config:datapath-id=0x5ad515c0])
> -AT_CHECK([ovs-vsctl --timeout=10 wait-until bridge br0 datapath-id=000000005ad515c0])
> +AT_CHECK([ovs-vsctl wait-until bridge br0 datapath-id=000000005ad515c0])
>  AT_CHECK([ovs-ofctl show br0 | strip_xids | head -1], [0], [dnl
>  OFPT_FEATURES_REPLY: dpid:000000005ad515c0
>  ])
>
>  # Set invalid all-zeros dpid and make sure that the default reappears.
>  AT_CHECK([ovs-vsctl set bridge br0 other-config:datapath-id=0x00])
> -AT_CHECK([ovs-vsctl --timeout=10 wait-until bridge br0 datapath-id=$orig_dpid])
> +AT_CHECK([ovs-vsctl wait-until bridge br0 datapath-id=$orig_dpid])
>  AT_CHECK_UNQUOTED([ovs-ofctl show br0 | strip_xids | head -1], [0], [dnl
>  OFPT_FEATURES_REPLY: dpid:$orig_dpid
>  ])
> diff --git a/tests/ovsdb-cluster.at b/tests/ovsdb-cluster.at
> index 24601c887..23ed7ec30 100644
> --- a/tests/ovsdb-cluster.at
> +++ b/tests/ovsdb-cluster.at
> @@ -23,7 +23,7 @@ ovsdb_check_cluster () {
>
>      for txn
>      do
> -      AT_CHECK([ovsdb-client --timeout=30 -vjsonrpc -vconsole:off -vsyslog:off -vvlog:off --log-file transact unix:s1.ovsdb,unix:s2.ovsdb,unix:s3.ovsdb "$txn"], [0], [stdout])
> +      AT_CHECK([ovsdb-client -vjsonrpc -vconsole:off -vsyslog:off -vvlog:off --log-file transact unix:s1.ovsdb,unix:s2.ovsdb,unix:s3.ovsdb "$txn"], [0], [stdout])
>        cat stdout >> output
>      done
>      AT_CHECK_UNQUOTED([uuidfilt output], [0], [$output])
> @@ -296,7 +296,7 @@ ovsdb|WARN|schema: changed 30 columns in 'Open_vSwitch' database from ephemeral
>          AT_CHECK([ovs-appctl -t "`pwd`"/s$delay_election_node cluster/failure-test delay-election], [0], [ignore])
>      fi
>      AT_CHECK([ovs-appctl -t "`pwd`"/s$crash_node cluster/failure-test $crash_command], [0], [ignore])
> -    AT_CHECK([ovs-vsctl -v --timeout=10 --db="$db" --no-leader-only --no-shuffle-remotes --no-wait create QoS type=x], [0], [ignore], [ignore])
> +    AT_CHECK([ovs-vsctl -v --db="$db" --no-leader-only --no-shuffle-remotes --no-wait create QoS type=x], [0], [ignore], [ignore])
>
>      # Make sure that the node really crashed.
>      AT_CHECK([ls s$crash_node.ovsdb], [2], [ignore], [ignore])
> @@ -442,7 +442,7 @@ ovsdb|WARN|schema: changed 30 columns in 'Open_vSwitch' database from ephemeral
>      remove_server() {
>          local i=$1
>          printf "\ns$i: removing from cluster\n"
> -        AT_CHECK([ovs-appctl --timeout=30 -t "`pwd`"/s$i cluster/leave Open_vSwitch])
> +        AT_CHECK([ovs-appctl -t "`pwd`"/s$i cluster/leave Open_vSwitch])
>          printf "\ns$i: waiting for removal to complete\n"
>          AT_CHECK([ovsdb_client_wait --log-file=remove$i.log unix:s$i.ovsdb $schema removed])
>          stop_server $i
> @@ -543,7 +543,7 @@ ovsdb|WARN|schema: changed 30 columns in 'Open_vSwitch' database from ephemeral
>              done
>          done
>      done | sort > expout
> -    AT_CHECK([ovs-vsctl --timeout=30 --db="$db" --no-wait --log-file=finalize.log -vtimeval:off -vfile -vsyslog:off --bare get Open_vSwitch . external-ids | tr ',' '\n' | sed 's/[[{}"" ]]//g' | sort], [0], [expout])
> +    AT_CHECK([ovs-vsctl --db="$db" --no-wait --log-file=finalize.log -vtimeval:off -vfile -vsyslog:off --bare get Open_vSwitch . external-ids | tr ',' '\n' | sed 's/[[{}"" ]]//g' | sort], [0], [expout])
>
>      for i in `seq $n`; do
>          if test $i != $victim || test $(cat phase) != 1; then
> diff --git a/tests/ovsdb-macros.at b/tests/ovsdb-macros.at
> index 1613642d5..0f8e4bd20 100644
> --- a/tests/ovsdb-macros.at
> +++ b/tests/ovsdb-macros.at
> @@ -102,6 +102,6 @@ m4_define([OVSDB_CHECK_NEGATIVE_CPY],
>
>  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 "$@"
> +    ovsdb-client -vconsole:warn -vreconnect:err -vjsonrpc:err -vtimeval:off -vfile -vsyslog:off -vvlog:off wait "$@"
>  }
>  OVS_END_SHELL_HELPERS
> diff --git a/tests/pmd.at b/tests/pmd.at
> index 96ae959c6..54dc5f235 100644
> --- a/tests/pmd.at
> +++ b/tests/pmd.at
> @@ -657,8 +657,8 @@ dnl busy, and uncover race conditions with the main thread.
>  AT_CHECK([ovs-vsctl set Interface p1 bfd:enable=true bfd:min_rx=1 bfd:min_tx=1])
>  AT_CHECK([ovs-vsctl set Interface p2 bfd:enable=true bfd:min_rx=1 bfd:min_tx=1])
>
> -AT_CHECK([ovs-vsctl --timeout=10 wait-until Interface p1 bfd_status:forwarding=true \
> -                              -- wait-until Interface p2 bfd_status:forwarding=true])
> +AT_CHECK([ovs-vsctl wait-until Interface p1 bfd_status:forwarding=true \
> +                 -- wait-until Interface p2 bfd_status:forwarding=true])
>
>  dnl Trigger reconfiguration of the datapath
>  AT_CHECK([ovs-vsctl set Interface p1 options:n_rxq=2])
> diff --git a/tests/vtep-ctl.at b/tests/vtep-ctl.at
> index f2b007f9a..3949f1623 100644
> --- a/tests/vtep-ctl.at
> +++ b/tests/vtep-ctl.at
> @@ -30,17 +30,17 @@ dnl RUN_VTEP_CTL(COMMAND, ...)
>  dnl
>  dnl Executes each vtep-ctl COMMAND.
>  m4_define([RUN_VTEP_CTL],
> -  [m4_foreach([command], [$@], [vtep-ctl --timeout=5 -vreconnect:emer --db=unix:socket command
> +  [m4_foreach([command], [$@], [vtep-ctl -vreconnect:emer --db=unix:socket command
>  ])])
>  m4_define([RUN_VTEP_CTL_ONELINE],
> -  [m4_foreach([command], [$@], [vtep-ctl --timeout=5 -vreconnect:emer --db=unix:socket --oneline -- command
> +  [m4_foreach([command], [$@], [vtep-ctl -vreconnect:emer --db=unix:socket --oneline -- command
>  ])])
>
>  dnl RUN_VTEP_CTL_TOGETHER(COMMAND, ...)
>  dnl
>  dnl Executes each vtep-ctl COMMAND in a single run of vtep-ctl.
>  m4_define([RUN_VTEP_CTL_TOGETHER],
> -  [vtep-ctl --timeout=5 -vreconnect:emer --db=unix:socket --oneline dnl
> +  [vtep-ctl -vreconnect:emer --db=unix:socket --oneline dnl
>  m4_foreach([command], [$@], [ -- command])])
>
>  dnl CHECK_PSWITCHES([PSWITCH], ...)
> @@ -934,7 +934,7 @@ AT_CHECK([RUN_VTEP_CTL(
>    [bind-ls a a1 100 ls1],
>    [set Physical_Switch a management_ips=[[4.3.2.1]] tunnel_ips=[[1.2.3.4]]])], [0], [ignore], [], [VTEP_CTL_CLEANUP])
>
> -AT_CHECK([vtep-ctl --timeout=5 -vreconnect:emer --db=unix:socket show | tail -n+2 | sed 's/=[[a-f0-9-]][[a-f0-9-]]*}/=<ls>}/' ], [0], [dnl
> +AT_CHECK([vtep-ctl -vreconnect:emer --db=unix:socket show | tail -n+2 | sed 's/=[[a-f0-9-]][[a-f0-9-]]*}/=<ls>}/' ], [0], [dnl
>      Manager "tcp:4.5.6.7"
>      Physical_Switch a
>          management_ips: [["4.3.2.1"]]
> --
> 2.17.1
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev


More information about the dev mailing list