pasta: Re-enable "Local forwarder, IPv4" test now that packages in CI images are fixed #17713
Conversation
They include Fedora package passt-0^20230227.gc538ee8-1.fc37 and passt-0.0~git20230227.c538ee8-1 for Debian, so that we can re-enable the "Local Forwarder, IPv4" test for pasta(1). Signed-off-by: Stefano Brivio <[email protected]>
sbrivio-rh
changed the title
openshift-ci
bot
added
the
do-not-merge/work-in-progress
| skip_if_no_ipv4 "IPv4 not routable on the host" | ||
|
|
||
| run_podman run --dns 198.51.100.1 \ | ||
| --net=pasta:--dns-forward,198.51.100.1 $IMAGE nslookup 127.0.0.1 | ||
| --net=pasta:--dns-forward,198.51.100.1 $IMAGE nslookup 127.0.0.1 || : |
There was a problem hiding this comment.
I see your commit message but why try to handle different behaviour? Wouldn't it be better to just nslookup google.com? We already do this in other tests so we know it works in all environments where the tests are run at the moment.
Also if a test accepts a unknown exit code write it as run_podman ? arguments...
There was a problem hiding this comment.
I see your commit message but why try to handle different behaviour? Wouldn't it be better to just nslookup google.com? We already do this in other tests so we know it works in all environments where the tests are run at the moment.
Two reasons, but not necessarily very practical:
- I'm not really fond of hardcoding in tests of publicly available software the domain name of a given company. I'm also not sure that google.com can be universally resolved. I kind of expect DNS censorship to be implemented with DNS poisoning rather than just dropping queries, but I wouldn't be so sure
- the host resolver might be temporarily unable to contact an upstream DNS, but this shouldn't affect this test in particular.
If you have a strong preference toward that, I'll change it, it's probably theoretical problems I'm posing.
Also if a test accepts a unknown exit code write it as run_podman ? arguments...
Oh, I didn't know that, I'll change it.
There was a problem hiding this comment.
You can keep your version, the test does its job. Not depending an a particular name is properly better but as said above we already use it so one more wouldn't make a difference.
There was a problem hiding this comment.
Also if a test accepts a unknown exit code write it as run_podman ? arguments...
Oh, I didn't know that, I'll change it.
Done.
…ponse
This case is fixed by passt commit bad252687271 ("conf, udp: Allow
any loopback address to be used as resolver") and the fix is now
available in packages included by the CI images.
Note that, depending on the resolver on the host, we might get
1.0.0.127.in-addr.arpa resolved to localhost, or simply NXDOMAIN for
it: accept a failure on the nslookup command, as long as we have a
response for 1.0.0.127.in-addr.arpa in the output. If we have any
response, that means we could talk to the resolver.
Signed-off-by: Stefano Brivio <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99, sbrivio-rh The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
sbrivio-rh
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
/lgtm Restarted two Cirrus infra flakes |
openshift-ci
bot
added
do-not-merge/hold
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
github-actions
bot
added
the
locked - please file new issue/PR
This fixes #17074.
Does this PR introduce a user-facing change?