From 060feeb454f1ebf061af8213624738e723f94520 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Wed, 15 Jun 2022 12:57:14 +0000 Subject: [PATCH 1/6] Fix DiG UDP query retry and fail-over bug When the `udp_ready()` callback function gets called with a failure result code, DiG erroneously cancels the lookup. Copy the logic behind `tcp_connected()` callback function into `udp_ready()` so that DiG will now retry the failed query (if retries are enabled) and then, if it fails again, it will fail-over to the next server in the list, which synchronizes the behavior between TCP and UDP modes. Also, `udp_ready()` was calling `lookup_detach()` without calling `lookup_attach()` first, but the issue was masked behind the fact that `clear_current_lookup()` wasn't being called when needed, and `lookup_detach()` was compensating for that. This also has been fixed. --- bin/dig/dighost.c | 48 ++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 5 deletions(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 8ef933b16d..e7bd59b1c9 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -3103,6 +3103,9 @@ send_udp(dig_query_t *query) { static void udp_ready(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { dig_query_t *query = (dig_query_t *)arg; + dig_query_t *next = NULL; + char sockstr[ISC_SOCKADDR_FORMATSIZE]; + dig_lookup_t *l = NULL; dig_query_t *readquery = NULL; int local_timeout = timeout * 1000; @@ -3123,30 +3126,63 @@ udp_ready(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { debug("udp_ready(%p, %s, %p)", handle, isc_result_totext(eresult), query); - if (eresult == ISC_R_CANCELED || query->canceled) { - dig_lookup_t *l = query->lookup; + LOCK_LOOKUP; + lookup_attach(query->lookup, &l); + if (eresult == ISC_R_CANCELED || query->canceled) { debug("in cancel handler"); if (!query->canceled) { cancel_lookup(l); } query_detach(&query); lookup_detach(&l); + clear_current_lookup(); + UNLOCK_LOOKUP; return; } else if (eresult != ISC_R_SUCCESS) { - dig_lookup_t *l = query->lookup; - debug("udp setup failed: %s", isc_result_totext(eresult)); + isc_sockaddr_format(&query->sockaddr, sockstr, sizeof(sockstr)); + dighost_warning("UDP setup with %s(%s) for %s failed: %s.", + sockstr, query->servname, l->textname, + isc_result_totext(eresult)); if (exitcode < 9) { exitcode = 9; } + + if (l->retries > 1) { + l->retries--; + debug("making new UDP request, %d tries left", + l->retries); + requeue_lookup(l, true); + next = NULL; + } else if ((l->current_query != NULL) && + (ISC_LINK_LINKED(l->current_query, link))) + { + next = ISC_LIST_NEXT(l->current_query, link); + } else { + next = NULL; + } + query_detach(&query); - cancel_lookup(l); + if (next == NULL) { + cancel_lookup(l); + } lookup_detach(&l); + + if (next != NULL) { + start_udp(next); + } else { + clear_current_lookup(); + } + + check_if_done(); + UNLOCK_LOOKUP; return; } + exitcode = 0; + query_attach(query, &readquery); debug("recving with lookup=%p, query=%p, handle=%p", query->lookup, @@ -3168,6 +3204,8 @@ udp_ready(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { send_udp(readquery); query_detach(&query); + lookup_detach(&l); + UNLOCK_LOOKUP; } /*% From 99085c587e0cd235797f980810d32b9dcf960df2 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Wed, 15 Jun 2022 12:58:00 +0000 Subject: [PATCH 2/6] Fix DiG query retry and fail-over bug When the `send_done()` callback function gets called with a failure result code, DiG erroneously cancels the lookup. Stop canceling the lookup and give DiG a chance to retry the failed query, or fail-over to another server, using the logic implemented in the `recv_done()` callback function. --- bin/dig/dighost.c | 1 - 1 file changed, 1 deletion(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index e7bd59b1c9..418540d1b4 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -2695,7 +2695,6 @@ send_done(isc_nmhandle_t *handle, isc_result_t eresult, void *arg) { return; } else if (eresult != ISC_R_SUCCESS) { debug("send failed: %s", isc_result_totext(eresult)); - cancel_lookup(l); query_detach(&query); lookup_detach(&l); UNLOCK_LOOKUP; From 8611aa759f7eb1fe4c439bfbbabc6b882e49e081 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Wed, 15 Jun 2022 13:41:10 +0000 Subject: [PATCH 3/6] DiG: use the same retry and fail-over logic for different failure types DiG implements different logic in the `recv_done()` callback function when processing a failure: 1. For a timed-out query it applies the "retries" logic first, then, when it fails, fail-overs to the next server. 2. For an EOF (end-of-file, or unexpected disconnect) error it tries to make a single retry attempt (even if the user has requested more retries), then, when it fails, fail-overs to the next server. 3. For other types of failures, DiG does not apply the "retries" logic, and tries to fail-over to the next servers (again, even if the user has requested to make retries). Simplify the logic and apply the same logic (1) of first retries, and then fail-over, for different types of failures in `recv_done()`. --- bin/dig/dighost.c | 84 ++++----------------------- bin/dig/dighost.h | 1 - bin/tests/system/acl/tests.sh | 3 +- bin/tests/system/legacy/tests.sh | 15 +++-- bin/tests/system/rpz/tests.sh | 2 +- bin/tests/system/serve-stale/tests.sh | 6 +- 6 files changed, 27 insertions(+), 84 deletions(-) diff --git a/bin/dig/dighost.c b/bin/dig/dighost.c index 418540d1b4..ad773d1df5 100644 --- a/bin/dig/dighost.c +++ b/bin/dig/dighost.c @@ -809,7 +809,6 @@ clone_lookup(dig_lookup_t *lookold, bool servers) { looknew->done_as_is = lookold->done_as_is; looknew->dscp = lookold->dscp; looknew->rrcomments = lookold->rrcomments; - looknew->eoferr = lookold->eoferr; if (lookold->ecs_addr != NULL) { size_t len = sizeof(isc_sockaddr_t); @@ -3358,8 +3357,7 @@ force_next(dig_query_t *query) { dighost_error("no response from %s\n", buf); } else { printf("%s", l->cmdline); - dighost_error("connection timed out; " - "no servers could be reached\n"); + dighost_error("no servers could be reached\n"); } if (exitcode < 9) { @@ -3373,28 +3371,6 @@ force_next(dig_query_t *query) { UNLOCK_LOOKUP; } -/*% - * Called when a peer closes a TCP socket prematurely. - */ -static void -requeue_or_update_exitcode(dig_lookup_t *lookup) { - if (lookup->eoferr == 0U && lookup->retries > 1) { - --lookup->retries; - /* - * Peer closed the connection prematurely for the first time - * for this lookup. Try again, keeping track of this failure. - */ - dig_lookup_t *requeued_lookup = requeue_lookup(lookup, true); - requeued_lookup->eoferr++; - } else { - /* - * Peer closed the connection prematurely and it happened - * previously for this lookup. Indicate an error. - */ - exitcode = 9; - } -} - /*% * For transfers that involve multiple recvs (XFR's in particular), * launch the next recv. @@ -3959,7 +3935,13 @@ recv_done(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, } } - if (eresult == ISC_R_TIMEDOUT) { + if (eresult != ISC_R_SUCCESS) { + char sockstr[ISC_SOCKADDR_FORMATSIZE]; + + isc_sockaddr_format(&query->sockaddr, sockstr, sizeof(sockstr)); + dighost_warning("communications error to %s: %s", sockstr, + isc_result_totext(eresult)); + if (l->retries > 1 && !l->tcp_mode) { dig_query_t *newq = NULL; @@ -4030,8 +4012,8 @@ recv_done(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, * and cancel the lookup. */ printf("%s", l->cmdline); - dighost_error("connection timed out; " - "no servers could be reached\n"); + dighost_error("no servers could be reached\n"); + if (exitcode < 9) { exitcode = 9; } @@ -4042,52 +4024,6 @@ recv_done(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, goto cancel_lookup; } - } else if (eresult != ISC_R_SUCCESS) { - dig_query_t *next = ISC_LIST_NEXT(query, link); - char sockstr[ISC_SOCKADDR_FORMATSIZE]; - isc_sockaddr_format(&query->sockaddr, sockstr, sizeof(sockstr)); - - /* - * There was a communication error with the current query, - * go to the next query, if there is one. - */ - if (next != NULL) { - if (l->current_query == query) { - query_detach(&l->current_query); - } - if (l->current_query == NULL) { - debug("starting next query %p", next); - if (l->tcp_mode) { - start_tcp(next); - } else { - start_udp(next); - } - } - if (check_if_queries_done(l, query)) { - goto cancel_lookup; - } - - goto detach_query; - } - - /* - * Otherwise, print an error message and cancel the - * lookup. - */ - dighost_error("communications error to %s: %s\n", sockstr, - isc_result_totext(eresult)); - - if (keep != NULL) { - isc_nmhandle_detach(&keep); - } - - if (eresult == ISC_R_EOF) { - requeue_or_update_exitcode(l); - } else if (exitcode < 9) { - exitcode = 9; - } - - goto cancel_lookup; } isc_buffer_init(&b, region->base, region->length); diff --git a/bin/dig/dighost.h b/bin/dig/dighost.h index d7246333e0..e9da8f618e 100644 --- a/bin/dig/dighost.h +++ b/bin/dig/dighost.h @@ -169,7 +169,6 @@ struct dig_lookup { unsigned int ednsflags; dns_opcode_t opcode; int rrcomments; - unsigned int eoferr; uint16_t qid; struct { bool http_plain; diff --git a/bin/tests/system/acl/tests.sh b/bin/tests/system/acl/tests.sh index fe54ef57f1..df23d6a2f1 100644 --- a/bin/tests/system/acl/tests.sh +++ b/bin/tests/system/acl/tests.sh @@ -161,7 +161,8 @@ grep "status: NOERROR" dig.out.3.${t} > /dev/null 2>&1 || ret=1 $DIG -p ${PORT} soa example. \ @10.53.0.2 -b 10.53.0.8 > dig.out.4.${t} grep "status: NOERROR" dig.out.4.${t} > /dev/null 2>&1 && ret=1 -grep "connection timed out" dig.out.4.${t} > /dev/null 2>&1 || ret=1 +grep "timed out" dig.out.4.${t} > /dev/null 2>&1 || ret=1 +grep ";; no servers could be reached" dig.out.4.${t} > /dev/null 2>&1 || ret=1 [ $ret -eq 0 ] || echo_i "failed" status=`expr $status + $ret` diff --git a/bin/tests/system/legacy/tests.sh b/bin/tests/system/legacy/tests.sh index 80d70899fb..5adeead291 100755 --- a/bin/tests/system/legacy/tests.sh +++ b/bin/tests/system/legacy/tests.sh @@ -106,7 +106,8 @@ n=`expr $n + 1` echo_i "checking drop edns server setup ($n)" ret=0 $DIG $DIGOPTS +edns @10.53.0.2 dropedns soa > dig.out.1.test$n && ret=1 -grep "connection timed out; no servers could be reached" dig.out.1.test$n > /dev/null || ret=1 +grep "timed out" dig.out.1.test$n > /dev/null || ret=1 +grep ";; no servers could be reached" dig.out.1.test$n > /dev/null || ret=1 $DIG $DIGOPTS +noedns @10.53.0.2 dropedns soa > dig.out.2.test$n || ret=1 grep "status: NOERROR" dig.out.2.test$n > /dev/null || ret=1 grep "EDNS: version:" dig.out.2.test$n > /dev/null && ret=1 @@ -114,7 +115,8 @@ $DIG $DIGOPTS +noedns +tcp @10.53.0.2 dropedns soa > dig.out.3.test$n || ret=1 grep "status: NOERROR" dig.out.3.test$n > /dev/null || ret=1 grep "EDNS: version:" dig.out.3.test$n > /dev/null && ret=1 $DIG $DIGOPTS +edns +tcp @10.53.0.2 dropedns soa > dig.out.4.test$n && ret=1 -grep "connection timed out; no servers could be reached" dig.out.4.test$n > /dev/null || ret=1 +grep "timed out" dig.out.4.test$n > /dev/null || ret=1 +grep ";; no servers could be reached" dig.out.4.test$n > /dev/null || ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi status=`expr $status + $ret` @@ -129,7 +131,8 @@ n=`expr $n + 1` echo_i "checking drop edns + no tcp server setup ($n)" ret=0 $DIG $DIGOPTS +edns @10.53.0.3 dropedns-notcp soa > dig.out.1.test$n && ret=1 -grep "connection timed out; no servers could be reached" dig.out.1.test$n > /dev/null || ret=1 +grep "timed out" dig.out.1.test$n > /dev/null || ret=1 +grep ";; no servers could be reached" dig.out.1.test$n > /dev/null || ret=1 $DIG $DIGOPTS +noedns +tcp @10.53.0.3 dropedns-notcp soa > dig.out.2.test$n && ret=1 grep "connection refused" dig.out.2.test$n > /dev/null || ret=1 $DIG $DIGOPTS +noedns @10.53.0.3 dropedns-notcp soa > dig.out.3.test$n || ret=1 @@ -192,7 +195,8 @@ $DIG $DIGOPTS +edns +tcp @10.53.0.6 edns512 txt > dig.out.2.test$n || ret=1 grep "status: NOERROR" dig.out.2.test$n > /dev/null || ret=1 grep "EDNS: version:" dig.out.2.test$n > /dev/null || ret=1 $DIG $DIGOPTS +edns +dnssec @10.53.0.6 edns512 txt > dig.out.3.test$n && ret=1 -grep "connection timed out; no servers could be reached" dig.out.3.test$n > /dev/null || ret=1 +grep "timed out" dig.out.3.test$n > /dev/null || ret=1 +grep ";; no servers could be reached" dig.out.3.test$n > /dev/null || ret=1 $DIG $DIGOPTS +edns +dnssec +bufsize=512 +ignore @10.53.0.6 edns512 soa > dig.out.4.test$n || ret=1 grep "status: NOERROR" dig.out.4.test$n > /dev/null || ret=1 grep "EDNS: version:" dig.out.4.test$n > /dev/null || ret=1 @@ -216,7 +220,8 @@ grep "EDNS: version:" dig.out.1.test$n > /dev/null || ret=1 $DIG $DIGOPTS +edns +tcp @10.53.0.7 edns512-notcp soa > dig.out.2.test$n && ret=1 grep "connection refused" dig.out.2.test$n > /dev/null || ret=1 $DIG $DIGOPTS +edns +dnssec @10.53.0.7 edns512-notcp soa > dig.out.3.test$n && ret=1 -grep "connection timed out; no servers could be reached" dig.out.3.test$n > /dev/null || ret=1 +grep "timed out" dig.out.3.test$n > /dev/null || ret=1 +grep ";; no servers could be reached" dig.out.3.test$n > /dev/null || ret=1 $DIG $DIGOPTS +edns +dnssec +bufsize=512 +ignore @10.53.0.7 edns512-notcp soa > dig.out.4.test$n || ret=1 grep "status: NOERROR" dig.out.4.test$n > /dev/null || ret=1 grep "EDNS: version:" dig.out.4.test$n > /dev/null || ret=1 diff --git a/bin/tests/system/rpz/tests.sh b/bin/tests/system/rpz/tests.sh index 49a8316b08..9975b686cf 100644 --- a/bin/tests/system/rpz/tests.sh +++ b/bin/tests/system/rpz/tests.sh @@ -423,7 +423,7 @@ here () { } # check dropped response -DROPPED='^;; connection timed out; no servers could be reached' +DROPPED='^;; no servers could be reached' drop () { make_dignm digcmd $* >$DIGNM diff --git a/bin/tests/system/serve-stale/tests.sh b/bin/tests/system/serve-stale/tests.sh index 2e5ea6ab5b..d38bc7ef64 100755 --- a/bin/tests/system/serve-stale/tests.sh +++ b/bin/tests/system/serve-stale/tests.sh @@ -1135,7 +1135,8 @@ n=$((n+1)) echo_i "check notincache.example times out (max-stale-ttl default) ($n)" ret=0 $DIG -p ${PORT} +tries=1 +timeout=3 @10.53.0.3 notfound.example TXT > dig.out.test$n 2>&1 -grep "connection timed out" dig.out.test$n > /dev/null || ret=1 +grep "timed out" dig.out.test$n > /dev/null || ret=1 +grep ";; no servers could be reached" dig.out.test$n > /dev/null || ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status+ret)) @@ -1764,7 +1765,8 @@ n=$((n+1)) echo_i "check not in cache longttl.example times out (stale-answer-client-timeout 1.8) ($n)" ret=0 wait_for_log 4 "longttl.example client timeout, stale answer unavailable" ns3/named.run || ret=1 -grep "connection timed out" dig.out.test$n > /dev/null || ret=1 +grep "timed out" dig.out.test$n > /dev/null || ret=1 +grep ";; no servers could be reached" dig.out.test$n > /dev/null || ret=1 if [ $ret != 0 ]; then echo_i "failed"; fi status=$((status+ret)) From c1eaf16293b7db88b1ffb15021dd907557daa8cb Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Mon, 20 Jun 2022 12:13:44 +0000 Subject: [PATCH 4/6] Add "digdelv" test to simulate DiG network unreachable error There are existing tests for simulating timeouts, read errors, and refused connecion errors. Implement also "network unreachable" simulation. Use "fixed" string search mode `-F` for `grep` in more places where it is appropriate to do so. --- bin/tests/system/digdelv/tests.sh | 32 ++++++++++++++++++++++++------- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/bin/tests/system/digdelv/tests.sh b/bin/tests/system/digdelv/tests.sh index 767e33f8e6..939560737f 100644 --- a/bin/tests/system/digdelv/tests.sh +++ b/bin/tests/system/digdelv/tests.sh @@ -1005,7 +1005,7 @@ if [ -x "$DIG" ] ; then echo "unstable" | sendcmd 10.53.0.8 ret=0 dig_with_opts +timeout=1 +nofail @10.53.0.8 a.example > dig.out.test$n 2>&1 || ret=1 - grep "status: SERVFAIL" dig.out.test$n > /dev/null || ret=1 + grep -F "status: SERVFAIL" dig.out.test$n > /dev/null || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) @@ -1015,7 +1015,25 @@ if [ -x "$DIG" ] ; then echo "unstable" | sendcmd 10.53.0.8 ret=0 dig_with_opts +timeout=1 +nofail +tcp @10.53.0.8 a.example > dig.out.test$n 2>&1 || ret=1 - grep "status: SERVFAIL" dig.out.test$n > /dev/null || ret=1 + grep -F "status: SERVFAIL" dig.out.test$n > /dev/null || ret=1 + if [ $ret -ne 0 ]; then echo_i "failed"; fi + status=$((status+ret)) + + n=$((n+1)) + echo_i "check that dig tries the next server after a UDP socket network unreachable error ($n)" + ret=0 + dig_with_opts @192.0.2.128 @10.53.0.3 a.example > dig.out.test$n 2>&1 || ret=1 + test $(grep -F -e "connection refused" -e "timed out" -e "network unreachable" dig.out.test$n | wc -l) -eq 3 || ret=1 + grep -F "status: NOERROR" dig.out.test$n > /dev/null || ret=1 + if [ $ret -ne 0 ]; then echo_i "failed"; fi + status=$((status+ret)) + + n=$((n+1)) + echo_i "check that dig tries the next server after a TCP socket network unreachable error ($n)" + ret=0 + dig_with_opts +tcp @192.0.2.128 @10.53.0.3 a.example > dig.out.test$n 2>&1 || ret=1 + test $(grep -F -e "connection refused" -e "timed out" -e "network unreachable" dig.out.test$n | wc -l) -eq 3 || ret=1 + grep -F "status: NOERROR" dig.out.test$n > /dev/null || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) @@ -1023,7 +1041,7 @@ if [ -x "$DIG" ] ; then echo_i "check that dig tries the next server after a UDP socket read error ($n)" ret=0 dig_with_opts @10.53.0.99 @10.53.0.3 a.example > dig.out.test$n 2>&1 || ret=1 - grep "status: NOERROR" dig.out.test$n > /dev/null || ret=1 + grep -F "status: NOERROR" dig.out.test$n > /dev/null || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) @@ -1033,7 +1051,7 @@ if [ -x "$DIG" ] ; then echo "close" | sendcmd 10.53.0.8 ret=0 dig_with_opts +tcp @10.53.0.8 @10.53.0.3 a.example > dig.out.test$n 2>&1 || ret=1 - grep "status: NOERROR" dig.out.test$n > /dev/null || ret=1 + grep -F "status: NOERROR" dig.out.test$n > /dev/null || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) @@ -1048,7 +1066,7 @@ if [ -x "$DIG" ] ; then ret=0 dig_with_opts +tcp @10.53.0.99 @10.53.0.3 a.example > dig.out.test$n 2>&1 || ret=1 test $(grep -F -e "connection refused" -e "timed out" -e "network unreachable" dig.out.test$n | wc -l) -eq 3 || ret=1 - grep "status: NOERROR" dig.out.test$n > /dev/null || ret=1 + grep -F "status: NOERROR" dig.out.test$n > /dev/null || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) @@ -1058,7 +1076,7 @@ if [ -x "$DIG" ] ; then echo "silent" | sendcmd 10.53.0.8 ret=0 dig_with_opts +timeout=1 @10.53.0.8 @10.53.0.3 a.example > dig.out.test$n 2>&1 || ret=1 - grep "status: NOERROR" dig.out.test$n > /dev/null || ret=1 + grep -F "status: NOERROR" dig.out.test$n > /dev/null || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) @@ -1068,7 +1086,7 @@ if [ -x "$DIG" ] ; then echo "silent" | sendcmd 10.53.0.8 ret=0 dig_with_opts +timeout=1 +tcp @10.53.0.8 @10.53.0.3 a.example > dig.out.test$n 2>&1 || ret=1 - grep "status: NOERROR" dig.out.test$n > /dev/null || ret=1 + grep -F "status: NOERROR" dig.out.test$n > /dev/null || ret=1 if [ $ret -ne 0 ]; then echo_i "failed"; fi status=$((status+ret)) From a8dea1d1ad9a04f899417c8b862f5efccadab0ee Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Mon, 20 Jun 2022 12:32:45 +0000 Subject: [PATCH 5/6] Add CHANGES note for [GL #3407] --- CHANGES | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGES b/CHANGES index 1c31fcefd3..c2a85fe12d 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,10 @@ +5930. [bug] Fix DiG query retry and fail-over bug in UDP mode. + Also simplify the overall retry and fail-over logic to + make it behave predictably, and always respect the + documented +retry/+tries count set by a command-line + option (or use the default values of 2 or 3 + respectively). [GL #3407] + 5929. [func] The use of the "max-zone-ttl" option in "zone" and "options" blocks is now deprecated; this should now be configured as part of "dnssec-policy" From 65d9d90b559ec7764049d3f5c9fada0838a51606 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Thu, 23 Jun 2022 16:24:29 +0000 Subject: [PATCH 6/6] Suppress warning/error comments in dig outputs for "rrsetorder" test In the CI dig sometimes produces warning/error comments when communicating with the server, which produces problems when comparing the outputs. Here is an example of a dig output with a warning message which is benign, because dig, after a retry, managed to query the server. ;; communications error to 10.53.0.3#7529: timed out 1.2.3.1 1.2.3.2 1.2.3.3 1.2.3.4 When comparing this to the expected output, which doesn't contain the comment line (starting with double ';'), the outputs don't match. Use grep inverse logic to strip the comments from the dig outputs. --- bin/tests/system/rrsetorder/tests.sh | 66 +++++++++++++++------------- 1 file changed, 35 insertions(+), 31 deletions(-) diff --git a/bin/tests/system/rrsetorder/tests.sh b/bin/tests/system/rrsetorder/tests.sh index 2c2bc10442..6bd0ec02d5 100644 --- a/bin/tests/system/rrsetorder/tests.sh +++ b/bin/tests/system/rrsetorder/tests.sh @@ -14,7 +14,11 @@ . ../conf.sh DIGOPTS="+nosea +nocomm +nocmd +noquest +noadd +noauth +nocomm +nostat +short +nocookie" -DIGCMD="$DIG $DIGOPTS -p ${PORT}" + +dig_cmd() { + # shellcheck disable=SC2086 + "$DIG" $DIGOPTS -p "${PORT}" "$@" | grep -v '^;' +} status=0 @@ -36,7 +40,7 @@ if $test_fixed; then ret=0 for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 do - $DIGCMD @10.53.0.1 fixed.example > dig.out.fixed || ret=1 + dig_cmd @10.53.0.1 fixed.example > dig.out.fixed || ret=1 diff dig.out.fixed dig.out.fixed.good >/dev/null || ret=1 done if [ $ret != 0 ]; then echo_i "failed"; fi @@ -48,7 +52,7 @@ else for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 do j=$((i % 4)) - $DIGCMD @10.53.0.1 fixed.example > dig.out.fixed || ret=1 + dig_cmd @10.53.0.1 fixed.example > dig.out.fixed || ret=1 if [ $i -le 4 ]; then cp dig.out.fixed dig.out.$j else @@ -75,7 +79,7 @@ matches=0 for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 do j=$((i % 4)) - $DIGCMD @10.53.0.1 cyclic.example > dig.out.cyclic || ret=1 + dig_cmd @10.53.0.1 cyclic.example > dig.out.cyclic || ret=1 if [ $i -le 4 ]; then cp dig.out.cyclic dig.out.$j else @@ -101,7 +105,7 @@ matches=0 for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 do j=$((i % 4)) - $DIGCMD @10.53.0.1 cyclic2.example > dig.out.cyclic2 || ret=1 + dig_cmd @10.53.0.1 cyclic2.example > dig.out.cyclic2 || ret=1 if [ $i -le 4 ]; then cp dig.out.cyclic2 dig.out.$j else @@ -125,7 +129,7 @@ do done for i in a b c d e f g h i j k l m n o p q r s t u v w x y z 0 1 2 3 4 5 6 7 9 do - $DIGCMD @10.53.0.1 random.example > dig.out.random || ret=1 + dig_cmd @10.53.0.1 random.example > dig.out.random || ret=1 match=0 for j in $GOOD_RANDOM do @@ -147,13 +151,13 @@ status=$((status + ret)) echo_i "Checking order none (primary)" ret=0 # Fetch the "reference" response and ensure it contains the expected records. -$DIGCMD @10.53.0.1 none.example > dig.out.none || ret=1 +dig_cmd @10.53.0.1 none.example > dig.out.none || ret=1 for i in 1 2 3 4; do grep -F -q 1.2.3.$i dig.out.none || ret=1 done # Ensure 20 further queries result in the same response as the "reference" one. for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20; do - $DIGCMD @10.53.0.1 none.example > dig.out.test$i || ret=1 + dig_cmd @10.53.0.1 none.example > dig.out.test$i || ret=1 diff dig.out.none dig.out.test$i >/dev/null || ret=1 done if [ $ret != 0 ]; then echo_i "failed"; fi @@ -167,7 +171,7 @@ if $test_fixed; then ret=0 for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 do - $DIGCMD @10.53.0.2 fixed.example > dig.out.fixed || ret=1 + dig_cmd @10.53.0.2 fixed.example > dig.out.fixed || ret=1 diff dig.out.fixed dig.out.fixed.good || ret=1 done if [ $ret != 0 ]; then echo_i "failed"; fi @@ -183,7 +187,7 @@ matches=0 for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 do j=$((i % 4)) - $DIGCMD @10.53.0.2 cyclic.example > dig.out.cyclic || ret=1 + dig_cmd @10.53.0.2 cyclic.example > dig.out.cyclic || ret=1 if [ $i -le 4 ]; then cp dig.out.cyclic dig.out.$j else @@ -209,7 +213,7 @@ matches=0 for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 do j=$((i % 4)) - $DIGCMD @10.53.0.2 cyclic2.example > dig.out.cyclic2 || ret=1 + dig_cmd @10.53.0.2 cyclic2.example > dig.out.cyclic2 || ret=1 if [ $i -le 4 ]; then cp dig.out.cyclic2 dig.out.$j else @@ -234,7 +238,7 @@ do done for i in a b c d e f g h i j k l m n o p q r s t u v w x y z 0 1 2 3 4 5 6 7 9 do - $DIGCMD @10.53.0.2 random.example > dig.out.random || ret=1 + dig_cmd @10.53.0.2 random.example > dig.out.random || ret=1 match=0 for j in $GOOD_RANDOM do @@ -256,13 +260,13 @@ status=$((status + ret)) echo_i "Checking order none (secondary)" ret=0 # Fetch the "reference" response and ensure it contains the expected records. -$DIGCMD @10.53.0.2 none.example > dig.out.none || ret=1 +dig_cmd @10.53.0.2 none.example > dig.out.none || ret=1 for i in 1 2 3 4; do grep -F -q 1.2.3.$i dig.out.none || ret=1 done # Ensure 20 further queries result in the same response as the "reference" one. for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20; do - $DIGCMD @10.53.0.2 none.example > dig.out.test$i || ret=1 + dig_cmd @10.53.0.2 none.example > dig.out.test$i || ret=1 diff dig.out.none dig.out.test$i >/dev/null || ret=1 done if [ $ret != 0 ]; then echo_i "failed"; fi @@ -292,7 +296,7 @@ if $test_fixed; then ret=0 for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 do - $DIGCMD @10.53.0.2 fixed.example > dig.out.fixed || ret=1 + dig_cmd @10.53.0.2 fixed.example > dig.out.fixed || ret=1 diff dig.out.fixed dig.out.fixed.good || ret=1 done if [ $ret != 0 ]; then echo_i "failed"; fi @@ -308,7 +312,7 @@ matches=0 for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 do j=$((i % 4)) - $DIGCMD @10.53.0.2 cyclic.example > dig.out.cyclic || ret=1 + dig_cmd @10.53.0.2 cyclic.example > dig.out.cyclic || ret=1 if [ $i -le 4 ]; then cp dig.out.cyclic dig.out.$j else @@ -334,7 +338,7 @@ matches=0 for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 do j=$((i % 4)) - $DIGCMD @10.53.0.2 cyclic2.example > dig.out.cyclic2 || ret=1 + dig_cmd @10.53.0.2 cyclic2.example > dig.out.cyclic2 || ret=1 if [ $i -le 4 ]; then cp dig.out.cyclic2 dig.out.$j else @@ -359,7 +363,7 @@ do done for i in a b c d e f g h i j k l m n o p q r s t u v w x y z 0 1 2 3 4 5 6 7 9 do - $DIGCMD @10.53.0.2 random.example > dig.out.random || ret=1 + dig_cmd @10.53.0.2 random.example > dig.out.random || ret=1 match=0 for j in $GOOD_RANDOM do @@ -381,13 +385,13 @@ status=$((status + ret)) echo_i "Checking order none (secondary loaded from disk)" ret=0 # Fetch the "reference" response and ensure it contains the expected records. -$DIGCMD @10.53.0.2 none.example > dig.out.none || ret=1 +dig_cmd @10.53.0.2 none.example > dig.out.none || ret=1 for i in 1 2 3 4; do grep -F -q 1.2.3.$i dig.out.none || ret=1 done # Ensure 20 further queries result in the same response as the "reference" one. for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20; do - $DIGCMD @10.53.0.2 none.example > dig.out.test$i || ret=1 + dig_cmd @10.53.0.2 none.example > dig.out.test$i || ret=1 diff dig.out.none dig.out.test$i >/dev/null || ret=1 done if [ $ret != 0 ]; then echo_i "failed"; fi @@ -401,7 +405,7 @@ if $test_fixed; then ret=0 for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 do - $DIGCMD @10.53.0.3 fixed.example > dig.out.fixed || ret=1 + dig_cmd @10.53.0.3 fixed.example > dig.out.fixed || ret=1 diff dig.out.fixed dig.out.fixed.good || ret=1 done if [ $ret != 0 ]; then echo_i "failed"; fi @@ -414,12 +418,12 @@ fi echo_i "Checking order cyclic (cache + additional)" ret=0 # prime acache -$DIGCMD @10.53.0.3 cyclic.example > dig.out.cyclic || ret=1 +dig_cmd @10.53.0.3 cyclic.example > dig.out.cyclic || ret=1 matches=0 for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 do j=$((i % 4)) - $DIGCMD @10.53.0.3 cyclic.example > dig.out.cyclic || ret=1 + dig_cmd @10.53.0.3 cyclic.example > dig.out.cyclic || ret=1 if [ $i -le 4 ]; then cp dig.out.cyclic dig.out.$j else @@ -442,12 +446,12 @@ status=$((status + ret)) echo_i "Checking order cyclic (cache)" ret=0 # prime acache -$DIGCMD @10.53.0.3 cyclic2.example > dig.out.cyclic2 || ret=1 +dig_cmd @10.53.0.3 cyclic2.example > dig.out.cyclic2 || ret=1 matches=0 for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 do j=$((i % 4)) - $DIGCMD @10.53.0.3 cyclic2.example > dig.out.cyclic2 || ret=1 + dig_cmd @10.53.0.3 cyclic2.example > dig.out.cyclic2 || ret=1 if [ $i -le 4 ]; then cp dig.out.cyclic2 dig.out.$j else @@ -472,7 +476,7 @@ do done for i in a b c d e f g h i j k l m n o p q r s t u v w x y z 0 1 2 3 4 5 6 7 9 do - $DIGCMD @10.53.0.3 random.example > dig.out.random || ret=1 + dig_cmd @10.53.0.3 random.example > dig.out.random || ret=1 match=0 for j in $GOOD_RANDOM do @@ -494,13 +498,13 @@ status=$((status + ret)) echo_i "Checking order none (cache)" ret=0 # Fetch the "reference" response and ensure it contains the expected records. -$DIGCMD @10.53.0.3 none.example > dig.out.none || ret=1 +dig_cmd @10.53.0.3 none.example > dig.out.none || ret=1 for i in 1 2 3 4; do grep -F -q 1.2.3.$i dig.out.none || ret=1 done # Ensure 20 further queries result in the same response as the "reference" one. for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20; do - $DIGCMD @10.53.0.3 none.example > dig.out.test$i || ret=1 + dig_cmd @10.53.0.3 none.example > dig.out.test$i || ret=1 diff dig.out.none dig.out.test$i >/dev/null || ret=1 done if [ $ret != 0 ]; then echo_i "failed"; fi @@ -514,7 +518,7 @@ do done for i in a b c d e f g h i j k l m n o p q r s t u v w x y z 0 1 2 3 4 5 6 7 9 do - $DIGCMD @10.53.0.5 random.example > dig.out.random || ret=1 + dig_cmd @10.53.0.5 random.example > dig.out.random || ret=1 match=0 for j in $GOOD_RANDOM do @@ -536,13 +540,13 @@ status=$((status + ret)) echo_i "Checking default order no match in rrset-order (cache)" ret=0 # Fetch the "reference" response and ensure it contains the expected records. -$DIGCMD @10.53.0.4 nomatch.example > dig.out.nomatch || ret=1 +dig_cmd @10.53.0.4 nomatch.example > dig.out.nomatch || ret=1 for i in 1 2 3 4; do grep -F -q 1.2.3.$i dig.out.nomatch || ret=1 done # Ensure 20 further queries result in the same response as the "reference" one. for i in 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20; do - $DIGCMD @10.53.0.4 nomatch.example > dig.out.test$i || ret=1 + dig_cmd @10.53.0.4 nomatch.example > dig.out.test$i || ret=1 diff dig.out.nomatch dig.out.test$i >/dev/null || ret=1 done if [ $ret != 0 ]; then echo_i "failed"; fi