Skip to content

Worker: Bind to the first non-link-local IPv4 address #137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Jun 16, 2025
Merged

Worker: Bind to the first non-link-local IPv4 address #137

merged 5 commits into from
Jun 16, 2025

Conversation

DilumAluthge
Copy link
Member

No description provided.

@DilumAluthge DilumAluthge mentioned this pull request Jun 10, 2025
Closed
1 task
@DilumAluthge
Copy link
Member Author

@tanmaykm Can you review this?

@codecov Codecov
Copy link

codecov bot commented Jun 10, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.38%. Comparing base (ff8689a) to head (875cd5a).
Report is 6 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #137      +/-   ##
==========================================
- Coverage   79.42%   79.38%   -0.04%     
==========================================
  Files          10       10              
  Lines        1949     1955       +6     
==========================================
+ Hits         1548     1552       +4     
- Misses        401      403       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@JamesWrigley
Copy link
Member

I have no idea if someone is depending on this functionality, but just to be safe could we prioritize non-link-local addresses instead of completely ignoring link-local addresses?

@DilumAluthge
Copy link
Member Author

Seems reasonable to me. So how about this logic:

  1. Get all IPv4 addresses.
  2. If there is a non-link-local IPv4 address, take the first non-link-local IPv4 address.
  3. Otherwise, take the first link-local IPv4 address.

Do we need to support IPv6? Or does Distributed only support IPv4?

@JamesWrigley
Copy link
Member

Yep that sounds good to me 👍 Distributed should be able to support IPv6 too, but for the sake of matching the old behaviour and making the PR safe to backport I'd stick with IPv4 when choosing an interface automatically.

vchuravy
vchuravy reviewed Jun 10, 2025
View reviewed changes
@DilumAluthge DilumAluthge force-pushed the dpa/dont-use-link-local branch from d9805b3 to 2a6ee53 Compare June 10, 2025 22:19
JamesWrigley
JamesWrigley approved these changes Jun 11, 2025
View reviewed changes
Copy link
Member

@JamesWrigley JamesWrigley left a comment

Choose a reason for hiding this comment

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

LGTM!

vchuravy
vchuravy reviewed Jun 11, 2025
View reviewed changes
@DilumAluthge DilumAluthge merged commit 3679026 into master Jun 16, 2025
7 of 8 checks passed
@DilumAluthge DilumAluthge deleted the dpa/dont-use-link-local branch June 16, 2025 18:08
@DilumAluthge
Copy link
Member Author

😭 I should have squash-merged this. Too late now.

@JamesWrigley
Copy link
Member

Does this need to be backported to the release-1.10 branch as well?

@DilumAluthge
Copy link
Member Author

Yeah.

@DilumAluthge
Copy link
Member Author

And ideally also 1.11, since Julia 1.12 isn't out yet.

DilumAluthge added a commit to JuliaLang/julia that referenced this pull request Jul 3, 2025
@DilumAluthge
Distributed: Worker: Bind to the first non-link-local IPv4 address
bca937b 
This is a manual backport of JuliaLang/Distributed.jl#137 to Julia 1.10.x
@DilumAluthge DilumAluthge mentioned this pull request Jul 3, 2025
Merged
@DilumAluthge
Copy link
Member Author

Here's the backport to Julia 1.10:

@DilumAluthge DilumAluthge mentioned this pull request Jul 5, 2025
Merged
@DilumAluthge
Copy link
Member Author

The backport to Julia 1.12 is included in this:

DilumAluthge added a commit to JuliaLang/julia that referenced this pull request Jul 8, 2025
@DilumAluthge
[backports-release-1.10]: Distributed: Worker: Bind to the first no…
c96d507 
…n-link-local IPv4 address (#58895)

This is a manual backport of
JuliaLang/Distributed.jl#137 to Julia 1.10.x.

Targets `backports-release-1.10` (#58889).

This is a bugfix, and thus it is eligible to be backported.
DilumAluthge added a commit that referenced this pull request Aug 25, 2025
@DilumAluthge
Merge pull request #137 from JuliaLang/dpa/dont-use-link-local
f6f0f02 
Worker: Bind to the first non-link-local IPv4 address
(cherry picked from commit 3679026)
@DilumAluthge DilumAluthge mentioned this pull request Aug 25, 2025
Merged
3 tasks
DilumAluthge added a commit that referenced this pull request Aug 25, 2025
@DilumAluthge @dependabot
Backports for Julia 1.11.x (#141)
e9b9023 
* Merge pull request #137 from JuliaLang/dpa/dont-use-link-local

Worker: Bind to the first non-link-local IPv4 address
(cherry picked from commit 3679026)

* Bump actions/cache from 1 to 4 (#98)

Bumps [actions/cache](https://github.com/actions/cache) from 1 to 4.
- [Release notes](https://github.com/actions/cache/releases)
- [Changelog](https://github.com/actions/cache/blob/main/RELEASES.md)
- [Commits](actions/cache@v1...v4)

---
updated-dependencies:
- dependency-name: actions/cache
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
(cherry picked from commit cde4078)

* `release-julia-1.11`: Run CI on Julia 1.11.x (#142)

---------

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@DilumAluthge
Copy link
Member Author

The backport to 1.11 is included in this:

Which has been merged into this:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vchuravy vchuravy vchuravy approved these changes

@JamesWrigley JamesWrigley JamesWrigley approved these changes

Assignees

No one assigned

Labels

bug fix

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@DilumAluthge @JamesWrigley @vchuravy