[Scheduler] Replace AddMetadataToPod with AddMetadataToChildResource across all schedulers #4123

win5923 · 2025-10-10T14:28:21Z

Why are these changes needed?

Replace the RayCluster-only method AddMetadataToPod with AddMetadataToChildResource to support RayJob and future CRDs. This simplifies scheduler integration and future extensibility.

For kai-scheduler and scheduler-plugins, only RayCluster is supported.
For Volcano and YuniKorn, both RayCluster and RayJob are supported.

E2E:

Volcano
- RayCluster
- RayJob
  - submissionMode: "K8sJobMode"
  - submissionMode: "SidecarMode"

Yunikorn

RayCluster

k get pod
NAME                                      READY   STATUS    RESTARTS   AGE
test-yunikorn-0-head-7h28f                1/1     Running   0          42m
test-yunikorn-0-worker-worker-j9sn6       1/1     Running   0          42m
test-yunikorn-0-worker-worker-sw94t       1/1     Running   0          42m
test-yunikorn-1-head-xkvxt                0/1     Pending   0          2m44s
test-yunikorn-1-worker-worker-6gvbn       0/1     Pending   0          2m44s
test-yunikorn-1-worker-worker-cmcrc       0/1     Pending   0          2m20s
tg-test-yunikorn-1-headgroup-z455wd19i0   0/1     Pending   0          2m42s
tg-test-yunikorn-1-worker-ejszgg5oqw      1/1     Running   0          2m42s
tg-test-yunikorn-1-worker-txnkkpfvtr      0/1     Pending   0          2m42s

RayJob

submissionMode: "k8sJobMode"

k get pod
NAME                                               READY   STATUS      RESTARTS   AGE
rayjob-yunikorn-0-bjvjf-head-p8mwp                 1/1     Running     0          2m36s
rayjob-yunikorn-0-bjvjf-small-group-worker-zrnlw   1/1     Running     0          2m36s
rayjob-yunikorn-0-f96wv                            0/1     Completed   0          2m7s
rayjob-yunikorn-1-5jztp-head-qnnkg                 0/1     Pending     0          61s
rayjob-yunikorn-1-5jztp-small-group-worker-2r4j9   0/1     Pending     0          61s
tg-rayjob-yunikorn-1-headgroup-5vzbjzlk8n          0/1     Pending     0          59s
tg-rayjob-yunikorn-1-small-group-xd8ckjlb91        0/1     Pending     0          59s
tg-rayjob-yunikorn-1-submittergroup-pezuwpn2e6     1/1     Running     0          59s

submissionMode: "SidecarMode"

 k get pod
 NAME                                               READY   STATUS     RESTARTS   AGE
 rayjob-yunikorn-0-pd9s5-head-nvm9h                 1/2     NotReady   0          57s
 rayjob-yunikorn-0-pd9s5-small-group-worker-hp52b   1/1     Running    0          57s
 rayjob-yunikorn-1-8g8gb-head-2xmbc                 0/2     Pending    0          25s
 rayjob-yunikorn-1-8g8gb-small-group-worker-dbkl5   0/1     Pending    0          25s
 tg-rayjob-yunikorn-1-headgroup-2k9po3hy3k          0/1     Pending    0          24s
 tg-rayjob-yunikorn-1-small-group-9jklkkj7up        0/1     Pending    0          24s

Related issue number

Closes #4097

Checks

I've made sure the tests are passing.
Testing Strategy
- Unit tests
- Manual tests
- This PR is not tested :(

Copilot

Pull Request Overview

This PR replaces the RayCluster-specific AddMetadataToPod method with the more generic AddMetadataToChildResource across all batch schedulers to support both RayCluster and RayJob resources, improving extensibility for future CRDs.

Key Changes:

Removed AddMetadataToPod method from the BatchScheduler interface and all implementations
Updated all scheduler implementations to use AddMetadataToChildResource with generic metav1.Object types
Modified call sites in RayClusterReconciler to use the new method signature

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`ray-operator/controllers/ray/raycluster_controller.go`	Updated method calls from `AddMetadataToPod` to `AddMetadataToChildResource` with reordered parameters
`ray-operator/controllers/ray/batchscheduler/yunikorn/yunikorn_scheduler.go`	Removed deprecated `AddMetadataToPod` method
`ray-operator/controllers/ray/batchscheduler/volcano/volcano_scheduler.go`	Removed deprecated `AddMetadataToPod` method
`ray-operator/controllers/ray/batchscheduler/scheduler-plugins/scheduler_plugins_test.go`	Updated test function names and method calls to use `AddMetadataToChildResource`
`ray-operator/controllers/ray/batchscheduler/scheduler-plugins/scheduler_plugins.go`	Replaced `AddMetadataToPod` with `AddMetadataToChildResource`, added `addSchedulerNameToObject` helper
`ray-operator/controllers/ray/batchscheduler/kai-scheduler/kai_scheduler_test.go`	Updated test function names and method calls to use `AddMetadataToChildResource`
`ray-operator/controllers/ray/batchscheduler/kai-scheduler/kai_scheduler.go`	Replaced `AddMetadataToPod` with `AddMetadataToChildResource`, added `addSchedulerNameToObject` helper, removed RayCluster-specific import
`ray-operator/controllers/ray/batchscheduler/interface/interface.go`	Removed `AddMetadataToPod` from interface and default implementation, removed unused imports

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-23T19:34:21Z

ray-operator/controllers/ray/batchscheduler/scheduler-plugins/scheduler_plugins.go

+func addSchedulerNameToObject(obj metav1.Object, schedulerName string) {
+	switch obj := obj.(type) {
+	case *corev1.Pod:
+		obj.Spec.SchedulerName = schedulerName
+	case *corev1.PodTemplateSpec:
+		obj.Spec.SchedulerName = schedulerName
+	}
 }


The addSchedulerNameToObject function is duplicated across multiple scheduler implementations (scheduler-plugins and kai-scheduler). Consider extracting this utility function to a shared package to reduce code duplication and improve maintainability.

Copilot · 2025-10-23T19:34:22Z

ray-operator/controllers/ray/batchscheduler/kai-scheduler/kai_scheduler.go

+func addSchedulerNameToObject(obj metav1.Object, schedulerName string) {
+	switch obj := obj.(type) {
+	case *corev1.Pod:
+		obj.Spec.SchedulerName = schedulerName
+	case *corev1.PodTemplateSpec:
+		obj.Spec.SchedulerName = schedulerName
+	}
 }


This function is an exact duplicate of the one in scheduler-plugins. Consider extracting this utility function to a shared package to reduce code duplication and improve maintainability.

win5923 · 2025-10-23T19:39:07Z

@troychiu @owenowenisme @Future-Outlier
PTAL when you have time, thanks.

rueian · 2025-10-23T21:01:30Z

ray-operator/controllers/ray/batchscheduler/kai-scheduler/kai_scheduler.go

+		obj.Spec.SchedulerName = schedulerName
+	case *corev1.PodTemplateSpec:
+		obj.Spec.SchedulerName = schedulerName
+	}


Should we do a panic for safty if the object does not match all the cases?

Hi @rueian, thanks for the review.
I think we don't need panic here, cause current silent no-op behavior for unsupported types is intentional.

At line 899, a *rayv1.RayCluster is passed in. This isn’t a Pod or PodTemplateSpec, and that’s by design.
AddMetadataToChildResource has two roles:

propagate metadata (labels, annotations)

set the scheduler name.

When it’s called with a RayCluster, only the metadata propagation part applies. RayCluster itself doesn’t need a schedulerName; only the actual Pods (Head/Worker Pods for RayCluster or Job for RayJob) do.

kuberay/ray-operator/controllers/ray/rayjob_controller.go

Lines 875 to 922 in 2d52001

func (r *RayJobReconciler) getOrCreateRayClusterInstance(ctx context.Context, rayJobInstance *rayv1.RayJob) (*rayv1.RayCluster, error) {

logger := ctrl.LoggerFrom(ctx)

rayClusterNamespacedName := common.RayJobRayClusterNamespacedName(rayJobInstance)

logger.Info("try to find existing RayCluster instance", "name", rayClusterNamespacedName.Name)

rayClusterInstance := &rayv1.RayCluster{}

if err := r.Get(ctx, rayClusterNamespacedName, rayClusterInstance); err != nil {

if errors.IsNotFound(err) {

logger.Info("RayCluster not found", "RayCluster", rayClusterNamespacedName)

if len(rayJobInstance.Spec.ClusterSelector) != 0 {

err := fmt.Errorf("clusterSelector mode is enabled, but RayCluster %s/%s is not found: %w", rayClusterNamespacedName.Namespace, rayClusterNamespacedName.Name, err)

r.Recorder.Eventf(rayJobInstance, corev1.EventTypeWarning, string(utils.RayClusterNotFound), "RayCluster %s/%s set in the clusterSelector is not found. It must be created manually", rayClusterNamespacedName.Namespace, rayClusterNamespacedName.Name)

return nil, err

}

logger.Info("RayCluster not found, creating RayCluster!", "RayCluster", rayClusterNamespacedName)

rayClusterInstance, err = r.constructRayClusterForRayJob(rayJobInstance, rayClusterNamespacedName.Name)

if err != nil {

return nil, err

}

if r.options.BatchSchedulerManager != nil && rayJobInstance.Spec.SubmissionMode == rayv1.K8sJobMode {

if scheduler, err := r.options.BatchSchedulerManager.GetScheduler(); err == nil {

// Group name is only used for individual pods to specify their task group ("headgroup", "worker-group-1", etc.).

// RayCluster contains multiple groups, so we pass an empty string.

scheduler.AddMetadataToChildResource(ctx, rayJobInstance, rayClusterInstance, "")

} else {

return nil, err

}

}

if err := r.Create(ctx, rayClusterInstance); err != nil {

r.Recorder.Eventf(rayJobInstance, corev1.EventTypeWarning, string(utils.FailedToCreateRayCluster), "Failed to create RayCluster %s/%s: %v", rayClusterInstance.Namespace, rayClusterInstance.Name, err)

return nil, err

}

r.Recorder.Eventf(rayJobInstance, corev1.EventTypeNormal, string(utils.CreatedRayCluster), "Created RayCluster %s/%s", rayClusterInstance.Namespace, rayClusterInstance.Name)

} else {

return nil, err

}

}

logger.Info("Found the associated RayCluster for RayJob", "RayCluster", rayClusterNamespacedName)

// Verify that RayJob is not in cluster selector mode first to avoid nil pointer dereference error during spec comparison.

// This is checked by ensuring len(rayJobInstance.Spec.ClusterSelector) equals 0.

if len(rayJobInstance.Spec.ClusterSelector) == 0 && !utils.CompareJsonStruct(rayClusterInstance.Spec, *rayJobInstance.Spec.RayClusterSpec) {

logger.Info("Disregard changes in RayClusterSpec of RayJob")

}

return rayClusterInstance, nil

}

Move AddSchedulerNameToObject into shared utils to reduce duplication. 16484bf

Shouldn't it set the scheduler name to the PodTemplateSpecs in the rayv1.RayCluster?

I think both are fine. Do you have any concerns?

…across all schedulers Signed-off-by: win5923 <[email protected]>

Signed-off-by: win5923 <[email protected]>

win5923 marked this pull request as ready for review October 23, 2025 18:04

win5923 requested review from MortalHappiness, andrewsykim, kevin85421 and rueian as code owners October 23, 2025 18:04

rueian requested a review from Copilot October 23, 2025 19:33

Copilot AI reviewed Oct 23, 2025

View reviewed changes

rueian reviewed Oct 23, 2025

View reviewed changes

win5923 force-pushed the scheduler-interface-migrate branch 2 times, most recently from 16484bf to ae3c527 Compare October 24, 2025 12:09

[Scheduler] Replace AddMetadataToPod with AddMetadataToChildResource …

e8c884f

…across all schedulers Signed-off-by: win5923 <[email protected]>

win5923 force-pushed the scheduler-interface-migrate branch from ae3c527 to 148b6f7 Compare October 24, 2025 12:19

Extract AddSchedulerNameToObject to batchscheduler utils package

8601856

Signed-off-by: win5923 <[email protected]>

win5923 force-pushed the scheduler-interface-migrate branch from 148b6f7 to 8601856 Compare October 24, 2025 12:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[Scheduler] Replace AddMetadataToPod with AddMetadataToChildResource across all schedulers #4123

[Scheduler] Replace AddMetadataToPod with AddMetadataToChildResource across all schedulers #4123

Uh oh!

win5923 commented Oct 10, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 23, 2025

Uh oh!

Copilot AI Oct 23, 2025

Uh oh!

win5923 commented Oct 23, 2025

Uh oh!

rueian Oct 23, 2025

Uh oh!

win5923 Oct 24, 2025

Uh oh!

win5923 Oct 24, 2025

Uh oh!

rueian Oct 24, 2025

Uh oh!

win5923 Oct 25, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	func (r RayJobReconciler) getOrCreateRayClusterInstance(ctx context.Context, rayJobInstance rayv1.RayJob) (*rayv1.RayCluster, error) {
	logger := ctrl.LoggerFrom(ctx)
	rayClusterNamespacedName := common.RayJobRayClusterNamespacedName(rayJobInstance)
	logger.Info("try to find existing RayCluster instance", "name", rayClusterNamespacedName.Name)

	rayClusterInstance := &rayv1.RayCluster{}
	if err := r.Get(ctx, rayClusterNamespacedName, rayClusterInstance); err != nil {
	if errors.IsNotFound(err) {
	logger.Info("RayCluster not found", "RayCluster", rayClusterNamespacedName)
	if len(rayJobInstance.Spec.ClusterSelector) != 0 {
	err := fmt.Errorf("clusterSelector mode is enabled, but RayCluster %s/%s is not found: %w", rayClusterNamespacedName.Namespace, rayClusterNamespacedName.Name, err)
	r.Recorder.Eventf(rayJobInstance, corev1.EventTypeWarning, string(utils.RayClusterNotFound), "RayCluster %s/%s set in the clusterSelector is not found. It must be created manually", rayClusterNamespacedName.Namespace, rayClusterNamespacedName.Name)
	return nil, err
	}

	logger.Info("RayCluster not found, creating RayCluster!", "RayCluster", rayClusterNamespacedName)
	rayClusterInstance, err = r.constructRayClusterForRayJob(rayJobInstance, rayClusterNamespacedName.Name)
	if err != nil {
	return nil, err
	}
	if r.options.BatchSchedulerManager != nil && rayJobInstance.Spec.SubmissionMode == rayv1.K8sJobMode {
	if scheduler, err := r.options.BatchSchedulerManager.GetScheduler(); err == nil {
	// Group name is only used for individual pods to specify their task group ("headgroup", "worker-group-1", etc.).
	// RayCluster contains multiple groups, so we pass an empty string.
	scheduler.AddMetadataToChildResource(ctx, rayJobInstance, rayClusterInstance, "")
	} else {
	return nil, err
	}
	}
	if err := r.Create(ctx, rayClusterInstance); err != nil {
	r.Recorder.Eventf(rayJobInstance, corev1.EventTypeWarning, string(utils.FailedToCreateRayCluster), "Failed to create RayCluster %s/%s: %v", rayClusterInstance.Namespace, rayClusterInstance.Name, err)
	return nil, err
	}
	r.Recorder.Eventf(rayJobInstance, corev1.EventTypeNormal, string(utils.CreatedRayCluster), "Created RayCluster %s/%s", rayClusterInstance.Namespace, rayClusterInstance.Name)
	} else {
	return nil, err
	}
	}
	logger.Info("Found the associated RayCluster for RayJob", "RayCluster", rayClusterNamespacedName)

	// Verify that RayJob is not in cluster selector mode first to avoid nil pointer dereference error during spec comparison.
	// This is checked by ensuring len(rayJobInstance.Spec.ClusterSelector) equals 0.
	if len(rayJobInstance.Spec.ClusterSelector) == 0 && !utils.CompareJsonStruct(rayClusterInstance.Spec, *rayJobInstance.Spec.RayClusterSpec) {
	logger.Info("Disregard changes in RayClusterSpec of RayJob")
	}

	return rayClusterInstance, nil
	}

[Scheduler] Replace AddMetadataToPod with AddMetadataToChildResource across all schedulers #4123

Are you sure you want to change the base?

[Scheduler] Replace AddMetadataToPod with AddMetadataToChildResource across all schedulers #4123

Uh oh!

Conversation

win5923 commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why are these changes needed?

Related issue number

Checks

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

win5923 commented Oct 23, 2025

Uh oh!

rueian Oct 23, 2025

Choose a reason for hiding this comment

Uh oh!

win5923 Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

win5923 Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

rueian Oct 24, 2025

Choose a reason for hiding this comment

Uh oh!

win5923 Oct 25, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

win5923 commented Oct 10, 2025 •

edited

Loading