[Feature] Support recreate pods for RayCluster using RayClusterSpec.upgradeStrategy #4185
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -158,12 +158,27 @@ func configureGCSFaultTolerance(podTemplate *corev1.PodTemplateSpec, instance ra | |
| } | ||
| } | ||
|
|
||
| func GeneratePodTemplateHash(template corev1.PodTemplateSpec) (string, error) { | ||
|
Collaborator
There was a problem hiding this comment. Just curious, is it necessary to have this wrapper function, or is it redundant?
Member
There was a problem hiding this comment. I think this is for better readability.
Collaborator
There was a problem hiding this comment. Do we really consider only the Pod template? I feel that most fields should actually be taken into account, with only a few exceptions in the worker group spec, such as ScaleStrategy, Suspend, Replicas, MinReplicas, MaxReplicas, and IdleTimeoutSeconds.
Member
There was a problem hiding this comment. I don’t have a strong preference since RayCluster is a custom resource. Also, in Kubernetes, both Deployments and StatefulSets primarily compare/check the Pod template. source:
Member
Author
There was a problem hiding this comment. Yes, most of the requirements raised by users at the moment are primarily related to image updates, which is why my starting point is at the Pod level rather than the RayCluster level. But your point makes sense, there are many configurations that should trigger a Pod recreate (like
Member
There was a problem hiding this comment. I think you can implement Rueian's advice, since RayCluster is a custom resource, therefore we have custom behavior make sense.
Member
There was a problem hiding this comment. and either we support which version, in the future, if we want to change to another version are all breaking change, so I would vote to Rueian's solution.
Member
There was a problem hiding this comment. just talked with @win5923
Member
Author
There was a problem hiding this comment. Hi @Future-Outlier, @rueian, @CheyuWu, @machichima I followed the RayService approach to implement the UpgradeStrategy for RayCluster. During reconciliation, we only need to compare the hash on the head Pod to determine whether an upgrade is required. This allows us to avoid re-comparing the spec across all head and worker Pods, simplifying the upgrade detection logic and reducing unnecessary overhead. |
||
| return utils.GenerateJsonHash(template) | ||
| } | ||
|
|
||
| // DefaultHeadPodTemplate sets the config values | ||
| func DefaultHeadPodTemplate(ctx context.Context, instance rayv1.RayCluster, headSpec rayv1.HeadGroupSpec, podName string, headPort string) corev1.PodTemplateSpec { | ||
| // TODO (Dmitri) The argument headPort is essentially unused; | ||
| // headPort is passed into setMissingRayStartParams but unused there for the head pod. | ||
| // To mitigate this awkwardness and reduce code redundancy, unify head and worker pod configuration logic. | ||
|
|
||
| log := ctrl.LoggerFrom(ctx) | ||
| // Calculate the pod template hash before any modifications | ||
| // This ensures the hash reflects the original user-defined template for upgrade detection | ||
| templateHash := "" | ||
| podTemplate := headSpec.Template | ||
| if hash, err := GeneratePodTemplateHash(podTemplate); err == nil { | ||
| templateHash = hash | ||
| } else { | ||
| log.Error(err, "Failed to generate pod template hash for head group") | ||
| } | ||
|
win5923 marked this conversation as resolved.
Outdated
|
||
|
|
||
| if utils.IsDeterministicHeadPodNameEnabled() { | ||
| podTemplate.Name = podName | ||
| } else { | ||
|
|
@@ -173,6 +188,13 @@ func DefaultHeadPodTemplate(ctx context.Context, instance rayv1.RayCluster, head | |
| // This ensures privilege of KubeRay users are contained within the namespace of the RayCluster. | ||
| podTemplate.ObjectMeta.Namespace = instance.Namespace | ||
|
|
||
| if templateHash != "" { | ||
| if podTemplate.Annotations == nil { | ||
| podTemplate.Annotations = make(map[string]string) | ||
| } | ||
| podTemplate.Annotations[utils.PodTemplateHashKey] = templateHash | ||
| } | ||
|
|
||
| // Update rayStartParams with top-level Resources for head group. | ||
| updateRayStartParamsResources(ctx, headSpec.RayStartParams, headSpec.Resources) | ||
|
|
||
|
|
@@ -296,12 +318,29 @@ func getEnableProbesInjection() bool { | |
|
|
||
| // DefaultWorkerPodTemplate sets the config values | ||
| func DefaultWorkerPodTemplate(ctx context.Context, instance rayv1.RayCluster, workerSpec rayv1.WorkerGroupSpec, podName string, fqdnRayIP string, headPort string, replicaGrpName string, replicaIndex int, numHostIndex int) corev1.PodTemplateSpec { | ||
| log := ctrl.LoggerFrom(ctx) | ||
|
|
||
| podTemplate := workerSpec.Template | ||
| // Calculate the pod template hash before any modifications | ||
| // This ensures the hash reflects the original user-defined template for upgrade detection | ||
| templateHash := "" | ||
| if hash, err := GeneratePodTemplateHash(podTemplate); err == nil { | ||
| templateHash = hash | ||
| } else { | ||
| log.Error(err, "Failed to generate pod template hash for worker group", "groupName", workerSpec.GroupName) | ||
| } | ||
| podTemplate.GenerateName = podName | ||
|
|
||
| // Pods created by RayCluster should be restricted to the namespace of the RayCluster. | ||
| // This ensures privilege of KubeRay users are contained within the namespace of the RayCluster. | ||
| podTemplate.ObjectMeta.Namespace = instance.Namespace | ||
|
|
||
| if templateHash != "" { | ||
| if podTemplate.Annotations == nil { | ||
| podTemplate.Annotations = make(map[string]string) | ||
| } | ||
| podTemplate.Annotations[utils.PodTemplateHashKey] = templateHash | ||
| } | ||
| // The Ray worker should only start once the GCS server is ready. | ||
| // only inject init container only when ENABLE_INIT_CONTAINER_INJECTION is true | ||
| enableInitContainerInjection := getEnableInitContainerInjection() | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.