Add fqdn - #5108
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hi @AlterHoodie, I think it would be better to add a separate feature flag/field for worker FQDNs, since users may not use the RayMultiHostIndexing feature flag.
There is also a similar PR to support mTLS with stable, resolvable FQDNs.
#5191
|
Hey @win5923, thanks for the pointer to #5191, went through it to understand the overlap. Two questions on scope before I proceed:
|
Yes, that's a separate use case. Users may only need stable FQDNs for their Ray Pods as stable identities for TLS certificates, but Ray itself would also need to register and advertise those FQDNs instead of Pod IPs. Another use case is Istio. Today, KubeRay workers are primarily addressable by their Pod IPs, and KubeRay does not create a stable per-pod DNS identity for worker Pods by default. This is a mismatch with how Istio's service discovery and identity model works: Istio relies on Kubernetes Services and their endpoints to discover workloads and associate network traffic with stable workload identities. But this is not related to Ray itslef. In #5191, users can choose whether their RayCluster should register Ray nodes using the Pod FQDN instead of the Pod IP. This allows the FQDN to be used not only as a Kubernetes/DNS identity, but also as the address advertised by Ray itself. It maintains backward compatibility, so users who are still using Autoscaler v1 won't be affected when choosing the
I think we can keep the current behavior now, and address this in another PR. |
|
Hi @AlterHoodie, I think #5191 overlaps with yours. Would you like to pick up another issue instead? I can find some issues for you, feel free to reach out to me on the Ray Slack. |
|
Hey @win5923, was just finishing up with the PR. Will push the changes in sometime, if you guys haven't decided on how to make the FQDN stable yet and if my implementation feels appropriate, please do use it as reference. |
…d FQDNs Assign replica/host indices so hostname+subdomain can stay unique, including multi-host groups when indexing is off, via NeedsWorkerIndices.
|
The operator feature gate RayClusterWorkerFQDN (alpha, default off) gives each worker Pod a stable FQDN by setting hostname and subdomain and creating the headless Service {cluster}-headless. Single host names are {group}-{replicaIndex}. Multi host names are {group}-{replicaIndex}-{hostIndex}. Replica and host numbers are not in the YAML. reconcilePods assigns them: if numOfHosts is greater than 1 and NeedsWorkerIndices() is true it uses reconcileMultiHostWorkerGroup (inner loop is host index), otherwise the old per Pod loop with hostIndex always 0. Stamping is in DefaultWorkerPodTemplate. The headless Service is created when FQDN is on or when numOfHosts is greater than 1 (the old multi host peer DNS behavior). NeedsWorkerIndices() is RayMultiHostIndexing or RayClusterWorkerFQDN. That coupling is a bit awkward. FQDN needs unique replica and host numbers so hostnames do not collide. For a multi host group that means taking the slice reconcile path and writing slice labels even when RayMultiHostIndexing is off. Indexing itself is only labels and atomic slices, not DNS. Turning FQDN on with indexing off therefore changes how multi host groups are reconciled, not only whether hostname is set. Also on a side note, Curious whether this is the right approach or there is a cleaner split. For example keeping slice reconcile strictly behind RayMultiHostIndexing and having FQDN assign hostnames some other way, or documenting that FQDN for numOfHosts greater than 1 expects indexing. Unit coverage is in pod_test.go and TestReconcileHeadlessService, including FQDN off with indexing still on. E2E is raycluster_worker_fqdn_test.go (single host, recreate, scale). Haven't thoroughly tested |
|
Closing, since this is a duplicate of #5191 |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
Reviewed by Cursor Bugbot for commit 24cb74d. Configure here.
| } | ||
|
|
||
| isRayMultiHostIndexing := worker.NumOfHosts > 1 && features.Enabled(features.RayMultiHostIndexing) | ||
| isRayMultiHostIndexing := worker.NumOfHosts > 1 && features.NeedsWorkerIndices() |
There was a problem hiding this comment.
FQDN orphans existing multi-host pods
Medium Severity
With RayClusterWorkerFQDN on and RayMultiHostIndexing off, NeedsWorkerIndices routes NumOfHosts > 1 groups into reconcileMultiHostWorkerGroup. That path only tracks Pods that already have ray.io/worker-group-replica-name. Existing unlabeled multi-host workers from the legacy path are ignored while a full new replica set is created, leaving the cluster over-provisioned with orphaned Pods.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 24cb74d. Configure here.
Sure, after the ray summit i will talk to the kuberay maintainer which approach is better. |
2 participants
Why are these changes needed?
Ray worker Pods are only addressable by Pod IP today, and that IP changes whenever a Pod is recreated. This PR sets worker
hostname/subdomainfrom replica (and host - for multihost) indices and always creates the RayCluster headless Service so each worker gets a stable Pod FQDN that can survive recreates.Hostname/subdomain assignment still sits behind
RayMultiHostIndexing, since that is where replica indices are assigned. Creating the headless Service is no longer limited to multi-host groups (NumOfHosts > 1).That stretches the meaning of
RayMultiHostIndexing. Looking for guidance on whether to:RayMultiHostIndexing, orRayMultiHostIndexing(default on).Note: The multi_host e2e test has been extended to support FQDN verification , depending on above decision will split into its own e2e test.
Related issue number
Closes #5000
Checks
Manual test instructions
./hack/local_deploy.shor equivalent) and ensureRayMultiHostIndexing=true.kubectl get svc | grep headless kubectl get pods -l ray.io/node-type=worker -o custom-columns=\ NAME:.metadata.name,HOSTNAME:.spec.hostname,SUBDOMAIN:.spec.subdomain,IP:.status.podIP