[Feature] Make replicas optional for WorkerGroupSpec #1443
Conversation
There was a problem hiding this comment.
Somewhere in the code, should add a comment describing the reasons described in your PR description? Otherwise it might be hard to trace back the reason for making it optional if people look at the code later.
Ideally we can link to some Argo documentation for "If the field is optional, ArgoCD will not have the reverting behavior."
Otherwise just minor questions/concerns, looks good to me
| @@ -215,11 +215,26 @@ func GenerateIdentifier(clusterName string, nodeType rayv1alpha1.RayNodeType) st | |||
| return fmt.Sprintf("%s-%s", clusterName, nodeType) | |||
| } | |||
|
|
|||
| func GetWorkerGroupDesiredReplicas(workerGroupSpec rayv1alpha1.WorkerGroupSpec) int32 { | |||
| // Always adhere to min/max replicas constraints. If minReplicas > maxReplicas, minReplicas will be used. | |||
There was a problem hiding this comment.
If minReplicas > maxReplicas shouldn't we raise an error instead of silently using minReplicas? At the very least should we add a warning log?
Done
Update the PR description. |
Make replicas optional for WorkerGroupSpec
Why are these changes needed?
Some GitOps systems, like Flux, will revert changes that are not initiated by the system itself. The Ray Autoscaler modifies the
Replicasfield to scale the Ray cluster up or down. Consequently, a conflict arises where the Autoscaler makes updates to the field, and Flux continuously reverts them. To address this issue, this PR makes theReplicasfield optional. If the field is optional, Flux will not have the reverting behavior.Related issue number
Closes #1154
Closes #1120
Closes #265
Checks