Skip to content

Conversation

@jackkleeman
Copy link
Contributor

This change supports headless initial addresses for the metadata client. Previously if the dns name resolves to 5 ips, we would use whatever getaddrinfo returns first (typically this is either your own ip or another ip in the same zone), and we would also divide our conn timeout by 5. Which this change, we use our full conn timeout on a single randomly chosen ip of the 5. We only use this new code path in the initial addresses flow - the rest of the time, a tonic channel is intended for a single node and we have no reason to need to ensure we are using all the returned ips.

@jackkleeman jackkleeman marked this pull request as ready for review November 18, 2025 11:07
@github-actions
Copy link

github-actions bot commented Nov 18, 2025

Test Results

  7 files  ±0    7 suites  ±0   3m 27s ⏱️ +17s
 47 tests ±0   47 ✅ ±0  0 💤 ±0  0 ❌ ±0 
200 runs  ±0  200 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 4ca0492. ± Comparison against base commit e39b2d2.

♻️ This comment has been updated with latest results.

@AhmedSoliman
Copy link
Contributor

P.S. I didn't read the PR, but could a trick similar to

fn guess_my_routable_ip() -> &'static str {
work to get the remote address?

@jackkleeman
Copy link
Contributor Author

@AhmedSoliman this pr is about resolving the initial addresses over dns, with the goal of reaching other nodes to join the metadata cluster - its not related to discovering your own ip. In k8s environments the initial address is typically a single address that dns resolves to the ips of every node. The problem is that we tend to use the first ip, and the first ip is not random, its usually either yourself or a zone local ip, which isnt conducive to joining a cluster

Copy link
Contributor

@tillrohrmann tillrohrmann left a comment

Choose a reason for hiding this comment

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

Thanks for adding support for handling headless initial addresses when connecting to the metadata server @jackkleeman. The changes look good to me. I left a question about the need of maintaining the ipv4/ipv6 order which is not clear to me.

Did you test your changes with a headless service where there are multiple endpoints? If yes, and things were working, then +1 for merging.

@jackkleeman jackkleeman force-pushed the jk/znsrqzuxznvm branch 2 times, most recently from 2bcd5f3 to 10c532c Compare December 1, 2025 10:30
@jackkleeman
Copy link
Contributor Author

i have tested the change and it works well

@jackkleeman jackkleeman merged commit 043fde1 into main Dec 1, 2025
27 checks passed
@jackkleeman jackkleeman deleted the jk/znsrqzuxznvm branch December 1, 2025 13:17
@github-actions github-actions bot locked and limited conversation to collaborators Dec 1, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants