[Core] Bind gRPC servers to the node address family - #65867
Conversation
Signed-off-by: 400Ping <jiekaichang@apache.org>
Signed-off-by: 400Ping <jiekaichang@apache.org>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates Ray's gRPC server binding logic to accept a node_ip_address string instead of a boolean flag, allowing servers to dynamically bind to the correct wildcard address (0.0.0.0 or ::) matching the node's address family. This change is integrated across both C++ and Python/Cython layers. Feedback on these changes includes simplifying the IsIpv6LoopbackAvailable test helper to avoid a full TCP handshake, passing node_ip_address by const std::string & in the GrpcServer constructor to prevent unnecessary string copies, and removing the Optional[str] type hint in Cython to avoid a potential runtime NameError.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
Reviewed by Cursor Bugbot for commit ac759e2. Configure here.
| if is_localhost(node_ip_address) | ||
| else get_all_interfaces_ip(node_ip_address) | ||
| ) | ||
| return build_address(grpc_ip, grpc_port) |
There was a problem hiding this comment.
Dashboard agent ignores loopback family
Medium Severity
_build_grpc_address maps every localhost value through get_localhost_ip(), so 127.0.0.1 and ::1 follow the system's localhost family instead of the advertised address. Core gRPC servers keep the literal loopback address, so an IPv6-localhost node can leave the dashboard agent listening on IPv4 loopback while clients connect to ::1.
Reviewed by Cursor Bugbot for commit ac759e2. Configure here.
|
|
||
| /// Internal helper that returns the address on which a gRPC server should listen for a | ||
| /// Ray node address. This is not a public networking API. | ||
| std::string GetGrpcServerBindAddress(const std::string &node_ip_address); |
There was a problem hiding this comment.
Missing Doxygen on bind helper
Low Severity
/** ... */ block comments using @ tags (@brief, and @param/@return where applicable). Newly added GetGrpcServerBindAddress has only a brief /// comment and omits @brief, @param, and @return.
Triggered by project rule: Bugbot Rules
Reviewed by Cursor Bugbot for commit ac759e2. Configure here.
2 participants
Description
This PR allows Ray's core gRPC servers to bind using the address family of the
node's advertised IP address.
Previously,
ray start --node-ip-address=<IPv6 address>advertised an IPv6node address, but
GrpcServerselected its wildcard bind address from thesystem's localhost resolution. On IPv4-first systems, this caused the server
to listen on
0.0.0.0even when the Ray node advertised an IPv6 address.As a result, an IPv6-only worker could attempt to connect to the GCS through
IPv6 while the GCS was only listening on an IPv4 socket.
The new bind behavior is:
localhost127.0.0.1127.0.0.1::1::10.0.0.0::Changes
0.0.0.0::CoreWorker, and the dashboard agent.
IPv6.
port.
Related issues
Related to
#54660
ray-project/kuberay#5215
Additional information