[Feature] Add ValidateRayClusterSpec to Webhook

ray-operator/apis/ray/v1/constant.go

@@ -0,0 +1,12 @@
+package v1
+
+const (
+	// In KubeRay, the Ray container must be the first application container in a head or worker Pod.
+	RayContainerIndex = 0
+
+	// Use as container env variable
+	RAY_REDIS_ADDRESS = "RAY_REDIS_ADDRESS"
+	REDIS_PASSWORD    = "REDIS_PASSWORD"
+	// Ray GCS FT related annotations
+	RayFTEnabledAnnotationKey = "ray.io/ft-enabled"
+)

ray-operator/apis/ray/v1/pod.go

@@ -0,0 +1,8 @@
+package v1
+
+import "strings"
+
+func IsGCSFaultToleranceEnabled(instance RayCluster) bool {
+	v, ok := instance.Annotations[RayFTEnabledAnnotationKey]
+	return (ok && strings.ToLower(v) == "true") || instance.Spec.GcsFaultToleranceOptions != nil
+}

ray-operator/apis/ray/v1/pod_test.go

@@ -0,0 +1,70 @@
+package v1
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestIsGCSFaultToleranceEnabled(t *testing.T) {
+	tests := []struct {
+		name     string
+		instance RayCluster
+		expected bool
+	}{
+		{
+			name: "ray.io/ft-enabled is true",
+			instance: RayCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						RayFTEnabledAnnotationKey: "true",
+					},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "ray.io/ft-enabled is false",
+			instance: RayCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{
+						RayFTEnabledAnnotationKey: "false",
+					},
+				},
+			},
+			expected: false,
+		},
+		{
+			name: "ray.io/ft-enabled is nil, GcsFaultToleranceOptions is not nil",
+			instance: RayCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{},
+				},
+				Spec: RayClusterSpec{
+					GcsFaultToleranceOptions: &GcsFaultToleranceOptions{},
+				},
+			},
+			expected: true,
+		},
+		{
+			name: "ray.io/ft-enabled is nil, GcsFaultToleranceOptions is nil",
+			instance: RayCluster{
+				ObjectMeta: metav1.ObjectMeta{
+					Annotations: map[string]string{},
+				},
+				Spec: RayClusterSpec{
+					GcsFaultToleranceOptions: nil,
+				},
+			},
+			expected: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			result := IsGCSFaultToleranceEnabled(tt.instance)
+			assert.Equal(t, tt.expected, result)
+		})
+	}
+}

ray-operator/apis/ray/v1/raycluster_webhook.go

@@ -1,6 +1,7 @@
 package v1
 
 import (
+	"fmt"
 	"regexp"
 
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
@@ -59,6 +60,10 @@ func (r *RayCluster) validateRayCluster() error {
 		allErrs = append(allErrs, err)
 	}
 
+	if err := r.ValidateRayClusterSpec(); err != nil {
+		allErrs = append(allErrs, err)
+	}
+
 	if len(allErrs) == 0 {
 		return nil
 	}
@@ -87,3 +92,68 @@ func (r *RayCluster) validateWorkerGroups() *field.Error {
 
 	return nil
 }
+
+func (r *RayCluster) ValidateRayClusterSpec() *field.Error {
+	if len(r.Spec.HeadGroupSpec.Template.Spec.Containers) == 0 {
+		return field.Invalid(
+			field.NewPath("spec").Child("headGroupSpec").Child("template").Child("spec").Child("containers"),
+			r.Spec.HeadGroupSpec.Template.Spec.Containers,
+			"headGroupSpec should have at least one container",
+		)
+	}
+
+	for _, workerGroup := range r.Spec.WorkerGroupSpecs {
+		if len(workerGroup.Template.Spec.Containers) == 0 {
+			return field.Invalid(
+				field.NewPath("spec").Child("workerGroupSpecs"),
+				r.Spec.WorkerGroupSpecs,
+				"workerGroupSpec should have at least one container",
+			)
+		}
+	}
+
+	if r.Annotations[RayFTEnabledAnnotationKey] != "" && r.Spec.GcsFaultToleranceOptions != nil {
+		return field.Invalid(
+			field.NewPath("metadata").Child("annotations").Child(RayFTEnabledAnnotationKey),
+			r.Annotations[RayFTEnabledAnnotationKey],
+			fmt.Sprintf("%s annotation and GcsFaultToleranceOptions are both set. "+
+				"Please use only GcsFaultToleranceOptions to configure GCS fault tolerance", RayFTEnabledAnnotationKey),
+		)
+	}
+
+	if !IsGCSFaultToleranceEnabled(*r) {
+		if EnvVarExists(RAY_REDIS_ADDRESS, r.Spec.HeadGroupSpec.Template.Spec.Containers[RayContainerIndex].Env) {
+			return field.Invalid(
+				field.NewPath("spec").Child("headGroupSpec").Child("template").Child("spec").Child("containers").Index(RayContainerIndex).Child("env"),
+				r.Spec.HeadGroupSpec.Template.Spec.Containers[RayContainerIndex].Env,
+				fmt.Sprintf("%s is set which implicitly enables GCS fault tolerance, "+
+					"but GcsFaultToleranceOptions is not set. Please set GcsFaultToleranceOptions "+
+					"to enable GCS fault tolerance", RAY_REDIS_ADDRESS),
+			)
+		}
+	}
+
+	if r.Spec.GcsFaultToleranceOptions != nil {
+		if redisPassword := r.Spec.HeadGroupSpec.RayStartParams["redis-password"]; redisPassword != "" {
+			return field.Invalid(
+				field.NewPath("spec").Child("headGroupSpec").Child("rayStartParams"),
+				r.Spec.HeadGroupSpec.RayStartParams,
+				"cannot set `redis-password` in rayStartParams when GcsFaultToleranceOptions is enabled - use GcsFaultToleranceOptions.RedisPassword instead",
+			)
+		}
+
+		headContainer := r.Spec.HeadGroupSpec.Template.Spec.Containers[RayContainerIndex]
+		if EnvVarExists(REDIS_PASSWORD, headContainer.Env) {
+			return field.Invalid(
+				field.NewPath("spec").Child("headGroupSpec").Child("template").Child("spec").Child("containers").Index(RayContainerIndex).Child("env"),
+				headContainer.Env,
+				"cannot set `REDIS_PASSWORD` env var in head Pod when GcsFaultToleranceOptions is enabled - use GcsFaultToleranceOptions.RedisPassword instead",
+			)
+		}
+	}
+
+	// TODO (kevin85421): If GcsFaultToleranceOptions is set, users should use `GcsFaultToleranceOptions.RedisAddress` instead of `RAY_REDIS_ADDRESS`.
+	// TODO (kevin85421): If GcsFaultToleranceOptions is set, users should use `GcsFaultToleranceOptions.ExternalStorageNamespace` instead of
+	// the annotation `ray.io/external-storage-namespace`.
+	return nil
+}