[Feature] Support managedBy on RayService for integration with MultiKueue

[hiboyang]

Search before asking

• I had searched in the issues and found no similar feature requirement.

Description

To integrate RayService with Kueue MultiKueue, need a new field on RayService: spec.managedBy. See previous discussion: #2544

The logic will be simple. RayJob already has this field:

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

Lines 108 to 111 in 0e64bd8

	if manager := utils.ManagedByExternalController(rayJobInstance.Spec.ManagedBy); manager != nil {
	logger.Info("Skipping RayJob managed by a custom controller", "managed-by", manager)
	return ctrl.Result{}, nil
	}

Use case

For Kueue MultiKueue, one cluster can manage and dispatch jobs to multiple worker clusters.

Related issues

#2544

Are you willing to submit a PR?

• Yes I am willing to submit a PR!

[Future-Outlier]

Hi, could you describe the specific use case for adding this field in the GitHub issue?

I have the following questions:

1. Would the managedBy field under RayService's RayClusterSpec be sufficient to address your use case?

2. I looked through Kueue's current implementation for handling KubeRay CRDs:

   1. Currently, Kueue only has a RayJob controller and a RayCluster controller.
      1. The Kueue's RayJob controller handles RayJob CRs.
      2. The Kueue's RayCluster controller handles both RayCluster and RayService CRs.
         1. It supports autoscaling.
         2. If we add this field, wouldn't the logic for handling managedBy need to diverge between RayCluster and RayService? That doesn't feel symmetric. My understanding is that the design philosophy is to keep RayCluster and RayService handling as uniform as possible, which aligns with why Kueue only has a RayJob controller and a RayCluster controller rather than a separate RayService controller.

3. Could you list the specific changes required if we add this field? What controller code changes are needed in KubeRay, and what changes are needed in Kueue?

[Future-Outlier]

Hi, @hiboyang thanks for opening an issue, plz reply my questions before supporting this, tks!

[hiboyang]

Hi @Future-Outlier , thanks for the questions, here are answers:

1. Would the managedBy field under RayService's RayClusterSpec be sufficient to address your use case?

Yes, should be good as starting point to unblock our use case (run RayService with MultiKueue).

2. MultiKueue has a way to support external custom job: https://github.com/kubernetes-sigs/kueue/tree/main/keps/2349-multikueue-external-custom-job-support, which requires managedBy field on the custom job. I plan to use it to test RayService.

3. Right now, only need change from KubeRay to add managedBy field on RayService, and by-pass RayService reconciliation. Same logic as RayJob:

 if manager := utils.ManagedByExternalController(rayJobInstance.Spec.ManagedBy); manager != nil { 
 	logger.Info("Skipping RayJob managed by a custom controller", "managed-by", manager) 
 	return ctrl.Result{}, nil 
 } 

[lorriexingfang]

I will take this one

[Future-Outlier]

Thank you for replying so quickly. I think it’s OK to support this change to make the API more explicit, even if it isn’t strictly necessary and still works as-is today.

@rueian and @andrewsykim are both OK with this change, and I’ll sync with @kevin85421 to make sure he’s OK with it as well.

[mimowo]

+1, Kueue maintainer here

For the reliable support of RayService via MultiKueue we need to integrate at the RayService level, which requires the managedBy field. This allows Kueue to integrate at the "top level" (RayService) and create a copy of the entire RayService on the worker cluster, while suspending the operation of the RaySerivce on the management cluster.

Copying the top level object is important, for example, to make sure the Gateway is managed.

[Future-Outlier]

chatted with @kevin85421 , the API looks good to him!

[Future-Outlier]

+1, looks good to me now.