Fix comments

Signed-off-by: Ryan O'Leary <ryanaoleary@google.com>

Author: ryanaoleary

docs/reference/api.md

@@ -138,6 +138,7 @@ _Appears in:_
 | `template` _[PodTemplateSpec](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#podtemplatespec-v1-core)_ | Template is the exact pod template used in K8s deployments, statefulsets, etc. |  |  |
 | `headService` _[Service](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#service-v1-core)_ | HeadService is the Kubernetes service of the head pod. |  |  |
 | `enableIngress` _boolean_ | EnableIngress indicates whether operator should create ingress object for head service or not. |  |  |
+| `resources` _object (keys:string, values:string)_ | Resources specifies the resource quantities for the head group.<br />These values override the resources passed to `rayStartParams` for the group, but<br />have no effect on the resources set at the K8s Pod container level. |  |  |
 | `labels` _object (keys:string, values:string)_ | Labels specifies the Ray node labels for the head group.<br />These labels will also be added to the Pods of this head group and override the `--labels`<br />argument passed to `rayStartParams`. |  |  |
 | `rayStartParams` _object (keys:string, values:string)_ | RayStartParams are the params of the start command: node-manager-port, object-store-memory, ... |  |  |
 | `serviceType` _[ServiceType](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#servicetype-v1-core)_ | ServiceType is Kubernetes service type of the head service. it will be used by the workers to connect to the head pod |  |  |
@@ -418,6 +419,7 @@ _Appears in:_
 | `minReplicas` _integer_ | MinReplicas denotes the minimum number of desired Pods for this worker group. | 0 |  |
 | `maxReplicas` _integer_ | MaxReplicas denotes the maximum number of desired Pods for this worker group, and the default value is maxInt32. | 2147483647 |  |
 | `idleTimeoutSeconds` _integer_ | IdleTimeoutSeconds denotes the number of seconds to wait before the v2 autoscaler terminates an idle worker pod of this type.<br />This value is only used with the Ray Autoscaler enabled and defaults to the value set by the AutoscalingConfig if not specified for this worker group. |  |  |
+| `resources` _object (keys:string, values:string)_ | Resources specifies the resource quantities for this worker group.<br />These values override the resources passed to `rayStartParams` for the group, but<br />have no effect on the resources set at the K8s Pod container level. |  |  |
 | `labels` _object (keys:string, values:string)_ | Labels specifies the Ray node labels for this worker group.<br />These labels will also be added to the Pods of this worker group and override the `--labels`<br />argument passed to `rayStartParams`. |  |  |
 | `rayStartParams` _object (keys:string, values:string)_ | RayStartParams are the params of the start command: address, object-store-memory, ... |  |  |
 | `template` _[PodTemplateSpec](https://kubernetes.io/docs/reference/generated/kubernetes-api/v1.28/#podtemplatespec-v1-core)_ | Template is a pod template for the worker |  |  |

helm-chart/kuberay-operator/crds/ray.io_rayclusters.yaml

@@ -76,9 +76,10 @@ type HeadGroupSpec struct {
 	// +optional
 	EnableIngress *bool `json:"enableIngress,omitempty"`
 	// Resources specifies the resource quantities for the head group.
-	// These values override the resources passed to `rayStartParams` for the group.
+	// These values override the resources passed to `rayStartParams` for the group, but
+	// have no effect on the resources set at the K8s Pod container level.
 	// +optional
-	Resources corev1.ResourceList `json:"resources,omitempty"`
+	Resources map[string]string `json:"resources,omitempty"`
 	// Labels specifies the Ray node labels for the head group.
 	// These labels will also be added to the Pods of this head group and override the `--labels`
 	// argument passed to `rayStartParams`.
@@ -116,9 +117,10 @@ type WorkerGroupSpec struct {
 	// +optional
 	IdleTimeoutSeconds *int32 `json:"idleTimeoutSeconds,omitempty"`
 	// Resources specifies the resource quantities for this worker group.
-	// These values override the resources passed to `rayStartParams` for the group.
+	// These values override the resources passed to `rayStartParams` for the group, but
+	// have no effect on the resources set at the K8s Pod container level.
 	// +optional
-	Resources corev1.ResourceList `json:"resources,omitempty"`
+	Resources map[string]string `json:"resources,omitempty"`
 	// Labels specifies the Ray node labels for this worker group.
 	// These labels will also be added to the Pods of this worker group and override the `--labels`
 	// argument passed to `rayStartParams`.

helm-chart/kuberay-operator/crds/ray.io_rayjobs.yaml

@@ -172,11 +172,11 @@ func DefaultHeadPodTemplate(ctx context.Context, instance rayv1.RayCluster, head
 	// This ensures privilege of KubeRay users are contained within the namespace of the RayCluster.
 	podTemplate.ObjectMeta.Namespace = instance.Namespace
 
-	// Reconcile top-level Resources for head group with rayStartParams.
-	reconcileRayStartParamsResources(headSpec.RayStartParams, headSpec.Resources)
+	// Update rayStartParams with top-level Resources for head group.
+	updateRayStartParamsResources(ctx, headSpec.RayStartParams, headSpec.Resources)
 
-	// Reconcile top-level Labels for head group with `--labels` in rayStartParams.
-	reconcileRayStartParamsLabels(headSpec.RayStartParams, headSpec.Labels)
+	// Update --labels` in rayStartParams with top-level Labels for head group.
+	updateRayStartParamsLabels(headSpec.RayStartParams, headSpec.Labels)
 
 	// Merge K8s labels from the Pod template and the top-level `Labels` field.
 	mergedLabels := mergeLabels(headSpec.Template.ObjectMeta.Labels, headSpec.Labels)
@@ -318,11 +318,11 @@ func DefaultWorkerPodTemplate(ctx context.Context, instance rayv1.RayCluster, wo
 	// Hence, we set `ObjectMeta.Name` to an empty string, and use GenerateName to prevent name conflicts.
 	podTemplate.ObjectMeta.Name = ""
 
-	// Reconcile top-level Resources for worker group with rayStartParams.
-	reconcileRayStartParamsResources(workerSpec.RayStartParams, workerSpec.Resources)
+	// Update rayStartParams with top-level Resources for worker group.
+	updateRayStartParamsResources(ctx, workerSpec.RayStartParams, workerSpec.Resources)
 
-	// Reconcile top-level Labels for worker group with `--labels` in rayStartParams.
-	reconcileRayStartParamsLabels(workerSpec.RayStartParams, workerSpec.Labels)
+	// Update --labels` in rayStartParams with top-level Labels for worker group.
+	updateRayStartParamsLabels(workerSpec.RayStartParams, workerSpec.Labels)
 
 	// Merge K8s labels from the Pod template and the top-level `Labels` field.
 	mergedLabels := mergeLabels(workerSpec.Template.ObjectMeta.Labels, workerSpec.Labels)
@@ -1040,58 +1040,71 @@ func findMemoryReqOrLimit(container corev1.Container) (res *resource.Quantity) {
 	return nil
 }
 
-// reconcileRayStartParamsLabels reconciles `--labels` in rayStartParams based on group `Labels`.
-func reconcileRayStartParamsLabels(rayStartParams map[string]string, groupLabels map[string]string) {
-	if len(groupLabels) > 0 {
-		var labels []string
-		// Sort label keys for deterministic output.
-		keys := make([]string, 0, len(groupLabels))
-		for k := range groupLabels {
-			keys = append(keys, k)
-		}
-		sort.Strings(keys)
+// updateRayStartParamsLabels reconciles `--labels` in rayStartParams based on group `Labels`.
+func updateRayStartParamsLabels(rayStartParams map[string]string, groupLabels map[string]string) {
+	if len(groupLabels) == 0 {
+		return
+	}
+	var labels []string
+	// Sort label keys for deterministic output.
+	keys := make([]string, 0, len(groupLabels))
+	for k := range groupLabels {
+		keys = append(keys, k)
+	}
+	sort.Strings(keys)
 
-		for _, k := range keys {
-			labels = append(labels, fmt.Sprintf("%s=%s", k, groupLabels[k]))
-		}
-		// When provided this will override any `--labels` value specified in rayStartParams.
-		rayStartParams["labels"] = strings.Join(labels, ",")
+	for _, k := range keys {
+		labels = append(labels, fmt.Sprintf("%s=%s", k, groupLabels[k]))
 	}
+	// When provided this will override any `--labels` value specified in rayStartParams.
+	rayStartParams["labels"] = strings.Join(labels, ",")
 }
 
-// reconcileRayStartParamsResources reconciles rayStartParams based on the top-level `Resources` field.
-func reconcileRayStartParamsResources(rayStartParams map[string]string, topLevelResources corev1.ResourceList) {
-	if len(topLevelResources) > 0 {
-		// Override relevant rayStartParams fields to ensure consistency.
-		rayResourcesJson := make(map[string]float64)
-		for name, quantity := range topLevelResources {
-			strName := string(name)
-			if name == corev1.ResourceCPU {
-				rayStartParams["num-cpus"] = strconv.FormatInt(quantity.Value(), 10)
-			} else if name == corev1.ResourceMemory {
-				rayStartParams["memory"] = strconv.FormatInt(quantity.Value(), 10)
-			} else if utils.IsGPUResourceKey(strName) {
-				rayStartParams["num-gpus"] = strconv.FormatInt(quantity.Value(), 10)
-			} else {
-				rayResourcesJson[strName] = quantity.AsApproximateFloat64()
-			}
+// updateRayStartParamsResources reconciles rayStartParams based on the top-level `Resources` field.
+func updateRayStartParamsResources(ctx context.Context, rayStartParams map[string]string, groupResources map[string]string) {
+	log := ctrl.LoggerFrom(ctx)
+
+	if len(groupResources) == 0 {
+		return
+	}
+	// Override relevant rayStartParams fields to ensure consistency.
+	rayResourcesJson := make(map[string]float64)
+	for name, quantity := range groupResources {
+		q, err := resource.ParseQuantity(quantity)
+		if err != nil {
+			log.Info("Skipping resource %s: failed to parse quantity '%s': %v", name, quantity, err)
+			continue
 		}
 
-		if len(rayResourcesJson) > 0 {
-			jsonBytes, _ := json.Marshal(rayResourcesJson)
-			rayStartParams["resources"] = fmt.Sprintf("'%s'", string(jsonBytes))
+		if name == string(corev1.ResourceCPU) {
+			rayStartParams["num-cpus"] = strconv.FormatInt(q.Value(), 10)
+		} else if name == string(corev1.ResourceMemory) {
+			rayStartParams["memory"] = strconv.FormatInt(q.Value(), 10)
+		} else if utils.IsGPUResourceKey(name) {
+			rayStartParams["num-gpus"] = strconv.FormatInt(q.Value(), 10)
+		} else {
+			rayResourcesJson[name] = q.AsApproximateFloat64()
+		}
+	}
+
+	if len(rayResourcesJson) > 0 {
+		jsonBytes, err := json.Marshal(rayResourcesJson)
+		if err != nil {
+			log.Error(err, "Failed to marshal Ray Resources JSON for rayStartParams.")
+			return
 		}
+		rayStartParams["resources"] = fmt.Sprintf("'%s'", string(jsonBytes))
 	}
 }
 
-// mergeLabels combines labels from a pod template and a top-level `labels` spec,
+// mergeLabels combines labels from a pod template and a group `labels` spec,
 // with the top-level labels field taking precedence.
-func mergeLabels(templateLabels map[string]string, topLevelLabels map[string]string) map[string]string {
+func mergeLabels(templateLabels map[string]string, groupLabels map[string]string) map[string]string {
 	merged := make(map[string]string)
 	for k, v := range templateLabels {
 		merged[k] = v
 	}
-	for k, v := range topLevelLabels {
+	for k, v := range groupLabels {
 		merged[k] = v
 	}
 	return merged