Re: [ovs-dev] [PATCH ovn] tests: fixed userspace-system tests not properly cleaned up

2023-07-04 Thread Ales Musil
On Tue, Jul 4, 2023 at 12:08 PM Xavier Simonart  wrote:

> Usually, when cleaning up a system test we dexecute (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).
>
> Signed-off-by: Xavier Simonart 
> ---
>  tests/system-ovn.at | 90 +
>  1 file changed, 90 insertions(+)
>
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 44a8072d6..8db08da70 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=,dport=4242),reply=(s
>
>  
> tcp,orig=(src=42.42.42.3,dst=66.66.66.66,sport=,dport=666),reply=(src=42.42.42.2,dst=42.42.42.3,sport=4242,dport=),zone=,mark=2,protoinfo=(state=)
>  ])
>
> +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=,dport=4242),reply=(src=424
>
>  
> tcp,orig=(src=4242::3,dst=::1,sport=,dport=666),reply=(src=4242::2,dst=4242::3,sport=4242,dport=),zone=,mark=2,protoinfo=(state=)
>  ])
>
> +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
>  ])
>
> @@ -7136,6 +7166,21 @@ as
>  OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
>  /.*terminating with signal 15.*/d"])
>
> +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
>  ])
>

The cleanup was already in this test (DNAT LR hairpin IPv4) the second one
makes it fail constantly.
I have also noticed one thing. Some tests perform only "kill $(pidof
ovn-controller)" instead of
"OVS_APP_EXIT_AND_WAIT([ovn-controller])". Would you mind replacing those
in v2 as well?


>
> @@ -7238,6 +7283,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
>  ])
>
> @@ -9168,6 +9228,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 +9382,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
>
>
Thanks,
Ales

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA 

amu...@redhat.comIM: amusil

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


Re: [ovs-dev] [PATCH ovn] tests: fixed userspace-system tests not properly cleaned up

2023-07-04 Thread Ales Musil
On Tue, Jul 4, 2023 at 12:08 PM Xavier Simonart  wrote:

> Usually, when cleaning up a system test we dexecute (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).
>
> Signed-off-by: Xavier Simonart 
> ---
>  tests/system-ovn.at | 90 +
>  1 file changed, 90 insertions(+)
>
> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
> index 44a8072d6..8db08da70 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=,dport=4242),reply=(s
>
>  
> tcp,orig=(src=42.42.42.3,dst=66.66.66.66,sport=,dport=666),reply=(src=42.42.42.2,dst=42.42.42.3,sport=4242,dport=),zone=,mark=2,protoinfo=(state=)
>  ])
>
> +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=,dport=4242),reply=(src=424
>
>  
> tcp,orig=(src=4242::3,dst=::1,sport=,dport=666),reply=(src=4242::2,dst=4242::3,sport=4242,dport=),zone=,mark=2,protoinfo=(state=)
>  ])
>
> +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
>  ])
>
> @@ -7136,6 +7166,21 @@ as
>  OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
>  /.*terminating with signal 15.*/d"])
>
> +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
>  ])
>
> @@ -7238,6 +7283,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
>  ])
>
> @@ -9168,6 +9228,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 +9382,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 

-- 

Ales Musil

Senior Software Engineer - OVN Core

Red Hat EMEA 

amu...@redhat.comIM: amusil

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH ovn] tests: fixed userspace-system tests not properly cleaned up

2023-07-04 Thread Xavier Simonart
Usually, when cleaning up a system test we dexecute (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).

Signed-off-by: Xavier Simonart 
---
 tests/system-ovn.at | 90 +
 1 file changed, 90 insertions(+)

diff --git a/tests/system-ovn.at b/tests/system-ovn.at
index 44a8072d6..8db08da70 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=,dport=4242),reply=(s
 
tcp,orig=(src=42.42.42.3,dst=66.66.66.66,sport=,dport=666),reply=(src=42.42.42.2,dst=42.42.42.3,sport=4242,dport=),zone=,mark=2,protoinfo=(state=)
 ])
 
+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=,dport=4242),reply=(src=424
 
tcp,orig=(src=4242::3,dst=::1,sport=,dport=666),reply=(src=4242::2,dst=4242::3,sport=4242,dport=),zone=,mark=2,protoinfo=(state=)
 ])
 
+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
 ])
 
@@ -7136,6 +7166,21 @@ as
 OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
 /.*terminating with signal 15.*/d"])
 
+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
 ])
 
@@ -7238,6 +7283,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
 ])
 
@@ -9168,6 +9228,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 +9382,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

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev