On 7/7/23 10:46, Ales Musil wrote: > On Wed, Jul 5, 2023 at 5:57 PM Xavier Simonart <[email protected]> wrote: > >> Usually, when cleaning up a system test we execute (for the ovn-vswitchd >> process): >> - OVS_TRAFFIC_VSWITCHD_STOP (i.e. ovs-appctl -t $1 exit --cleanup, wait up >> to 30 seconds >> and report failure if not properly cleaned up). >> - kill_ovs_vswitchd (i.e. if ovs-vsitwhd still running, >> ovs-appctl -t ovs-vswitchd exit --cleanup, wait 1 to 10 seconds and kill >> ovs-vswitvchd >> if not properly cleaned up). >> >> If we do not execute OVS_TRAFFIC_VSWITCHD_STOP, wait time is much smaller >> which might result in >> - clean up not done properly, ovs-vswitchd killed, ovs-netdev not deleted, >> and no error reported >> - next tests will fail >> >> Using this patch, we properly execute OVS_TRAFFIC_VSWITCHD_STOP, so the >> delay waiting for >> ovs-vswitchd to exit is longer, and, if by any chance this delay is still >> too short, >> the proper test will fail (i.e. the first test failing will be the one >> unable to clean). >> >> Note that, after this patch, some extra tests might fail such as >> "load-balancer template IPv4". >> This is due to the fact that, instead of killing ovn-controller at exit, >> we now properly exit and check the unexpected presence of warnings. In >> other words, we might now higlight some existing issues which were >> ignoring before.
Hi Xavier, Ales, Ales shared this incremental privately which adds the "offending" warnings to the list of ignored ones. At the same time we opened a bug to track the reason why the warning was issued in the first place: https://bugzilla.redhat.com/show_bug.cgi?id=2222252 diff --git a/tests/ofproto-macros.at b/tests/ofproto-macros.at index f4ebdafc72..b0101330f5 100644 --- a/tests/ofproto-macros.at +++ b/tests/ofproto-macros.at @@ -287,6 +287,7 @@ check_logs () { /timeval.*context switches: [[0-9]]* voluntary, [[0-9]]* involuntary/d /ovs_rcu.*blocked [[0-9]]* ms waiting for .* to quiesce/d /Dropped [[0-9]]* log messages/d +/.*Trying to release unknown interface.*/d /|WARN|/p /|ERR|/p /|EMER|/p" ${logs} --- I had another look at this patch and I think we should also add the following change: diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at index 6b6181e8cc..97c6e433bb 100644 --- a/tests/system-common-macros.at +++ b/tests/system-common-macros.at @@ -488,7 +488,7 @@ AT_CHECK([ovn-nbctl get logical_router_port rp-sw0 ipv6_prefix | cut -c3-16], [0 [] ]) -kill $(pidof ovn-controller) +OVS_APP_EXIT_AND_WAIT([ovn-controller]) as ovn-sb OVS_APP_EXIT_AND_WAIT([ovsdb-server]) --- If this looks OK to you can fold both of them in and remove the note from the commit log. Please let me know what you think. Thanks, Dumitru >> >> Signed-off-by: Xavier Simonart <[email protected]> >> >> --- >> v2: - updated based on Ales' feedback: >> - some cleanup wrongly done twice (e.g. for DNAT LR hairpin IPv4) >> causing test to always fail >> - replaced the 'kill ovn-controller' in some tests by proper cleanup >> - added a few additional missing OVS_WAIT_AND_EXIT (ovsdb-server) >> --- >> tests/system-ovn.at | 103 ++++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 99 insertions(+), 4 deletions(-) >> >> diff --git a/tests/system-ovn.at b/tests/system-ovn.at >> index 44a8072d6..8f6a647a0 100644 >> --- a/tests/system-ovn.at >> +++ b/tests/system-ovn.at >> @@ -5819,6 +5819,21 @@ >> tcp,orig=(src=42.42.42.3,dst=42.42.42.2,sport=<clnt_s_port>,dport=4242),reply=(s >> >> >> tcp,orig=(src=42.42.42.3,dst=66.66.66.66,sport=<clnt_s_port>,dport=666),reply=(src=42.42.42.2,dst=42.42.42.3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>) >> ]) >> >> +OVS_APP_EXIT_AND_WAIT([ovn-controller]) >> + >> +as ovn-sb >> +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> + >> +as ovn-nb >> +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> + >> +as northd >> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE]) >> + >> +as >> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d >> +/connection dropped.*/d"]) >> + >> AT_CLEANUP >> ]) >> >> @@ -5915,6 +5930,21 @@ >> tcp,orig=(src=4242::3,dst=4242::2,sport=<clnt_s_port>,dport=4242),reply=(src=424 >> >> >> tcp,orig=(src=4242::3,dst=6666::1,sport=<clnt_s_port>,dport=666),reply=(src=4242::2,dst=4242::3,sport=4242,dport=<clnt_s_port>),zone=<cleared>,mark=2,protoinfo=(state=<cleared>) >> ]) >> >> +OVS_APP_EXIT_AND_WAIT([ovn-controller]) >> + >> +as ovn-sb >> +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> + >> +as ovn-nb >> +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> + >> +as northd >> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE]) >> + >> +as >> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d >> +/connection dropped.*/d"]) >> + >> AT_CLEANUP >> ]) >> >> @@ -6716,7 +6746,7 @@ AT_CHECK([ovn-nbctl set Logical_Switch_Port sw02 >> options:qos_burst=1000000]) >> OVS_WAIT_UNTIL([tc class show dev ovs-public | \ >> grep -q 'class htb .* prio 0 rate 5Gbit ceil 6Gbit burst >> 125000b cburst 124500b']) >> >> -kill $(pidof ovn-controller) >> +OVS_APP_EXIT_AND_WAIT([ovn-controller]) >> >> as ovn-sb >> OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> @@ -6867,7 +6897,7 @@ Allowing connections from 1000::a >> wait_column "up" nb:bfd status logical_port=rp-public >> ovn-nbctl destroy bfd $uuid_v6 >> >> -kill $(pidof ovn-controller) >> +OVS_APP_EXIT_AND_WAIT([ovn-controller]) >> >> as ovn-sb >> OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> @@ -7121,7 +7151,7 @@ NS_CHECK_EXEC([vm2], [ping -q -c 3 -i 0.3 -w 2 >> 172.18.2.10 | FORMAT_PING], \ >> 3 packets transmitted, 3 received, 0% packet loss, time 0ms >> ]) >> >> -kill $(pidof ovn-controller) >> +OVS_APP_EXIT_AND_WAIT([ovn-controller]) >> >> as ovn-sb >> OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> @@ -7238,6 +7268,21 @@ NS_CHECK_EXEC([vm1], [ping -q -c 3 -i 0.3 -w 2 >> 172.18.2.12 | FORMAT_PING], \ >> 3 packets transmitted, 3 received, 0% packet loss, time 0ms >> ]) >> >> +OVS_APP_EXIT_AND_WAIT([ovn-controller]) >> + >> +as ovn-sb >> +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> + >> +as ovn-nb >> +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> + >> +as northd >> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE]) >> + >> +as >> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d >> +/connection dropped.*/d"]) >> + >> AT_CLEANUP >> ]) >> >> @@ -7421,7 +7466,7 @@ OVS_WAIT_UNTIL([ >> ]) >> kill $(pidof tcpdump) >> >> -kill $(pidof ovn-controller) >> +OVS_APP_EXIT_AND_WAIT([ovn-controller]) >> >> as ovn-sb >> OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> @@ -8494,6 +8539,16 @@ >> tcp,orig=(src=192.168.2.2,dst=172.16.1.2,sport=<cleared>,dport=<cleared>),reply= >> AT_CHECK([ovs-appctl dpctl/flush-conntrack]) >> >> OVS_APP_EXIT_AND_WAIT([ovn-controller]) >> + >> +as ovn-sb >> +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> + >> +as ovn-nb >> +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> + >> +as northd >> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE]) >> + >> as >> OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d >> /connection dropped.*/d"]) >> @@ -8646,6 +8701,16 @@ NS_CHECK_EXEC([ls1p1], [ping6 -q -c 3 -i 0.3 -w 2 >> 1711::1 | FORMAT_PING], \ >> ]) >> >> OVS_APP_EXIT_AND_WAIT([ovn-controller]) >> + >> +as ovn-sb >> +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> + >> +as ovn-nb >> +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> + >> +as northd >> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE]) >> + >> as >> OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d >> /connection dropped.*/d >> @@ -9168,6 +9233,21 @@ OVS_WAIT_UNTIL([ >> test "${requests}" -ge "6" >> ]) >> >> +OVS_APP_EXIT_AND_WAIT([ovn-controller]) >> + >> +as ovn-sb >> +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> + >> +as ovn-nb >> +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> + >> +as northd >> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE]) >> + >> +as >> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d >> +/connection dropped.*/d"]) >> + >> AT_CLEANUP >> ]) >> >> @@ -9307,6 +9387,21 @@ OVS_WAIT_UNTIL([ >> test "${requests}" -ge "6" >> ]) >> >> +OVS_APP_EXIT_AND_WAIT([ovn-controller]) >> + >> +as ovn-sb >> +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> + >> +as ovn-nb >> +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) >> + >> +as northd >> +OVS_APP_EXIT_AND_WAIT([NORTHD_TYPE]) >> + >> +as >> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d >> +/connection dropped.*/d"]) >> + >> AT_CLEANUP >> ]) >> >> -- >> 2.31.1 >> >> > Looks good to me, thanks! > > Acked-by: Ales Musil <[email protected]> > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
