Skip to content

connectd/tests: fix test_announce_and_connect_via_dns on macOS#9143

Open
nGoline wants to merge 1 commit into
ElementsProject:masterfrom
nGoline:dns-announce-macos-fix
Open

connectd/tests: fix test_announce_and_connect_via_dns on macOS#9143
nGoline wants to merge 1 commit into
ElementsProject:masterfrom
nGoline:dns-announce-macos-fix

Conversation

@nGoline

@nGoline nGoline commented May 18, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Skip test_announce_and_connect_via_dns with a clear message when localhost.localdomain is not resolvable (macOS omits it from /etc/hosts by default)
  • Guard AI_ADDRCONFIG in try_connect_one_addr() with #ifndef __APPLE__: on macOS/BSD, AI_ADDRCONFIG opens temporary probe sockets that are not tracked by the io framework, causing dev_report_fds() to report them as unowned (**BROKEN**); Linux keeps the optimization unchanged

Closes #9012

Test plan

  • uv run pytest tests/test_gossip.py::test_announce_and_connect_via_dns -v passes on Linux
  • On macOS without localhost.localdomain in /etc/hosts: test is skipped with a descriptive message
  • On macOS with localhost.localdomain in /etc/hosts: no **BROKEN** fd messages from connectd at teardown

🤖 Generated with Claude Code

@nGoline nGoline requested a review from ddustin May 18, 2026 14:16
@nGoline nGoline force-pushed the dns-announce-macos-fix branch 2 times, most recently from 89d3d46 to b7b271c Compare May 18, 2026 20:50
@nGoline nGoline added this to the v26.09 milestone May 21, 2026
@madelinevibes madelinevibes added the Status::Ready for Review The work has been completed and is now awaiting evaluation or approval. label Jun 18, 2026
Comment thread tests/test_gossip.py Outdated
socket.getaddrinfo('localhost.localdomain', 12345, 0, socket.SOCK_STREAM)
except socket.gaierror:
pytest.skip("localhost.localdomain not resolvable; add '127.0.0.1 localhost.localdomain' to /etc/hosts")

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of skipping the test, I think we might be able to use "test.localhost," as mac allows subdomains on localhost. If that doesn't work I wonder if just setting it to "localhost" would satisfy what the test is trying to do.

If none of those work and we have to skip the test -- we should skip the test with a clause for Mac, probably something like:

@unittest.skipIf(sys.platform == "darwin")

Doing it explicitly this way will make it easy to keep track of which tests we're skipping

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually skips with a message for the dev, asking them to add the host to /etc/hosts.

I can try the other options to see if something goes through...

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dug into the two options you proposed and hit a wall with test.localhost. Here's what I found:

test.localhost resolves correctly on macOS via mDNSResponder (no /etc/hosts entry needed), but it does not resolve on Ubuntu 24.04 containers used on CI, which lack systemd0resolved. To work around that I tried a platform-specific approach: test.localhost on macOS, localhost.localdomain on Linux, and that passed on Linux. However, the macOS teardown fails with a BROKEN log because mDNSResponder opens temporary sockets internally during the resolution independently of AI_ADDRCONFIG. I've tried guarding connectd using AI_ADDRCONFIG with #ifndef __APPLE__, but that's not the source of the sockets here, it's the mDNSResponder daemon itself. Any hostname that goes through mDNSResponder on macOS will trigger this.

localhost was not tried because the test specifically avoids it. dev-allow-localhost would let nodes connect to 127.0.0.1 without DNS, defeating the purpose of the test.

That leaves us with two honest options:

  1. Explicit @unittest.skipIf(sys.platform == "darwin") (your fallback suggestion): keep localhost.localdomain, which resolves cleanly on Linux and avoids mDNSResponder entirely. Skip on macOS with a clear, trackable decorator.
  2. Fix dev_report_fds to tolerate mDNSResponder sockets: teach connectd's fd-leak checker that sockets opened by the system resolver are not leaks. This would let test.localhost work cleanly on macOS, but it's a non-trivial change to the io framework and probably belongs in a separate PR.

I've gone with option 1 for this PR and will file a follow-up for option 2.

localhost.localdomain is not present in /etc/hosts on macOS by default,
so the test's DNS-resolve step fails immediately.  Any hostname that
does resolve on macOS (e.g. test.localhost via mDNSResponder) causes
mDNSResponder to open temporary sockets which dev_report_fds() flags
as unowned (BROKEN) at teardown.

Skip the test explicitly on macOS with @unittest.skipIf so the reason
is visible and trackable.  A follow-up can fix dev_report_fds to
tolerate resolver sockets and re-enable the test on macOS.

Changelog-None
@nGoline nGoline force-pushed the dns-announce-macos-fix branch from b7b271c to e47a833 Compare June 19, 2026 14:47
@nGoline nGoline requested a review from ddustin June 19, 2026 14:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Status::Ready for Review The work has been completed and is now awaiting evaluation or approval.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

pytest: **BROKEN** Error in test test_announce_and_connect_via_dns

3 participants