Skip to content

test: unit tests for idleTimeoutSeconds config per worker groups #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 2 commits into
base: master
Choose a base branch
from
+181 −0
Open

test: unit tests for idleTimeoutSeconds config per worker groups #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
ae88086
test: add unit tests for idleTimeoutSeconds config per worker groups
alimaazamat Oct 30, 2025
abc35ba
autoscaler part2 e2e tests should set version in spec
alimaazamat Nov 13, 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
18 changes: 18 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 @@ -103,6 +103,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 @@ -574,3 +577,18 @@ 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 && *idleTimeoutSeconds < 0 {
return fmt.Errorf("idleTimeoutSeconds must be non-negative, got %d", *idleTimeoutSeconds)
}

// idleTimeoutSeconds only allowed on autoscaler v2
if idleTimeoutSeconds != nil && !IsAutoscalingV2Enabled(spec) {
return fmt.Errorf("worker group %s has idleTimeoutSeconds set, but autoscaler version is not v2. Please set .spec.autoscalerOptions.version to v2", workerGroup.GroupName)
}

return nil
}
161 changes: 161 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 @@ -1823,3 +1823,164 @@ 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),
AutoscalerOptions: &rayv1.AutoscalerOptions{
Version: ptr.To(rayv1.AutoscalerVersionV2),
},
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: "",
},
"should reject negative idleTimeoutSeconds": {
spec: rayv1.RayClusterSpec{
EnableInTreeAutoscaling: ptr.To(true),
AutoscalerOptions: &rayv1.AutoscalerOptions{
Version: ptr.To(rayv1.AutoscalerVersionV2),
},
HeadGroupSpec: rayv1.HeadGroupSpec{
Template: podTemplateSpec(nil, 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),
AutoscalerOptions: &rayv1.AutoscalerOptions{
Version: ptr.To(rayv1.AutoscalerVersionV2),
},
HeadGroupSpec: rayv1.HeadGroupSpec{
Template: podTemplateSpec(nil, 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),
AutoscalerOptions: &rayv1.AutoscalerOptions{
Version: ptr.To(rayv1.AutoscalerVersionV1),
},
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 autoscaler version is not v2. Please set .spec.autoscalerOptions.version to 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: "worker group worker-group-1 has idleTimeoutSeconds set, but autoscaler version is not v2. Please set .spec.autoscalerOptions.version to v2",
},
"should reject idleTimeoutSeconds when AutoscalerOptions is nil": {
spec: rayv1.RayClusterSpec{
EnableInTreeAutoscaling: ptr.To(true),
AutoscalerOptions: nil,
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 autoscaler version is not v2. Please set .spec.autoscalerOptions.version to v2",
},
"should accept worker group without idleTimeoutSeconds and without autoscaler v2": {
spec: rayv1.RayClusterSpec{
EnableInTreeAutoscaling: ptr.To(true),
AutoscalerOptions: &rayv1.AutoscalerOptions{
Version: ptr.To(rayv1.AutoscalerVersionV1),
},
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)
}
})
}
}
2 changes: 2 additions & 0 deletions ray-operator/test/e2eautoscaler/raycluster_autoscaler_part2_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ func TestRayClusterAutoscalerV2IdleTimeout(t *testing.T) {
rayClusterSpecAC := rayv1ac.RayClusterSpec().
WithEnableInTreeAutoscaling(true).
WithRayVersion(GetRayVersion()).
WithAutoscalerOptions(rayv1ac.AutoscalerOptions().
WithVersion(rayv1.AutoscalerVersionV2)).
Comment on lines +45 to +46
Copy link
Collaborator

@rueian rueian Dec 1, 2025

Choose a reason for hiding this comment

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

Suggested change
WithAutoscalerOptions(rayv1ac.AutoscalerOptions().
WithVersion(rayv1.AutoscalerVersionV2)).

Are these required? The tests[1] used here already has autoscaler v2 enabled.

Copy link
Contributor Author

@alimaazamat alimaazamat Dec 2, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Even thought tests[1] has v2 enabled as an env variable, but doesn't actually change the spec without

WithAutoscalerOptions(rayv1ac.AutoscalerOptions().
		WithVersion(rayv1.AutoscalerVersionV2)).

I wanted it updated in the spec as well for the validation tests I wrote, since in validation.go the other tests reference the spec not env vars e.g. workerGroup.Template.Spec.RestartPolicy

Copy link
Contributor Author

@alimaazamat alimaazamat Dec 2, 2025

Choose a reason for hiding this comment

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

https://github.com/alimaazamat/kuberay/blob/idleTimeoutSecondsTests/ray-operator/controllers/ray/utils/validation.go#L180 "Please only use the former" spec version not env var

WithHeadGroupSpec(rayv1ac.HeadGroupSpec().
WithRayStartParams(map[string]string{"num-cpus": "0"}).
WithTemplate(tc.HeadPodTemplateGetter())).
Expand Down
Loading