Search before asking
KubeRay Component
ray-operator
KubeRay Version
master
What happened
RayClusterReconciler contains three statements that attempt to clear ScaleStrategy.WorkersToDelete, but they are ineffective.
For example:
for _, worker := range instance.Spec.WorkerGroupSpecs {
worker.ScaleStrategy.WorkersToDelete = []string{}
}worker is a value copy of WorkerGroupSpec, so the assignment only modifies the local copy and does not update instance.Spec.
The same applies to reconcileMultiHostWorkerGroup. Additionally, the modified spec is never written back to the API server, and one of the assignments is immediately followed by a return.
The operator never clears the field, and that is intentional — the field is owned by the Ray Autoscaler.
See cleanUpWorkersToDelete in suite_helpers_test.go, a test helper that exists precisely to simulate the autoscaler doing it:
// Updating WorkersToDelete is the responsibility of the Ray Autoscaler. In this function,
// we simulate the behavior of the Ray Autoscaler after the scaling process has finished.
Expected behavior
These statements should not appear to clear WorkersToDelete when they have no effect.
WorkersToDelete is currently cleared by the Ray Autoscaler, and whether the operator should also clear it is being discussed in #733.
Proposal
Remove the three statements.
Are you willing to submit a PR?
Search before asking
KubeRay Component
ray-operator
KubeRay Version
master
What happened
RayClusterReconcilercontains three statements that attempt to clearScaleStrategy.WorkersToDelete, but they are ineffective.For example:
worker is a value copy of
WorkerGroupSpec, so the assignment only modifies the local copy and does not updateinstance.Spec.The same applies to
reconcileMultiHostWorkerGroup. Additionally, the modified spec is never written back to the API server, and one of the assignments is immediately followed by a return.raycluster_controller.go#L1153raycluster_controller.go#L1358raycluster_controller.go#L1362The operator never clears the field, and that is intentional — the field is owned by the Ray Autoscaler.
See
cleanUpWorkersToDeleteinsuite_helpers_test.go, a test helper that exists precisely to simulate the autoscaler doing it:Expected behavior
These statements should not appear to clear
WorkersToDeletewhen they have no effect.WorkersToDelete is currently cleared by the Ray Autoscaler, and whether the operator should also clear it is being discussed in #733.
Proposal
Remove the three statements.
Are you willing to submit a PR?