[ovs-dev,2/5] system-traffic: Wait for L7 servers to start.

Use OVS_WAIT_UNTIL() with netstat to ensure servers are listening before
sending requests to them. This allows us to drop --retry-connrefused on
wget, needed before we can switch to curl.

Signed-off-by: Joe Stringer <joe@ovn.org>
---
 tests/system-common-macros.at | 4 ++++
 tests/system-traffic.at       | 4 ----
 2 files changed, 4 insertions(+), 4 deletions(-)

2016-12-20 13:28 GMT-08:00 Joe Stringer <joe@ovn.org>:
> Use OVS_WAIT_UNTIL() with netstat to ensure servers are listening before
> sending requests to them. This allows us to drop --retry-connrefused on
> wget, needed before we can switch to curl.

This commit doesn't actually remove --retry-connrefused.  Should we
remove --retry-connrefused or should we update the commit message
(maybe I misunderstood)?  Also, the last patch introduces --retry 2 in
CURL_OPT, so I'm assuming we should keep --retry-connrefused.

>
> Signed-off-by: Joe Stringer <joe@ovn.org>

Thanks for doing that, it's a nice improvement!

Acked-by: Daniele Di Proietto <diproiettod@vmware.com>

> ---
>  tests/system-common-macros.at | 4 ++++
>  tests/system-traffic.at       | 4 ----
>  2 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
> index eb5478d38d1b..90f6065c1ba4 100644
> --- a/tests/system-common-macros.at
> +++ b/tests/system-common-macros.at
> @@ -238,6 +238,10 @@ m4_define([OVS_CHECK_FIREWALL],
>  m4_define([OVS_START_L7],
>     [PIDFILE=$(mktemp $2XXX.log)
>      NETNS_DAEMONIZE([$1], [[$PYTHON $srcdir/test-l7.py $2]], [$PIDFILE])
> +
> +    dnl netstat doesn't print http over IPv6 as "http6"; drop the number.
> +    PROTO=$(echo $2 | sed -e 's/\([[a-zA-Z]]*\).*/\1/')
> +    OVS_WAIT_UNTIL([NS_EXEC([$1], [netstat -l | grep $PROTO])])
>     ]
>  )
>
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index 67e06a11805b..803b00850511 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -1642,7 +1642,6 @@ AT_CHECK([ovs-ofctl --bundle replace-flows br0 flows1.txt])
>
>  OVS_START_L7([at_ns0], [ftp])
>  OVS_START_L7([at_ns1], [ftp])
> -OVS_WAIT_UNTIL([ip netns exec at_ns1 netstat -l | grep ftp])
>
>  dnl FTP requests from p1->p0 should fail due to network failure.
>  dnl Try 3 times, in 1 second intervals.
> @@ -1727,7 +1726,6 @@ dnl "connect: Cannot assign requested address"
>  OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::2 >/dev/null])
>
>  OVS_START_L7([at_ns1], [ftp])
> -OVS_WAIT_UNTIL([ip netns exec at_ns1 netstat -l | grep ftp])
>
>  dnl FTP requests from p0->p1 should work fine.
>  NS_CHECK_EXEC([at_ns0], [wget ftp://[[fc00::2]] -6 --no-passive-ftp -t 3 -T 1 --retry-connrefused -v --server-response --no-remove-listing -o wget0.log -d])
> @@ -2534,7 +2532,6 @@ m4_define([CHECK_FTP_NAT],
>      AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt])
>
>      OVS_START_L7([at_ns1], [ftp])
> -    OVS_WAIT_UNTIL([ip netns exec at_ns1 netstat -l | grep ftp])
>
>      dnl FTP requests from p0->p1 should work fine.
>      NS_CHECK_EXEC([at_ns0], [wget ftp://10.1.1.2 -4 --no-passive-ftp -t 3 -T 1 --retry-connrefused -v --server-response --no-remove-listing -o wget0.log -d])
> @@ -2772,7 +2769,6 @@ dnl "connect: Cannot assign requested address"
>  OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::2 >/dev/null])
>
>  OVS_START_L7([at_ns1], [ftp])
> -OVS_WAIT_UNTIL([ip netns exec at_ns1 netstat -l | grep ftp])
>
>  dnl FTP requests from p0->p1 should work fine.
>  NS_CHECK_EXEC([at_ns0], [wget ftp://[[fc00::2]] -6 --no-passive-ftp -t 3 -T 1 --retry-connrefused -v --server-response --no-remove-listing -o wget0.log -d])
> --
> 2.10.2
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev

On 21 December 2016 at 17:05, Daniele Di Proietto <diproiettod@ovn.org> wrote:
> 2016-12-20 13:28 GMT-08:00 Joe Stringer <joe@ovn.org>:
>> Use OVS_WAIT_UNTIL() with netstat to ensure servers are listening before
>> sending requests to them. This allows us to drop --retry-connrefused on
>> wget, needed before we can switch to curl.
>
> This commit doesn't actually remove --retry-connrefused.  Should we
> remove --retry-connrefused or should we update the commit message
> (maybe I misunderstood)?  Also, the last patch introduces --retry 2 in
> CURL_OPT, so I'm assuming we should keep --retry-connrefused.

Ah, let me drop that from the commit message. This helps with that,
but the subsequent patch that updates that is missing from this series
;) (I've got it in a queue somewhere but I'd rather get these
refactoring patches in first)

>>
>> Signed-off-by: Joe Stringer <joe@ovn.org>
>
> Thanks for doing that, it's a nice improvement!
>
> Acked-by: Daniele Di Proietto <diproiettod@vmware.com>

Thanks for the review.