-
Notifications
You must be signed in to change notification settings - Fork 664
[RayJob] Yunikorn Integration #3948
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.
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
[RayJob] Yunikorn Integration #3948
Conversation
Signed-off-by: Troy Chiu <[email protected]>
ed2f0b2 to
6c0d860
Compare
Signed-off-by: You-Cheng Lin (Owen) <[email protected]>
a091f5e to
ebf0cb3
Compare
Signed-off-by: You-Cheng Lin (Owen) <[email protected]>
Signed-off-by: You-Cheng Lin (Owen) <[email protected]>
Signed-off-by: You-Cheng Lin (Owen) <[email protected]>
Signed-off-by: You-Cheng Lin (Owen) <[email protected]>
ray-operator/controllers/ray/batchscheduler/yunikorn/yunikorn_scheduler.go
Outdated
Show resolved
Hide resolved
Signed-off-by: You-Cheng Lin (Owen) <[email protected]>
Signed-off-by: You-Cheng Lin (Owen) <[email protected]>
Signed-off-by: You-Cheng Lin (Owen) <[email protected]>
926225e to
4eb38e5
Compare
Signed-off-by: You-Cheng Lin (Owen) <[email protected]>
ray-operator/controllers/ray/batchscheduler/yunikorn/yunikorn_scheduler.go
Outdated
Show resolved
Hide resolved
ray-operator/controllers/ray/batchscheduler/yunikorn/yunikorn_scheduler.go
Outdated
Show resolved
Hide resolved
Co-authored-by: Jun-Hao Wan <[email protected]> Signed-off-by: Owen Lin (You-Cheng Lin) <[email protected]>
Co-authored-by: Jun-Hao Wan <[email protected]> Signed-off-by: Owen Lin (You-Cheng Lin) <[email protected]>
Signed-off-by: You-Cheng Lin (Owen) <[email protected]>
| name: rayjob-yunikorn-scheduler-0 | ||
| labels: | ||
| ray.io/gang-scheduling-enabled: "true" | ||
| yunikorn.apache.org/app-id: test-yunikorn-job-0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, just updated.
ray-operator/controllers/ray/batchscheduler/yunikorn/yunikorn_scheduler.go
Outdated
Show resolved
Hide resolved
| return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err | ||
| } | ||
| } else { | ||
| return ctrl.Result{RequeueAfter: RayJobDefaultRequeueDuration}, err |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: GetScheduler always returns nil. For consistency, we can change it to return an error like in other places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice advice, we could open a follow-up PR after this one, since RayCluster controller need to be modified as well.
win5923
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM, Thanks for your hard work! I’ll take another look at the test part.
cc @troychiu for review
…scheduler.go Co-authored-by: Jun-Hao Wan <[email protected]> Signed-off-by: Owen Lin (You-Cheng Lin) <[email protected]>
Signed-off-by: You-Cheng Lin (Owen) <[email protected]>
Signed-off-by: You-Cheng Lin (Owen) <[email protected]>
Future-Outlier
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Future-Outlier
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it looks good to me, thank you
| var submitterGroupSpec corev1.PodSpec | ||
| if rayJobSpec.SubmitterPodTemplate != nil { | ||
| submitterGroupSpec = rayJobSpec.SubmitterPodTemplate.Spec | ||
| } else { | ||
| submitterGroupSpec = common.GetDefaultSubmitterTemplate(rayJobSpec.RayClusterSpec).Spec | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should also check whether RayJobSpec.RayClusterSpec is nil before calling newTaskGroupsFromRayJobSpec, otherwise it can cause a nil pointer panic when RayJob uses an existing cluster with gang-scheduling enabled.
Alternatively, we could add this check in rayjob_controller.go, just prevent user use both batch scheduler and clusterSelector.
example like:
apiVersion: ray.io/v1
kind: RayJob
metadata:
name: rayjob-use-existing-raycluster
labels:
ray.io/gang-scheduling-enabled: "true"
yunikorn.apache.org/app-id: test-yunikorn-0
yunikorn.apache.org/queue: root.test
spec:
entrypoint: python -c "import ray; ray.init(); print(ray.cluster_resources())"
clusterSelector:
ray.io/cluster: raycluster-kuberay
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
But why would the user enable gang-scheduling with clusterSelector?
The RayJob will only have one submitter pod when using clusterSelector which will never need gang-scheduling IIUC?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Normally, this wouldn’t happen, but we want to prevent users from actually setting it this way, because with the current logic it would cause a panic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should avoid using gang scheduling + cluster selector, maybe we have to add this to validation rule in rayjob.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be a follow-up pr
|
cc @owenowenisme for solving the resolve conflict |
Signed-off-by: You-Cheng Lin (Owen) <[email protected]>
845f92a to
0646149
Compare
Future-Outlier
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thank you for solving the merge conflict
| // TODO: remove the legacy labels, i.e "applicationId" and "queue", directly populate labels | ||
| // RayClusterApplicationIDLabelName to RayClusterQueueLabelName to pod labels. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to keep these comments?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, added them back. Thanks!
// propagateTaskGroupsAnnotation is a helper function that propagates the task groups annotation to the child
// if the parent has the task groups annotation, it will be copied to the child
// if the parent doesn't have the task groups annotation, a new one will be created
// TODO: remove the legacy labels, i.e "applicationId" and "queue", directly populate labels
// RayApplicationIDLabelName and RayApplicationQueueLabelName to pod labels.
// Currently we use this function to translate labels "yunikorn.apache.org/app-id" and "yunikorn.apache.org/queue"
// to legacy labels "applicationId" and "queue", this is for the better compatibilities to support older yunikorn
// versions.
Signed-off-by: You-Cheng Lin (Owen) <[email protected]>
| if rayJobSpec.SubmitterPodTemplate != nil { | ||
| submitterGroupSpec = rayJobSpec.SubmitterPodTemplate.Spec | ||
| } else { | ||
| submitterGroupSpec = common.GetDefaultSubmitterTemplate(rayJobSpec.RayClusterSpec).Spec |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This logic looks like from
| submitterTemplate = common.GetDefaultSubmitterTemplate(&rayClusterInstance.Spec) |
Is it possible to use the submitter template passed in so that we don't have duplicate logic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about create a new public function?
// old one
func GetDefaultSubmitterTemplate(rayClusterSpec *rayv1.RayClusterSpec) corev1.PodTemplateSpec {
return corev1.PodTemplateSpec{
Spec: corev1.PodSpec{
Containers: []corev1.Container{
GetDefaultSubmitterContainer(rayClusterSpec),
},
RestartPolicy: corev1.RestartPolicyNever,
},
}
}
// new one
func GetDefaultSubmitterTemplate(rayClusterSpec *rayv1.RayClusterSpec) corev1.PodTemplateSpec {
return corev1.PodTemplateSpec{
Spec: GetSubmitterPodSpec(rayClusterSpec)
}
}
func GetSubmitterPodSpec(rayClusterSpec *rayv1.RayClusterSpec) corev1.PodSpec {
return corev1.PodSpec{
Containers: []corev1.Container{
GetDefaultSubmitterContainer(rayClusterSpec),
},
RestartPolicy: corev1.RestartPolicyNever,
}
}In this case, we can call GetSubmitterPodSpec directly.
3503dca to
99b2d5e
Compare
Signed-off-by: You-Cheng Lin (Owen) <[email protected]>
99b2d5e to
906ab83
Compare
Future-Outlier
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @troychiu for the final review!
troychiu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM Thank you! cc @rueian
Future-Outlier
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Starting with KubeRay 1.5.0, KubeRay supports gang scheduling for RayJob custom resources. Just add a mention for Yunikorn scheduler. Related to ray-project/kuberay#3948. Signed-off-by: win5923 <[email protected]>
…ect#58375) Starting with KubeRay 1.5.0, KubeRay supports gang scheduling for RayJob custom resources. Just add a mention for Yunikorn scheduler. Related to ray-project/kuberay#3948. Signed-off-by: win5923 <[email protected]>
…ect#58375) Starting with KubeRay 1.5.0, KubeRay supports gang scheduling for RayJob custom resources. Just add a mention for Yunikorn scheduler. Related to ray-project/kuberay#3948. Signed-off-by: win5923 <[email protected]>
…ect#58375) Starting with KubeRay 1.5.0, KubeRay supports gang scheduling for RayJob custom resources. Just add a mention for Yunikorn scheduler. Related to ray-project/kuberay#3948. Signed-off-by: win5923 <[email protected]> Signed-off-by: Aydin Abiar <[email protected]>
…ect#58375) Starting with KubeRay 1.5.0, KubeRay supports gang scheduling for RayJob custom resources. Just add a mention for Yunikorn scheduler. Related to ray-project/kuberay#3948. Signed-off-by: win5923 <[email protected]>




Why are these changes needed?
This PR integrates RayJob with Apache Yunikorn as a batch scheduler, enabling RayJob to leverage Yunikorn’s features such as gang-scheduling. This allows both the submitter pod and RayCluster to be included in gang-scheduling.
Special thanks to the original contribution from @troychiu! 🙏 (#3379)
In this PR:
batchSchedulerManagertoRayJobReconcileOptioninmain.goray-job.yunikorn-scheduler.yamlas a demonstration script for gang-scheduling with RayJobyunikorn_scheduler.goValidation
Set the Yunikorn queue to 6Gi memory and 4 cores, same as for RayCluster.

Deploy the KubeRay operator with the batch-scheduler set to Yunikorn:
Deploy a sample RayJob:
Each RayJob consists of:
Total: 2.5 CPU and 4.2Gi memory. Therefore, our queue cannot fit two RayJobs at the same time.
Deploy another RayJob using the same script but change the RayJob name and app ID from
job-0tojob-1.We can see that the resources for RayJob1 (including the submitter pod from job-1) are being held by Yunikorn.
Note that even though the submitter pod of

job-1has sufficient resources for scheduling, gang-scheduling requires that all pods be scheduled together.After deleting RayJob-0, the resources should be sufficient for RayJob-1, so RayJob-1 is now up and running.
Testing compatibility with RayJob HTTPMode
To make sure RayJob Yunikorn works well with HTTPMode submission, I did the following steps:
submissionModetoHTTPModeI enter the shell of head pod and query the job using
ray job listandray job logs JobID, it should be successful with the correct log.Related issue number
Closes #3284
Checks