[Feature] Support recreate pods for RayCluster using RayClusterSpec.upgradeStrategy

[win5923]

Why are these changes needed?

Currently, when users update a RayCluster spec (e.g., update the image), users must re-create the cluster or manually set spec.suspend to true and after all Pods are deleted and then set it back to false which is not particularly convenient for users deploying with GitOps systems like ArgoCD.

Ref:

• [Feature] Identify and apply changes on ray-cluster #2534
• [Feature] Add ability to modify the image of a worker group in RayCluster with rolling upgrade or restart #3905

Design doc: https://docs.google.com/document/d/1xQLm0-WQWD-FkufxBJYklOJGvVn4RLk0_vPjLD5ax7o/edit?usp=sharing

Changes

• Add spec.upgradeStrategy field to RayCluster CRD
• Supports two values:
  • Recreate: During upgrade, Recreate strategy will delete all existing pods before creating new ones.
  • None: No new pod will be created while the strategy is set to None

Implementation

• Store hash of HeadGroupSpec.Template to head pod and workerGroup.Template to worker pod annotations during creation with ray.io/pod-template-hash
• Compare stored hash with current head pod or worker pod template hash to detect changes and recreate all pods

I only compare the HeadGroupSpec.Template and workerGroup.Template because these define the pod-related configurations. The RayCluster.Spec contains many dynamic and component-specific settings (e.g., autoscaler configs, rayStartParams).

Example:

apiVersion: ray.io/v1
kind: RayCluster
metadata:
  name: raycluster-kuberay
spec:
  upgradeStrategy:
    type: Recreate
  rayVersion: '2.48.0'

Related issue number

Closes #2534 #3905

Checks

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

[win5923]

Hi @andrewsykim, I followed you previous comments to adding a spec.upgradeStrategy API to RayCluster. But for now. I'm concerned this approach may introduce some issues:

1. Confusion with existing API: We already have upgradeStrategy for RayService. Adding another upgradeStrategy to RayCluster could be confusing for users and creates unclear separation of concerns.
2. Breaking RayJob workflows: For RayJob, setting upgradeStrategy=Recreate on the RayCluster would cause pod recreation during job execution, leading to job interruption and loss of running jobs.

Maybe we can just add a feature gate instead of using spec.upgradeStrategy.type field in RayCluster to enable the recreate behavior. WDYT?

[andrewsykim]

Maybe we can just add a feature gate instead of using spec.upgradeStrategy.type field in RayCluster to enable the recreate behavior. WDYT?

Feature gates are used to gate features that are in early development and not ready for wider adoption, it shouldn't be used to change the behavior of RayCluster because it will eventually be on by default (and forced on).

[andrewsykim]

I think both of those concerns are valid, but I don't think this is a problem with separation of concerns as RayCluster is a building block for both RayService and RayJob. For those cases you mentioned, we should have validation to ensure RayCluster upgrade strategy cannot be set when used w/ RayJob and RayService

[Future-Outlier]

need some review help, cc @seanlaii @JiangJiaWei1103 @CheyuWu

[andrewsykim]

Store hash of HeadGroupSpec.Template to head pod and workerGroup.Template to worker pod annotations during creation with ray.io/pod-template-hash

Something to be mindful of with this strategy is that adding new fields or slightly changing some property of the template (e.g. changing default entrypoint), could trigger an unintended recreate when a user upgrades KubeRay. I think we ran into something similar when dealing with upgrade behavior in RayService, see #2320 for reference.

[andrewsykim]

also rename two constants above to start with RayService*

[win5923]

Thanks, updated in f3a9b81.

[seanlaii]

nit:

Suggested change

	Recreate RayClusterUpgradeType = "Recreate"
	RayClusterUpgradeRecreate RayClusterUpgradeType = "Recreate"

[win5923]

Thanks, updated in f3a9b81

[win5923]

Something to be mindful of with this strategy is that adding new fields or slightly changing some property of the template (e.g. changing default entrypoint), could trigger an unintended recreate when a user upgrades KubeRay. I think we ran into something similar when dealing with upgrade behavior in RayService, see #2320 for reference.

Thanks for pointing this out. I’ll add the ray.io/kuberay-version annotation to both the head and worker pods as well, so we can detect KubeRay version changes and avoid unintended recreates. I really appreciate your comment.

Updated: I think we don't need to check whether the KubeRayVersion is same or not, cause we only compare HeadGroupSpec.Template and WorkerGroupSpecs.Template, which are just corev1.PodTemplateSpec. This comparison is independent of the KubeRay Operator version.

[machichima]

Not quite understand this part. When the KubeRayVersion updated, we will update the pods' kuberay version annotation, then what will happen? It seems like we do not have extra handling when kuberay version update. If so, should we just remove it? Then I think we do not need updatePodsAnnotations and calculatePodTemplateHash.

[win5923]

My initial thought was to follow the approach used in RayService:
when the KubeRay Operator is upgraded, the reconciliation logic entering shouldRecreatePodsForUpgrade would first check whether the KubeRayVersion has changed. If it differed, we would compute a new pod template hash and update both ray.io/pod-template-hash and ray.io/kuberay-version.

After revisiting it, I realized that what we’re actually comparing is HeadGroupSpec.Template and WorkerGroupSpecs.Template, which are just corev1.PodTemplateSpec. This comparison is independent of the KubeRay Operator version, so I’ll remove it.

cc @andrewsykim

[win5923]

Revert the changes: e4cb491

Thanks for the review!!