diff mbox series

[ovs-dev] system-traffic: Fix syntax errors in FTP and IPv6 curl calls.

Message ID 20241129220350.2747976-1-i.maximets@ovn.org
State Accepted
Commit 249a9b56e2b25da7d4b1593feffcf43e952ea9fc
Headers show
Series [ovs-dev] system-traffic: Fix syntax errors in FTP and IPv6 curl calls. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Ilya Maximets Nov. 29, 2024, 10:03 p.m. UTC 
The system testsuite is broken due to syntax errors:

1. Misplaced parenthesis, like:

   NS_CHECK_EXEC[at_ns0], (OVS_GET_FTP_ACTIVE...
   NS_CHECK_EXEC([at_ns0], OVS_GET_FTP([[[fc00::2]]]), [--ipv6]), [0], ...

   at-groups/122/test-source: line 757:
    syntax error near unexpected token `curl'
   at-groups/122/test-source: line 757:
   `at_ns0, (curl ftp://10.1.1.2 --retry 3 --max-time 1 --retry-connrefused -v \'

   system-kmod-testsuite: WARNING: unable to parse test group: 122

2. Insufficient escaping of brackets around IPv6 addresses.
   It must be 4 brackets to survive double macro expansion.

   curl ftp://fc00::2 --retry 3 ...
   curl: (3) URL rejected: Port number was not a decimal number between 0 and 65535

3. OVS_GET_FTP macro is missing a line continuation:

   sh: 2: -v: not found

Interestingly enough, when a testsuite fails with syntax errors, there
are no tests to re-check, so recheck succeeds and so the jobs are green
in GitHub Actions, even if they are not actually running a lot of tests.

While at it, adjusting formatting in OVS_GET_FTP_ACTIVE to match other
macros.

Fixes: 6bafaebf34fc ("system-traffic: Replace wget with curl for negative and ftp tests.")
Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 tests/system-common-macros.at |  7 +++----
 tests/system-traffic.at       | 22 +++++++++++-----------
 2 files changed, 14 insertions(+), 15 deletions(-)

Comments

Eelco Chaudron Dec. 2, 2024, 8:17 a.m. UTC | #1 
On 29 Nov 2024, at 23:03, Ilya Maximets wrote:

> The system testsuite is broken due to syntax errors:
>
> 1. Misplaced parenthesis, like:
>
>    NS_CHECK_EXEC[at_ns0], (OVS_GET_FTP_ACTIVE...
>    NS_CHECK_EXEC([at_ns0], OVS_GET_FTP([[[fc00::2]]]), [--ipv6]), [0], ...
>
>    at-groups/122/test-source: line 757:
>     syntax error near unexpected token `curl'
>    at-groups/122/test-source: line 757:
>    `at_ns0, (curl ftp://10.1.1.2 --retry 3 --max-time 1 --retry-connrefused -v \'
>
>    system-kmod-testsuite: WARNING: unable to parse test group: 122
>
> 2. Insufficient escaping of brackets around IPv6 addresses.
>    It must be 4 brackets to survive double macro expansion.
>
>    curl ftp://fc00::2 --retry 3 ...
>    curl: (3) URL rejected: Port number was not a decimal number between 0 and 65535
>
> 3. OVS_GET_FTP macro is missing a line continuation:
>
>    sh: 2: -v: not found
>
> Interestingly enough, when a testsuite fails with syntax errors, there
> are no tests to re-check, so recheck succeeds and so the jobs are green
> in GitHub Actions, even if they are not actually running a lot of tests.
>
> While at it, adjusting formatting in OVS_GET_FTP_ACTIVE to match other
> macros.
>
> Fixes: 6bafaebf34fc ("system-traffic: Replace wget with curl for negative and ftp tests.")
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

Thanks for noticing and fixing this Ilya! The changes look good to me.

Acked-by: Eelco Chaudron <echaudro@redhat.com>
Ilya Maximets Dec. 2, 2024, 12:56 p.m. UTC | #2 
On 12/2/24 09:17, Eelco Chaudron wrote:
> 
> 
> On 29 Nov 2024, at 23:03, Ilya Maximets wrote:
> 
>> The system testsuite is broken due to syntax errors:
>>
>> 1. Misplaced parenthesis, like:
>>
>>    NS_CHECK_EXEC[at_ns0], (OVS_GET_FTP_ACTIVE...
>>    NS_CHECK_EXEC([at_ns0], OVS_GET_FTP([[[fc00::2]]]), [--ipv6]), [0], ...
>>
>>    at-groups/122/test-source: line 757:
>>     syntax error near unexpected token `curl'
>>    at-groups/122/test-source: line 757:
>>    `at_ns0, (curl ftp://10.1.1.2 --retry 3 --max-time 1 --retry-connrefused -v \'
>>
>>    system-kmod-testsuite: WARNING: unable to parse test group: 122
>>
>> 2. Insufficient escaping of brackets around IPv6 addresses.
>>    It must be 4 brackets to survive double macro expansion.
>>
>>    curl ftp://fc00::2 --retry 3 ...
>>    curl: (3) URL rejected: Port number was not a decimal number between 0 and 65535
>>
>> 3. OVS_GET_FTP macro is missing a line continuation:
>>
>>    sh: 2: -v: not found
>>
>> Interestingly enough, when a testsuite fails with syntax errors, there
>> are no tests to re-check, so recheck succeeds and so the jobs are green
>> in GitHub Actions, even if they are not actually running a lot of tests.
>>
>> While at it, adjusting formatting in OVS_GET_FTP_ACTIVE to match other
>> macros.
>>
>> Fixes: 6bafaebf34fc ("system-traffic: Replace wget with curl for negative and ftp tests.")
>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> 
> Thanks for noticing and fixing this Ilya! The changes look good to me.
> 
> Acked-by: Eelco Chaudron <echaudro@redhat.com>
> 

Thanks!  I applied this change to all affected branches down to 3.3.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
index a235f2ba2..a8093ec4e 100644
--- a/tests/system-common-macros.at
+++ b/tests/system-common-macros.at
@@ -279,8 +279,8 @@  m4_define([OVS_GET_HTTP],
 # Do a passive FTP get; we are currently using the curl command.
 #
 m4_define([OVS_GET_FTP],
-    [curl ftp://$1 --retry 3 --max-time 1 --retry-connrefused --disable-epsv
-         -v $2]
+    [curl ftp://$1 --retry 3 --max-time 1 --retry-connrefused \
+        --disable-eps -v $2]
 )
 
 # OVS_GET_FTP_ACTIVE([url], [optional_curl_arguments])
@@ -289,8 +289,7 @@  m4_define([OVS_GET_FTP],
 #
 m4_define([OVS_GET_FTP_ACTIVE],
     [curl ftp://$1 --retry 3 --max-time 1 --retry-connrefused -v \
-        --ftp-port - --disable-eprt $2
-    ]
+        --ftp-port - --disable-eprt $2]
 )
 
 # OVS_CHECK_FIREWALL()
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 15b3391f8..d2de50ab0 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -5499,7 +5499,7 @@  OVS_START_L7([at_ns0], [http6])
 OVS_START_L7([at_ns1], [http6])
 
 dnl HTTP requests from ns0->ns1 should work fine.
-NS_CHECK_EXEC([at_ns0], OVS_GET_HTTP([http://[[fc00::2]]]), [0], [ignore], [ignore])
+NS_CHECK_EXEC([at_ns0], OVS_GET_HTTP([[http://[[fc00::2]]]]), [0], [ignore], [ignore])
 
 AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fc00::2)], [0], [dnl
 tcp,orig=(src=fc00::1,dst=fc00::2,sport=<cleared>,dport=<cleared>),reply=(src=fc00::2,dst=fc00::1,sport=<cleared>,dport=<cleared>),protoinfo=(state=<cleared>)
@@ -5507,7 +5507,7 @@  tcp,orig=(src=fc00::1,dst=fc00::2,sport=<cleared>,dport=<cleared>),reply=(src=fc
 
 dnl HTTP requests from ns1->ns0 should fail due to network failure.
 dnl Try 3 times, in 1 second intervals.
-NS_CHECK_EXEC([at_ns1], OVS_GET_HTTP([http://[[fc00::1]]]), [28], [ignore], [ignore])
+NS_CHECK_EXEC([at_ns1], OVS_GET_HTTP([[http://[[fc00::1]]]]), [28], [ignore], [ignore])
 
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
@@ -6056,7 +6056,7 @@  AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.1)], [0], [dnl
 ])
 
 dnl FTP requests from p0->p1 should work fine.
-NS_CHECK_EXEC[at_ns0], (OVS_GET_FTP_ACTIVE([10.1.1.2]), [0], [ignore], [ignore])
+NS_CHECK_EXEC([at_ns0], OVS_GET_FTP_ACTIVE([10.1.1.2]), [0], [ignore], [ignore])
 AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl
 tcp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=<cleared>,dport=<cleared>),reply=(src=10.1.1.2,dst=10.1.1.1,sport=<cleared>,dport=<cleared>),protoinfo=(state=<cleared>),helper=ftp
 ])
@@ -6293,7 +6293,7 @@  OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::2 >/dev/null])
 OVS_START_L7([at_ns1], [ftp])
 
 dnl FTP requests from p0->p1 should work fine.
-NS_CHECK_EXEC([at_ns0], OVS_GET_FTP_ACTIVE([[[fc00::2]]], [--ipv6]), [0], [ignore], [ignore])
+NS_CHECK_EXEC([at_ns0], OVS_GET_FTP_ACTIVE([[[[fc00::2]]]], [--ipv6]), [0], [ignore], [ignore])
 
 dnl Discards CLOSE_WAIT and CLOSING
 AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fc00::2)], [0], [dnl
@@ -6352,7 +6352,7 @@  OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::2 >/dev/null])
 OVS_START_L7([at_ns1], [ftp])
 
 dnl FTP passive requests from p0->p1 should work fine.
-NS_CHECK_EXEC([at_ns0], OVS_GET_FTP([[[fc00::2]]]), [--ipv6]), [0], [ignore], [ignore])
+NS_CHECK_EXEC([at_ns0], OVS_GET_FTP([[[[fc00::2]]]], [--ipv6]), [0], [ignore], [ignore])
 
 dnl Discards CLOSE_WAIT and CLOSING
 AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fc00::2)], [0], [dnl
@@ -7804,12 +7804,12 @@  OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::2])
 dnl HTTP requests from ns0->ns1 should work fine.
 OVS_START_L7([at_ns1], [http6])
 
-NS_CHECK_EXEC([at_ns0], OVS_GET_HTTP([http://[[fc00::2]]]), [0], [ignore], [ignore])
+NS_CHECK_EXEC([at_ns0], OVS_GET_HTTP([[http://[[fc00::2]]]]), [0], [ignore], [ignore])
 
 dnl HTTP requests from ns1->ns0 should fail due to network failure.
 dnl Try 3 times, in 1 second intervals.
 OVS_START_L7([at_ns0], [http6])
-NS_CHECK_EXEC([at_ns1], OVS_GET_HTTP([http://[[fc00::1]]]), [28], [ignore], [ignore])
+NS_CHECK_EXEC([at_ns1], OVS_GET_HTTP([[http://[[fc00::1]]]]), [28], [ignore], [ignore])
 
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
@@ -7848,7 +7848,7 @@  NS_CHECK_EXEC([at_ns0], [ping6 -q -c 3 -i 0.3 -W 2 fc00::240 | FORMAT_PING], [0]
 
 dnl Should work with the virtual IP address through NAT
 OVS_START_L7([at_ns1], [http6])
-NS_CHECK_EXEC([at_ns0], OVS_GET_HTTP([http://[[fc00::240]]]), [0], [ignore], [ignore])
+NS_CHECK_EXEC([at_ns0], OVS_GET_HTTP([[http://[[fc00::240]]]]), [0], [ignore], [ignore])
 
 AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fc00::1)], [0], [dnl
 icmpv6,orig=(src=fc00::1,dst=fc00::240,id=<cleared>,type=128,code=0),reply=(src=fc00::2,dst=fc00::1,id=<cleared>,type=129,code=0),zone=1
@@ -7998,7 +7998,7 @@  OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::2 >/dev/null])
 OVS_START_L7([at_ns1], [ftp])
 
 dnl FTP requests from p0->p1 should work fine.
-NS_CHECK_EXEC([at_ns0], OVS_GET_FTP_ACTIVE([[[fc00::2]]], [--ipv6]), [0], [ignore], [ignore])
+NS_CHECK_EXEC([at_ns0], OVS_GET_FTP_ACTIVE([[[[fc00::2]]]], [--ipv6]), [0], [ignore], [ignore])
 
 dnl Discards CLOSE_WAIT and CLOSING
 AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fc00::2)], [0], [dnl
@@ -8059,7 +8059,7 @@  OVS_WAIT_UNTIL([ip netns exec at_ns0 ping6 -c 1 fc00::2 >/dev/null])
 OVS_START_L7([at_ns1], [ftp])
 
 dnl FTP requests from p0->p1 should work fine.
-NS_CHECK_EXEC([at_ns0], OVS_GET_FTP([[[fc00::2]]], [--ipv6]), [0], [ignore], [ignore])
+NS_CHECK_EXEC([at_ns0], OVS_GET_FTP([[[[fc00::2]]]], [--ipv6]), [0], [ignore], [ignore])
 
 dnl Discards CLOSE_WAIT and CLOSING
 AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fc00::2)], [0], [dnl
@@ -8119,7 +8119,7 @@  NETNS_DAEMONIZE([at_ns1], [[$PYTHON3 $srcdir/test-l7.py ftp]], [ftp0.pid])
 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], OVS_GET_FTP_ACTIVE([[[fc00::2]]], [--ipv6]), [0], [ignore], [ignore])
+NS_CHECK_EXEC([at_ns0], OVS_GET_FTP_ACTIVE([[[[fc00::2]]]], [--ipv6]), [0], [ignore], [ignore])
 
 dnl Discards CLOSE_WAIT and CLOSING
 AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(fc00::2)], [0], [dnl