Ray IPv6 support

[samrocketman]

Why are these changes needed?

Provides support for IPv6 only and dual-stack IPv4/IPv6.

Inspired by this discussion https://discuss.ray.io/t/lack-of-ipv6-support/2323

IPv6 is much more common than it used to be; in some cases the only available stack is IPv6. Commonly machines come with dual-stack connectivity enabled (IPv4 and IPv6 available at the same time).

I've already reviewed the source code of kuberay. I've determined due to its use of native Go net methods kuberay already supports IPv6.

Design

• See python/ray/_private/net.py. Centralize networking code to make it easier to support IPv6 alongside IPv4. This is simpler and clarifies where in your network code you need to support this when using low level python sockets.
• Favor IPv4 since that is what this project is typically used to; code will fall back to IPv6 if IPv4 is not available.
• It uses dual-stack IPv4 and IPv6 when it is available; for example when listening on a port across all interfaces or determining free ports.
• All places I could find for string parsing IP or host and port have been substituted. With a central method, updates can be made from which all dependent areas will benefit.

Shell code

I notice the following scripts may or may not work with IPv6. They should probably updated using bash parameter expansion.

• doc/source/cluster/doc_code/slurm-basic.sh
• doc/source/cluster/doc_code/slurm-template.sh

If I update them; it will be in a separate PR.

For example, the following bash code properly parses both IPv4 and IPv6 ip:port into two parts.

# IPv6 loopback port 8080
ip_and_port=::1:8080
ip="${ip_and_port%:*}"
port="${ip_and_port##*:}"

# IPv4 loopback port 8080
ip_and_port=127.0.0.1:8080
ip="${ip_and_port%:*}"
port="${ip_and_port##*:}"

Related issue number

I don't know if it will close all of these issues but I did an issues search for ipv6.

• closes [core] ipv6 support #6967
• [cluster] bringing up cluster fails: socket.gethostbyname("localhost") - socket.gaierror: [Errno -2] Name or service not known #43416
• Java : Fail to parse IPv6 address and port from bootstrap address in GcsClientOptions #41966
• Cloud Provider Requirements Documentation  #43910

Duplicated by

• [WIP][Feature commit] Initial commit for supporting IPv6 stack in Ray Clus… #40332

Checks

Initially, this pull request is in-draft. This is to enable reviewing sooner and manual testing to occur. Automated tests to follow.

• I've signed off every commit(by using the -s flag, i.e., git commit -s) in this PR.
• I've run scripts/format.sh to lint the changes in this PR.
• I've included any doc changes needed for https://docs.ray.io/en/master/.
• I've added any new APIs to the API Reference. For example, if I added a method in Tune, I've added it in doc/source/tune/api/ under the corresponding .rst file.
• I've made sure the tests are passing. Note that there might be a few flaky tests, see the recent failures at https://flakey-tests.ray.io/
• Testing Strategy
  • Unit tests
  • Release tests
  • This PR is not tested :(

[samrocketman]

Another review pass fixing issues I learned studying how Python sockets support IPv6.

I thought dualstack works when binding with a host but it does not (e.g. localhost dualstack only binds to IPv6 ::1 but not 127.0.0.1).

Binding dualstack only works when binding a port to all interfaces i.e. sock.bind(('', port)).

[samrocketman]

I appear to be misusing @staticmethod.

[samrocketman]

I guess my issues with staticmethod are best described here https://stackoverflow.com/questions/1859959/static-methods-how-to-call-a-method-from-another-method

[samrocketman]

After messing around with @staticmethod across different versions of python; the way I used it (on a file of functions without a class definition) worked in Python 3.10 but not Python 3.9. Rather than trying to declare the methods static which would work across multiple versions of python I decided to remove the symbol from the private net module.

[samrocketman]

We took a closer look. Unfortunately, it seems to go much deeper than a localized fix. While your change correctly parses the address as IPv6, it gets rejected shortly after. It turns out the broader Ray codebase assumes IPv4 in many places, and parsing is only the beginning. Changes are probably needed in most services, as well as supporting code (SSL...)

Thanks for taking a look. I will likely continue chasing all of the broader changes in this pull request and continue to iterate until I succeed with an IPv6 supported build.

So, from what I've seen, supporting IPv6 across Ray would require a more comprehensive initiative, which is quite a bit more effort than we can realistically take on at this point.

That's okay I appreciate your review.

It might make sense to raise this with the Ray maintainers, see if IPv6 support could be prioritized or scoped out officially.

This PR is raised. I think, in general, maintainer support is a hard requirement. Since I initially opened this pull request there have been a few merged PRs which added socket code which would have broken IPv6 if it was hypothetically available.

Because of this, there needs to be supporting tests and an knowlege-sharing initiative for maintainers to actively preserve IPv6 support with new contributions. I am happy to produce some sort of written guide or whatever written form makes sense for maintainers. I mentioned this in an earlier comment but I am reiterating it here. I need maintainers to advise on how they want this done.

At some point, I'll come back to this and dig in again (I'm away on vacation at the moment and this change start to finish has been spare time with exception for updating the PR once).

[samrocketman]

@jjyao can you clarify what adding that label means?

[github-actions]

This pull request has been automatically marked as stale because it has not had
any activity for 14 days. It will be closed in another 14 days if no further activity occurs.
Thank you for your contributions.

You can always ask for help on our discussion forum or Ray's public slack channel.

If you'd like to keep this open, just leave any comment, and the stale label will be removed.

[samrocketman]

Not stale; maintainers please label so stale bot does not mark as stale.

See also my previous questions

[github-actions]

This pull request has been automatically marked as stale because it has not had
any activity for 14 days. It will be closed in another 14 days if no further activity occurs.
Thank you for your contributions.

You can always ask for help on our discussion forum or Ray's public slack channel.

If you'd like to keep this open, just leave any comment, and the stale label will be removed.

[samrocketman]

Not stale; maintainers please label so stale bot does not mark as stale.

See also my previous questions

[github-actions]

This pull request has been automatically marked as stale because it has not had
any activity for 14 days. It will be closed in another 14 days if no further activity occurs.
Thank you for your contributions.

You can always ask for help on our discussion forum or Ray's public slack channel.

If you'd like to keep this open, just leave any comment, and the stale label will be removed.

[samrocketman]

Not stale; maintainers please label so stale bot does not mark as stale.

See also my previous questions

[samrocketman]

To be clear: I do not plan to resolve conflicts or work on this further until some maintainers come out of the wood work to talk with me about these changes. I've requested multiple times in this pull request for a maintainer to chime in and there appears to be silence.

Because of this, I do not find it is worth my time to keep resolving conflicts with no maintainer conversation; my time is not free.

You're welcome to reach out to me in this pull request or you may advise another method that's preferred.

[robertnishihara]

@samrocketman thanks a ton for starting this work, and sorry about neglecting this!

I think we should do a quick call (or email / Slack) discussion to figure out the path forward. Some folks at ByteDance recently mentioned they have an implementation of this feature as well and are interested in contributing it.

Could you email me at robertnishihara [at] gmail.com or message me on the Ray Slack to have a higher bandwidth conversation? FYI @richardliaw @jjyao

[samrocketman]

@robertnishihara emailed

[samrocketman]

Update: today at 2pm EDT (18:00 UTC-0) there was an in-person meeting between maintainers and I where we went over the current pull request and general information on how to support IPv6.

Much of what I've already shared in this pull request was shared within the meeting.

[psaab]

You need this diff as well or something similar

diff --git a/ray/python/ray/dashboard/modules/dashboard_sdk.py b/ray/python/ray/dashboard/modules/dashboard_sdk.py
--- a/ray/python/ray/dashboard/modules/dashboard_sdk.py
+++ b/ray/python/ray/dashboard/modules/dashboard_sdk.py
@@ -298,7 +298,13 @@
         Keyword arguments other than "cookies", "headers" are forwarded to the
         `requests.request()`.
         """
-        url = self._address + endpoint
+        url = f"{self._address}{endpoint}"
+        try:
+            if ipaddress.ip_address(self._address).version == 6:
+                url = f"[{self._address}]{endpoint}"
+        except ValueError:
+            pass
+

[samrocketman]

@psaab mind opening a PR to my fork so that you get attributed for that diff?

[github-actions]

This pull request has been automatically marked as stale because it has not had
any activity for 14 days. It will be closed in another 14 days if no further activity occurs.
Thank you for your contributions.

You can always ask for help on our discussion forum or Ray's public slack channel.

If you'd like to keep this open, just leave any comment, and the stale label will be removed.

[robertnishihara]

@samrocketman FYI @Yicheng-Lu-llll has been doing some work on this in #55129, #55281, #55282.

[richardliaw]

Closing this in lieu of the other PRs from @Yicheng-Lu-llll