Skip to content

[Autoscaler] Add validation to require RayCluster v2 when using idleTimeoutSeconds #4162

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+212 −0
Open

[Autoscaler] Add validation to require RayCluster v2 when using idleTimeoutSeconds #4162

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions ray-operator/controllers/ray/utils/validation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,9 @@ func ValidateRayClusterSpec(spec *rayv1.RayClusterSpec, annotations map[string]s
if err := validateRayGroupLabels(workerGroup.GroupName, workerGroup.RayStartParams, workerGroup.Labels); err != nil {
return err
}
if err := validateWorkerGroupIdleTimeout(workerGroup, spec); err != nil {
return err
}
}

if annotations[RayFTEnabledAnnotationKey] != "" && spec.GcsFaultToleranceOptions != nil {
Expand Down Expand Up @@ -596,3 +599,21 @@ func validateLegacyDeletionPolicies(rayJob *rayv1.RayJob) error {

return nil
}

// validateWorkerGroupIdleTimeout validates the idleTimeoutSeconds field in a worker group spec
func validateWorkerGroupIdleTimeout(workerGroup rayv1.WorkerGroupSpec, spec *rayv1.RayClusterSpec) error {
idleTimeoutSeconds := workerGroup.IdleTimeoutSeconds
if idleTimeoutSeconds != nil {
if *idleTimeoutSeconds < 0 {
return fmt.Errorf("idleTimeoutSeconds must be non-negative, got %d", *idleTimeoutSeconds)
}

// idleTimeoutSeconds only allowed on autoscaler v2
envVar, exists := EnvVarByName(RAY_ENABLE_AUTOSCALER_V2, spec.HeadGroupSpec.Template.Spec.Containers[RayContainerIndex].Env)
if !exists || (envVar.Value != "1" && envVar.Value != "true") {
return fmt.Errorf("worker group %s has idleTimeoutSeconds set, but %s environment variable is not set to 'true' in the head pod", workerGroup.GroupName, RAY_ENABLE_AUTOSCALER_V2)
}
}
Comment on lines +606 to +616
Copy link
Collaborator

@win5923 win5923 Dec 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if idleTimeoutSeconds != nil {
if *idleTimeoutSeconds < 0 {
return fmt.Errorf("idleTimeoutSeconds must be non-negative, got %d", *idleTimeoutSeconds)
}
// idleTimeoutSeconds only allowed on autoscaler v2
envVar, exists := EnvVarByName(RAY_ENABLE_AUTOSCALER_V2, spec.HeadGroupSpec.Template.Spec.Containers[RayContainerIndex].Env)
if !exists || (envVar.Value != "1" && envVar.Value != "true") {
return fmt.Errorf("worker group %s has idleTimeoutSeconds set, but %s environment variable is not set to 'true' in the head pod", workerGroup.GroupName, RAY_ENABLE_AUTOSCALER_V2)
}
}
if idleTimeoutSeconds != nil {
if *idleTimeoutSeconds < 0 {
return fmt.Errorf("idleTimeoutSeconds must be non-negative, got %d", *idleTimeoutSeconds)
}
// idleTimeoutSeconds only allowed on autoscaler v2
if IsAutoscalingV2Enabled(spec) {
return nil
}
envVar, exists := EnvVarByName(RAY_ENABLE_AUTOSCALER_V2, spec.HeadGroupSpec.Template.Spec.Containers[RayContainerIndex].Env)
if exists && (envVar.Value == "1" || envVar.Value == "true") {
return nil
}
return fmt.Errorf("worker group %s has idleTimeoutSeconds set, but autoscaler v2 is not enabled. Please set .spec.autoscalerOptions.version to 'v2' or set %s environment variable to 'true' in the head pod", workerGroup.GroupName, RAY_ENABLE_AUTOSCALER_V2)
}

Sorry, just realized we should also check whether the spec version is set.


return nil
}
191 changes: 191 additions & 0 deletions ray-operator/controllers/ray/utils/validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1882,3 +1882,194 @@ func TestValidateClusterUpgradeOptions(t *testing.T) {
})
}
}

func TestValidateWorkerGroupIdleTimeout(t *testing.T) {
tests := map[string]struct {
expectedErr string
spec rayv1.RayClusterSpec
}{
"should accept worker group with valid idleTimeoutSeconds": {
spec: rayv1.RayClusterSpec{
EnableInTreeAutoscaling: ptr.To(true),
HeadGroupSpec: rayv1.HeadGroupSpec{
Template: podTemplateSpec([]corev1.EnvVar{
{Name: RAY_ENABLE_AUTOSCALER_V2, Value: "1"},
}, nil),
},
WorkerGroupSpecs: []rayv1.WorkerGroupSpec{
{
GroupName: "worker-group-1",
Template: podTemplateSpec(nil, nil),
IdleTimeoutSeconds: ptr.To(int32(60)),
MinReplicas: ptr.To(int32(0)),
MaxReplicas: ptr.To(int32(10)),
},
},
},
expectedErr: "",
},
"should reject negative idleTimeoutSeconds": {
spec: rayv1.RayClusterSpec{
EnableInTreeAutoscaling: ptr.To(true),
HeadGroupSpec: rayv1.HeadGroupSpec{
Template: podTemplateSpec([]corev1.EnvVar{
{Name: RAY_ENABLE_AUTOSCALER_V2, Value: "1"},
}, nil),
},
WorkerGroupSpecs: []rayv1.WorkerGroupSpec{
{
GroupName: "worker-group-1",
Template: podTemplateSpec(nil, nil),
IdleTimeoutSeconds: ptr.To(int32(-10)),
MinReplicas: ptr.To(int32(0)),
MaxReplicas: ptr.To(int32(10)),
},
},
},
expectedErr: "idleTimeoutSeconds must be non-negative, got -10",
},
"should accept zero idleTimeoutSeconds": {
spec: rayv1.RayClusterSpec{
EnableInTreeAutoscaling: ptr.To(true),
HeadGroupSpec: rayv1.HeadGroupSpec{
Template: podTemplateSpec([]corev1.EnvVar{
{Name: RAY_ENABLE_AUTOSCALER_V2, Value: "1"},
}, nil),
},
WorkerGroupSpecs: []rayv1.WorkerGroupSpec{
{
GroupName: "worker-group-1",
Template: podTemplateSpec(nil, nil),
IdleTimeoutSeconds: ptr.To(int32(0)),
MinReplicas: ptr.To(int32(0)),
MaxReplicas: ptr.To(int32(10)),
},
},
},
expectedErr: "",
},
"should reject idleTimeoutSeconds when autoscaler version is not v2": {
spec: rayv1.RayClusterSpec{
EnableInTreeAutoscaling: ptr.To(true),
HeadGroupSpec: rayv1.HeadGroupSpec{
Template: podTemplateSpec(nil, nil),
},
WorkerGroupSpecs: []rayv1.WorkerGroupSpec{
{
GroupName: "worker-group-1",
Template: podTemplateSpec(nil, nil),
IdleTimeoutSeconds: ptr.To(int32(60)),
MinReplicas: ptr.To(int32(0)),
MaxReplicas: ptr.To(int32(10)),
},
},
},
expectedErr: "worker group worker-group-1 has idleTimeoutSeconds set, but RAY_enable_autoscaler_v2 environment variable is not set to 'true' in the head pod",
},
"should reject idleTimeoutSeconds when autoscaler version is not set": {
spec: rayv1.RayClusterSpec{
EnableInTreeAutoscaling: ptr.To(true),
HeadGroupSpec: rayv1.HeadGroupSpec{
Template: podTemplateSpec(nil, nil),
},
WorkerGroupSpecs: []rayv1.WorkerGroupSpec{
{
GroupName: "worker-group-1",
Template: podTemplateSpec(nil, nil),
IdleTimeoutSeconds: ptr.To(int32(60)),
MinReplicas: ptr.To(int32(0)),
MaxReplicas: ptr.To(int32(10)),
},
},
},
expectedErr: "worker group worker-group-1 has idleTimeoutSeconds set, but RAY_enable_autoscaler_v2 environment variable is not set to 'true' in the head pod",
},
"should reject idleTimeoutSeconds when AutoscalerOptions is nil": {
spec: rayv1.RayClusterSpec{
EnableInTreeAutoscaling: ptr.To(true),
HeadGroupSpec: rayv1.HeadGroupSpec{
Template: podTemplateSpec(nil, nil),
},
WorkerGroupSpecs: []rayv1.WorkerGroupSpec{
{
GroupName: "worker-group-1",
Template: podTemplateSpec(nil, nil),
IdleTimeoutSeconds: ptr.To(int32(60)),
MinReplicas: ptr.To(int32(0)),
MaxReplicas: ptr.To(int32(10)),
},
},
},
expectedErr: "worker group worker-group-1 has idleTimeoutSeconds set, but RAY_enable_autoscaler_v2 environment variable is not set to 'true' in the head pod",
},
"should reject idleTimeoutSeconds when env var is set to invalid value": {
spec: rayv1.RayClusterSpec{
EnableInTreeAutoscaling: ptr.To(true),
HeadGroupSpec: rayv1.HeadGroupSpec{
Template: podTemplateSpec([]corev1.EnvVar{
{Name: RAY_ENABLE_AUTOSCALER_V2, Value: "false"},
}, nil),
},
WorkerGroupSpecs: []rayv1.WorkerGroupSpec{
{
GroupName: "worker-group-1",
Template: podTemplateSpec(nil, nil),
IdleTimeoutSeconds: ptr.To(int32(60)),
MinReplicas: ptr.To(int32(0)),
MaxReplicas: ptr.To(int32(10)),
},
},
},
expectedErr: "worker group worker-group-1 has idleTimeoutSeconds set, but RAY_enable_autoscaler_v2 environment variable is not set to 'true' in the head pod",
},
"should accept worker group with idleTimeoutSeconds when env var is set to true": {
spec: rayv1.RayClusterSpec{
EnableInTreeAutoscaling: ptr.To(true),
HeadGroupSpec: rayv1.HeadGroupSpec{
Template: podTemplateSpec([]corev1.EnvVar{
{Name: RAY_ENABLE_AUTOSCALER_V2, Value: "true"},
}, nil),
},
WorkerGroupSpecs: []rayv1.WorkerGroupSpec{
{
GroupName: "worker-group-1",
Template: podTemplateSpec(nil, nil),
IdleTimeoutSeconds: ptr.To(int32(60)),
MinReplicas: ptr.To(int32(0)),
MaxReplicas: ptr.To(int32(10)),
},
},
},
expectedErr: "",
},
"should accept worker group without idleTimeoutSeconds and without autoscaler v2": {
spec: rayv1.RayClusterSpec{
EnableInTreeAutoscaling: ptr.To(true),
HeadGroupSpec: rayv1.HeadGroupSpec{
Template: podTemplateSpec(nil, nil),
},
WorkerGroupSpecs: []rayv1.WorkerGroupSpec{
{
GroupName: "worker-group-1",
Template: podTemplateSpec(nil, nil),
MinReplicas: ptr.To(int32(0)),
MaxReplicas: ptr.To(int32(10)),
},
},
},
expectedErr: "",
},
}

for name, tc := range tests {
t.Run(name, func(t *testing.T) {
err := ValidateRayClusterSpec(&tc.spec, nil)
if tc.expectedErr != "" {
require.Error(t, err)
require.EqualError(t, err, tc.expectedErr)
} else {
require.NoError(t, err)
}
})
}
}
Loading