On Wed, Jul 12, 2023 at 8:20 PM Dumitru Ceara <[email protected]> wrote:
> 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 > > Hi Ales, Dumitru Both look OK to me. Thanks, Dumitru and Ales Thanks Xavier > >> > >> 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
