[RayCluster] Toggle usage of deterministic/non-deterministic head pod name with feature flag (#3873)

* feat: use gen name for ray version > 2.48

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

* feat: head pod name to non deterministic

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

* test: compare head pod name between old and new

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

* feat: add feature flag for deterministic head pod name

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

* feat: use deterministic if feature flag set to true

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

* test: head pod name with/without feature flag == true

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

* test: sep test for head/worker pod name

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

* docs: add feature flag desc to helm chart

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

* fix: feature flag name

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

* refactor: simplify check if the feature flag is set

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

---------

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

Author: machichima

helm-chart/kuberay-operator/values.yaml

@@ -176,6 +176,9 @@ env:
 # If set to true, the RayJob CR itself will be deleted if shutdownAfterJobFinishes is set to true. Note that all resources created by the RayJob CR will be deleted, including the K8s Job. Otherwise, only the RayCluster CR will be deleted. Default is false.
 # - name: DELETE_RAYJOB_CR_AFTER_JOB_FINISHES
 #   value: "false"
+# If set to true, we will use deterministic name for head pod. Otherwise, the non-deterministic name is used.
+# - name: ENABLE_DETERMINISTIC_HEAD_POD_NAME
+#   value: "false"
 
 # -- Resource requests and limits for containers.
 resources:

ray-operator/controllers/ray/common/pod.go

@@ -164,7 +164,11 @@ func DefaultHeadPodTemplate(ctx context.Context, instance rayv1.RayCluster, head
 	// headPort is passed into setMissingRayStartParams but unused there for the head pod.
 	// To mitigate this awkwardness and reduce code redundancy, unify head and worker pod configuration logic.
 	podTemplate := headSpec.Template
-	podTemplate.GenerateName = podName
+	if utils.IsDeterministicHeadPodNameEnabled() {
+		podTemplate.Name = podName
+	} else {
+		podTemplate.GenerateName = podName
+	}
 	// Pods created by RayCluster should be restricted to the namespace of the RayCluster.
 	// This ensures privilege of KubeRay users are contained within the namespace of the RayCluster.
 	podTemplate.ObjectMeta.Namespace = instance.Namespace

ray-operator/controllers/ray/raycluster_controller.go

@@ -980,7 +980,7 @@ func (r *RayClusterReconciler) createWorkerPod(ctx context.Context, instance ray
 // Build head instance pod(s).
 func (r *RayClusterReconciler) buildHeadPod(ctx context.Context, instance rayv1.RayCluster) corev1.Pod {
 	logger := ctrl.LoggerFrom(ctx)
-	podName := utils.PodName(instance.Name, rayv1.HeadNode, true)
+	podName := utils.PodName(instance.Name, rayv1.HeadNode, !utils.IsDeterministicHeadPodNameEnabled())
 	fqdnRayIP := utils.GenerateFQDNServiceName(ctx, instance, instance.Namespace) // Fully Qualified Domain Name
 
 	// The Ray head port used by workers to connect to the cluster (GCS server port for Ray >= 1.11.0, Redis port for older Ray.)

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

@@ -164,6 +164,9 @@ const (
 	// by default starting with v1.4.0.
 	ENABLE_LOGIN_SHELL = "ENABLE_LOGIN_SHELL"
 
+	// If set to true, we will use deterministic name for head pod. Otherwise, the non-deterministic name is used.
+	ENABLE_DETERMINISTIC_HEAD_POD_NAME = "ENABLE_DETERMINISTIC_HEAD_POD_NAME"
+
 	// Ray core default configurations
 	DefaultWorkerRayGcsReconnectTimeoutS = "600"
 

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

@@ -712,3 +712,8 @@ func GetContainerCommand(additionalOptions []string) []string {
 	bashOptionsStr := strings.Join(bashOptions, "")
 	return []string{"/bin/bash", "-" + bashOptionsStr, "--"}
 }
+
+// GetEnableDeterministicHeadName returns true if deterministic head pod name is enabled.
+func IsDeterministicHeadPodNameEnabled() bool {
+	return strings.ToLower(os.Getenv(ENABLE_DETERMINISTIC_HEAD_POD_NAME)) == "true"
+}

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

@@ -104,42 +104,97 @@ func TestCheckAllPodsRunning(t *testing.T) {
 	}
 }
 
-func TestPodName(t *testing.T) {
+func TestWorkerPodName(t *testing.T) {
 	tests := []struct {
 		name     string
 		prefix   string
-		nodeType rayv1.RayNodeType
 		expected string
 	}{
-		{
-			name:     "short cluster name, head pod",
-			prefix:   "ray-cluster-01",
-			nodeType: rayv1.HeadNode,
-			expected: "ray-cluster-01-head-",
-		},
 		{
 			name:     "short cluster name, worker pod",
 			prefix:   "ray-cluster-group-name-01",
-			nodeType: rayv1.WorkerNode,
 			expected: "ray-cluster-group-name-01-worker-",
 		},
-		{
-			name:     "long cluster name, head pod",
-			prefix:   "ray-cluster-0000000000000000000000011111111122222233333333333333",
-			nodeType: rayv1.HeadNode,
-			expected: "ray-cluster-00000000000000000000000111111111222222-head-",
-		},
 		{
 			name:     "long cluster name, worker pod",
 			prefix:   "ray-cluster-0000000000000000000000011111111122222233333333333333-group-name",
-			nodeType: rayv1.WorkerNode,
 			expected: "ray-cluster-00000000000000000000000111111111222222-worker-",
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			str := PodName(test.prefix, test.nodeType, true)
+			str := PodName(test.prefix, rayv1.WorkerNode, true)
+			if str != test.expected {
+				t.Logf("expected: %q", test.expected)
+				t.Logf("actual: %q", str)
+				t.Error("PodName returned an unexpected string")
+			}
+
+			// 63 (max pod name length) - 5 random hexadecimal characters from generateName
+			if len(str) > 58 {
+				t.Error("Generated pod name is too long")
+			}
+		})
+	}
+}
+
+func TestHeadPodName(t *testing.T) {
+	defer os.Unsetenv(ENABLE_DETERMINISTIC_HEAD_POD_NAME)
+
+	tests := []struct {
+		name                       string
+		prefix                     string
+		enableDeterministicHeadPod string
+		expected                   string
+	}{
+		{
+			name:                       "short cluster name, deterministic head pod name",
+			prefix:                     "ray-cluster-01",
+			enableDeterministicHeadPod: "true",
+			expected:                   "ray-cluster-01-head",
+		},
+		{
+			name:                       "short cluster name, non-deterministic head pod name",
+			prefix:                     "ray-cluster-01",
+			enableDeterministicHeadPod: "false",
+			expected:                   "ray-cluster-01-head-",
+		},
+		{
+			name:                       "short cluster name, feature flag not set",
+			prefix:                     "ray-cluster-01",
+			enableDeterministicHeadPod: "unset",
+			expected:                   "ray-cluster-01-head-",
+		},
+		{
+			name:                       "long cluster name, deterministic head pod name",
+			prefix:                     "ray-cluster-0000000000000000000000011111111122222233333333333333",
+			enableDeterministicHeadPod: "true",
+			expected:                   "ray-cluster-00000000000000000000000111111111222222-head",
+		},
+		{
+			name:                       "long cluster name, non-deterministic head pod name",
+			prefix:                     "ray-cluster-0000000000000000000000011111111122222233333333333333",
+			enableDeterministicHeadPod: "false",
+			expected:                   "ray-cluster-00000000000000000000000111111111222222-head-",
+		},
+		{
+			name:                       "long cluster name, feature flag not set",
+			prefix:                     "ray-cluster-0000000000000000000000011111111122222233333333333333",
+			enableDeterministicHeadPod: "unset",
+			expected:                   "ray-cluster-00000000000000000000000111111111222222-head-",
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.name, func(t *testing.T) {
+			if test.enableDeterministicHeadPod == "unset" {
+				os.Unsetenv(ENABLE_DETERMINISTIC_HEAD_POD_NAME)
+			} else {
+				os.Setenv(ENABLE_DETERMINISTIC_HEAD_POD_NAME, test.enableDeterministicHeadPod)
+			}
+
+			str := PodName(test.prefix, rayv1.HeadNode, !IsDeterministicHeadPodNameEnabled())
 			if str != test.expected {
 				t.Logf("expected: %q", test.expected)
 				t.Logf("actual: %q", str)