tests: Fixes DNS tests false positives#70242
Conversation
If dig fails to reach the DNS, it will output errors to stdout, which can cause the test -n to falsely pass: ubuntu@ubuntu:~$ dig +notcp +noall +answer +search kubernetes.default A ;; connection timed out; no servers could be reached command terminated with exit code 9 ubuntu@ubuntu:~$ test -n "$(dig +notcp +noall +answer +search kubernetes.default A)" && echo OK OK This patch solves this issue by making sure the dig command actually succeeds before checking its output.
|
/assign @dnardo |
test/e2e/network/dns_common.go
Outdated
There was a problem hiding this comment.
I haven't investigated why the $s are doubled elsewhere, but do you need to double them in "$check" as well?
(Also, an alternative approach would be to just append 2>/dev/null to the dig command, to discard stderr so you're only checking stdout.)
There was a problem hiding this comment.
The explanation for $$ is just a few lines above this change. Basically, it says that
But for the 2> /dev/null thing, it won't work, because dig explicitly outputs the error to stdout instead of stderr. Otherwise, this shouldn't have been an issue in the 1st place.
There was a problem hiding this comment.
For example:
ubuntu@ubuntu:~$ dig +notcp +noall +answer +search kubernetes.default.svc.cluster.local A
;; connection timed out; no servers could be reached
ubuntu@ubuntu:~$ dig +notcp +noall +answer +search kubernetes.default.svc.cluster.local A 2> /dev/null
;; connection timed out; no servers could be reached
There was a problem hiding this comment.
And as far as
15a8cdc to
fb8d2f7
Compare
|
/lgtm |
|
/test pull-kubernetes-integration |
|
/area conformance |
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: bclau, spiffxp The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
What type of PR is this?
/kind bug
/sig testing
What this PR does / why we need it:
If dig fails to reach the DNS, it will output errors to stdout, which
can cause the test -n to falsely pass:
This patch solves this issue by making sure the dig command actually succeeds
before checking its output.
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #63366
Special notes for your reviewer:
Does this PR introduce a user-facing change?: