add v2 autoscaler validation

Signed-off-by: alimaazamat <[email protected]>

Author: alimaazamat

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

@@ -103,7 +103,7 @@ 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); err != nil {
+		if err := validateWorkerGroupIdleTimeout(workerGroup, spec); err != nil {
 			return err
 		}
 	}
@@ -579,11 +579,16 @@ func validateLegacyDeletionPolicies(rayJob *rayv1.RayJob) error {
 }
 
 // validateWorkerGroupIdleTimeout validates the idleTimeoutSeconds field in a worker group spec
-func validateWorkerGroupIdleTimeout(workerGroup rayv1.WorkerGroupSpec) error {
+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
 }

ray-operator/controllers/ray/utils/validation_test.go

@@ -1892,21 +1892,94 @@ func TestValidateWorkerGroupIdleTimeout(t *testing.T) {
 			},
 			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 testName, tc := range tests {
-		t.Run(testName, func(t *testing.T) {
+	for name, tc := range tests {
+		t.Run(name, func(t *testing.T) {
 			err := ValidateRayClusterSpec(&tc.spec, nil)
-			if tc.expectedErr == "" {
-				if err != nil {
-					t.Errorf("expected no error, but got: %v", err)
-				}
+			if tc.expectedErr != "" {
+				require.Error(t, err)
+				require.EqualError(t, err, tc.expectedErr)
 			} else {
-				if err == nil {
-					t.Errorf("expected error: %s, but got no error", tc.expectedErr)
-				} else if err.Error() != tc.expectedErr {
-					t.Errorf("expected error: %s, but got: %v", tc.expectedErr, err)
-				}
+				require.NoError(t, err)
 			}
 		})
 	}