OVS_WAIT_UNTIL() macro has only 2 arguments and doesn't check the
output of the command, but bonding and route tests are trying to use
it as if it was AT_CHECK macro. That makes checks in those tests
mostly useless, since they are not actually checking anything except
for command returning zero.
Introducing a new macro OVS_WAIT_UNTIL_EQUAL that will actually
perform the comparison with the desired output. Using it for
the bonding and route tests and fixing all the caught incorrect
expected outputs along the way.
Adding an explicit argument check to the OVS_WAIT_UNTIL/WHILE to
avoid the problem in the future.
Fixes: b4e50218a0f8 ("bond: Add 'primary' interface concept for active-backup
mode.")
Fixes: 9e11517e6ca6 ("ovs-router: Fix flushing of local routes.")
Signed-off-by: Ilya Maximets <[email protected]>
---
Version 2:
- Fixed the route test.
- Added explicit check for the arguments.
- Re-named the patch. v1:
https://patchwork.ozlabs.org/project/openvswitch/patch/[email protected]/
tests/ofproto-dpif.at | 24 ++++++++++++------------
tests/ovs-macros.at | 20 ++++++++++++++++++--
tests/system-route.at | 7 +++----
3 files changed, 33 insertions(+), 18 deletions(-)
diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index 912f3a1da..58f08dff9 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -81,11 +81,12 @@
recirc_id(0),in_port(4),packet_type(ns=0,id=0),eth(src=50:54:00:00:00:0b,dst=ff:
ovs-appctl netdev-dummy/set-admin-state p1 up
ovs-appctl time/warp 100
-OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID |
STRIP_ACTIVE_MEMBER_MAC], [0], [dnl
+OVS_WAIT_UNTIL_EQUAL([ovs-appctl bond/show | STRIP_RECIRC_ID |
STRIP_ACTIVE_MEMBER_MAC], [dnl
---- bond0 ----
bond_mode: active-backup
bond may use recirculation: no, <del>
bond-hash-basis: 0
+lb_output action: disabled, bond-id: -1
updelay: 0 ms
downdelay: 0 ms
lacp_status: off
@@ -99,7 +100,6 @@ member p1: enabled
member p2: enabled
may_enable: true
-
])
OVS_VSWITCHD_STOP
@@ -129,11 +129,12 @@ ovs-appctl time/warp 100
OVS_WAIT_UNTIL([test -n "`ovs-appctl bond/show | fgrep 'member p1:
disabled'`"])
ovs-appctl netdev-dummy/set-admin-state p1 up
ovs-appctl time/warp 100
-OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID |
STRIP_ACTIVE_MEMBER_MAC], [0], [dnl
+OVS_WAIT_UNTIL_EQUAL([ovs-appctl bond/show | STRIP_RECIRC_ID |
STRIP_ACTIVE_MEMBER_MAC], [dnl
---- bond0 ----
bond_mode: active-backup
bond may use recirculation: no, <del>
bond-hash-basis: 0
+lb_output action: disabled, bond-id: -1
updelay: 0 ms
downdelay: 0 ms
lacp_status: off
@@ -150,7 +151,6 @@ member p2: enabled
member p3: enabled
may_enable: true
-
])
dnl Now delete the primary and verify that the output shows that the
@@ -171,11 +171,12 @@ ovs-vsctl \
--id=@p1 create Interface name=p1 type=dummy
options:pstream=punix:$OVS_RUNDIR/p1.sock ofport_request=1 -- \
set Port bond0 interfaces="$uuids, @p1]"
ovs-appctl time/warp 100
-OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID |
STRIP_ACTIVE_MEMBER_MAC], [0], [dnl
+OVS_WAIT_UNTIL_EQUAL([ovs-appctl bond/show | STRIP_RECIRC_ID |
STRIP_ACTIVE_MEMBER_MAC], [dnl
---- bond0 ----
bond_mode: active-backup
bond may use recirculation: no, <del>
bond-hash-basis: 0
+lb_output action: disabled, bond-id: -1
updelay: 0 ms
downdelay: 0 ms
lacp_status: off
@@ -192,17 +193,17 @@ member p2: enabled
member p3: enabled
may_enable: true
-
])
dnl Switch to another primary
ovs-vsctl set port bond0 other_config:bond-primary=p2
ovs-appctl time/warp 100
-OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID |
STRIP_ACTIVE_MEMBER_MAC], [0], [dnl
+OVS_WAIT_UNTIL_EQUAL([ovs-appctl bond/show | STRIP_RECIRC_ID |
STRIP_ACTIVE_MEMBER_MAC], [dnl
---- bond0 ----
bond_mode: active-backup
bond may use recirculation: no, <del>
bond-hash-basis: 0
+lb_output action: disabled, bond-id: -1
updelay: 0 ms
downdelay: 0 ms
lacp_status: off
@@ -211,25 +212,25 @@ active-backup primary: p2
<active member mac del>
member p1: enabled
- active member
may_enable: true
member p2: enabled
+ active member
may_enable: true
member p3: enabled
may_enable: true
-
])
dnl Remove the "bond-primary" config directive from the bond.
AT_CHECK([ovs-vsctl remove Port bond0 other_config bond-primary])
ovs-appctl time/warp 100
-OVS_WAIT_UNTIL([ovs-appctl bond/show | STRIP_RECIRC_ID |
STRIP_ACTIVE_MEMBER_MAC], [0], [dnl
+OVS_WAIT_UNTIL_EQUAL([ovs-appctl bond/show | STRIP_RECIRC_ID |
STRIP_ACTIVE_MEMBER_MAC], [dnl
---- bond0 ----
bond_mode: active-backup
bond may use recirculation: no, <del>
bond-hash-basis: 0
+lb_output action: disabled, bond-id: -1
updelay: 0 ms
downdelay: 0 ms
lacp_status: off
@@ -238,15 +239,14 @@ active-backup primary: <none>
<active member mac del>
member p1: enabled
- active member
may_enable: true
member p2: enabled
+ active member
may_enable: true
member p3: enabled
may_enable: true
-
])
OVS_VSWITCHD_STOP
diff --git a/tests/ovs-macros.at b/tests/ovs-macros.at
index 66545da57..e6c5bc6e9 100644
--- a/tests/ovs-macros.at
+++ b/tests/ovs-macros.at
@@ -259,7 +259,20 @@ dnl Executes shell COMMAND in a loop until it returns
zero. If COMMAND does
dnl not return zero within a reasonable time limit, executes the commands
dnl in IF-FAILED (if provided) and fails the test.
m4_define([OVS_WAIT_UNTIL],
- [OVS_WAIT([$1], [$2], [AT_LINE], [until $1])])
+ [AT_FAIL_IF([test "$#" -ge 3])
+ dnl The second argument should not be a number (confused with AT_CHECK ?).
+ AT_FAIL_IF([test "$#" -eq 2 && test "$2" -eq "$2" 2>/dev/null])
+ OVS_WAIT([$1], [$2], [AT_LINE], [until $1])])
+
+dnl OVS_WAIT_UNTIL_EQUAL(COMMAND, OUTPUT)
+dnl
+dnl Executes shell COMMAND in a loop until it returns zero and the output
+dnl equals OUTPUT. If COMMAND does not return zero or a desired output within
+dnl a reasonable time limit, fails the test.
+m4_define([OVS_WAIT_UNTIL_EQUAL],
+ [AT_FAIL_IF([test "$#" -ge 3])
+ echo "$2" > wait_until_expected
+ OVS_WAIT_UNTIL([$1 | diff -u wait_until_expected - ])])
dnl OVS_WAIT_WHILE(COMMAND, [IF-FAILED])
dnl
@@ -267,7 +280,10 @@ dnl Executes shell COMMAND in a loop until it returns
nonzero. If COMMAND does
dnl not return nonzero within a reasonable time limit, executes the commands
dnl in IF-FAILED (if provided) and fails the test.
m4_define([OVS_WAIT_WHILE],
- [OVS_WAIT([if $1; then return 1; else return 0; fi], [$2],
+ [AT_FAIL_IF([test "$#" -ge 3])
+ dnl The second argument should not be a number (confused with AT_CHECK ?).
+ AT_FAIL_IF([test "$#" -eq 2 && test "$2" -eq "$2" 2>/dev/null])
+ OVS_WAIT([if $1; then return 1; else return 0; fi], [$2],
[AT_LINE], [while $1])])
dnl OVS_APP_EXIT_AND_WAIT(DAEMON)
diff --git a/tests/system-route.at b/tests/system-route.at
index 1714273e3..270956d13 100644
--- a/tests/system-route.at
+++ b/tests/system-route.at
@@ -14,10 +14,9 @@ dnl Add ip address.
AT_CHECK([ip addr add 10.0.0.17/24 dev p1-route], [0], [stdout])
dnl Check that OVS catches route updates.
-OVS_WAIT_UNTIL([ovs-appctl ovs/route/show | grep 'p1-route' | sort], [0], [dnl
-Cached: 10.0.0.17/24 dev p1-route SRC 10.0.0.17
-Cached: 10.0.0.17/32 dev p1-route SRC 10.0.0.17 local
-])
+OVS_WAIT_UNTIL_EQUAL([ovs-appctl ovs/route/show | grep 'p1-route' | sort], [dnl
+Cached: 10.0.0.0/24 dev p1-route SRC 10.0.0.17
+Cached: 10.0.0.17/32 dev p1-route SRC 10.0.0.17 local])
dnl Delete ip address.
AT_CHECK([ip addr del 10.0.0.17/24 dev p1-route], [0], [stdout])
--
2.34.1
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
