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
alimaazamat wants to merge 2 commits into
base: master
Choose a base branch
from
+216 −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
2 commits
Select commit Hold shift + click to select a range
a608d94
add validation for idleTimeoutSeconds config per worker groups
alimaazamat Oct 30, 2025
d76a861
Check spec version then fall back to env var
alimaazamat Dec 5, 2025
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
25 changes: 25 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
}
Comment on lines +107 to +109
Copy link
Member

@Future-Outlier Future-Outlier Dec 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

  1. Can we put the logic to here?

    kuberay/ray-operator/controllers/ray/utils/validation.go

    Lines 184 to 195 in 5282a64

    if IsAutoscalingV2Enabled(spec) {
    if spec.HeadGroupSpec.Template.Spec.RestartPolicy != "" && spec.HeadGroupSpec.Template.Spec.RestartPolicy != corev1.RestartPolicyNever {
    return fmt.Errorf("restartPolicy for head Pod should be Never or unset when using autoscaler V2")
    }
    for _, workerGroup := range spec.WorkerGroupSpecs {
    if workerGroup.Template.Spec.RestartPolicy != "" && workerGroup.Template.Spec.RestartPolicy != corev1.RestartPolicyNever {
    return fmt.Errorf("restartPolicy for worker group %s should be Never or unset when using autoscaler V2", workerGroup.GroupName)
    }
    }
    }
    }

  2. Can we also add validation logic for AutoscalerOptions.IdleTimeoutSeconds?

    kuberay/ray-operator/apis/ray/v1/raycluster_types.go

    Lines 189 to 192 in 58c2aad

    // IdleTimeoutSeconds is the number of seconds to wait before scaling down a worker pod which is not using Ray resources.
    // Defaults to 60 (one minute). It is not read by the KubeRay operator but by the Ray autoscaler.
    // +optional
    IdleTimeoutSeconds *int32 `json:"idleTimeoutSeconds,omitempty"`

Copy link
Contributor Author

@alimaazamat alimaazamat Dec 9, 2025

Choose a reason for hiding this comment

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

@Future-Outlier Why would we put the validation logic in validateWorkerGroupIdleTimeout? Above it we have other validation functions like validateRayGroupLabels and validateRayGroupResources that are structured the same way: where we define those functions seperate.

Copy link
Contributor Author

@alimaazamat alimaazamat Dec 9, 2025

Choose a reason for hiding this comment

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

For validation logic for AutoscalerOptions.IdleTimeoutSeconds I agree, let's do that in a seperate PR. This one is scoped out for worker groups.

Copy link
Member

@Future-Outlier Future-Outlier Dec 10, 2025

Choose a reason for hiding this comment

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

Hi, @alimaazamat

Why would we put the validation logic in validateWorkerGroupIdleTimeout? Above it we have other validation functions like validateRayGroupLabels and validateRayGroupResources that are structured the same way: where we define those functions seperate.

great question, since I don't want to call IsAutoscalingV2Enabled twice, but your implementation make sense too.

}

if annotations[RayFTEnabledAnnotationKey] != "" && spec.GcsFaultToleranceOptions != nil {
Expand Down Expand Up @@ -596,3 +599,25 @@ 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
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 if using KubeRay < 1.4.0)", workerGroup.GroupName, RAY_ENABLE_AUTOSCALER_V2)
}

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: fmt.Sprintf("worker group worker-group-1 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 if using KubeRay < 1.4.0)", RAY_ENABLE_AUTOSCALER_V2),
},
"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: fmt.Sprintf("worker group worker-group-1 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 if using KubeRay < 1.4.0)", RAY_ENABLE_AUTOSCALER_V2),
},
"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: fmt.Sprintf("worker group worker-group-1 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 if using KubeRay < 1.4.0)", RAY_ENABLE_AUTOSCALER_V2),
},
"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: fmt.Sprintf("worker group worker-group-1 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 if using KubeRay < 1.4.0)", RAY_ENABLE_AUTOSCALER_V2),
},
"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