[Helm] Fix: inject flag-based env into ConfigMap when configuration.enabled=true

[win5923]

Why are these changes needed?

Check #4268 (comment)

Some helm values, e.g. leaderElectionEnabled and EnableMetrics, are passed as flags, see here:

kuberay/helm-chart/kuberay-operator/templates/deployment.yaml

Lines 126 to 130 in 9e8a2c0

	{{- if hasKey .Values "leaderElectionEnabled" -}}
	{{- $argList = append $argList (printf "--enable-leader-election=%t" .Values.leaderElectionEnabled) -}}
	{{- end -}}
	{{- if and (hasKey .Values "metrics") (hasKey .Values.metrics "enabled") }}
	{{- $argList = append $argList (printf "--enable-metrics=%t" .Values.metrics.enabled) -}}

Then these flags get ignored when configuration.enabled=true, see here:

kuberay/ray-operator/main.go

Lines 117 to 141 in 9e8a2c0

	if configFile != "" {
	var err error
	configData, err := os.ReadFile(configFile)
	exitOnError(err, "failed to read config file")
	config, err = decodeConfig(configData, scheme)
	exitOnError(err, "failed to decode config file")
	} else {
	config.MetricsAddr = metricsAddr
	config.ProbeAddr = probeAddr
	config.EnableLeaderElection = &enableLeaderElection
	config.LeaderElectionNamespace = leaderElectionNamespace
	config.ReconcileConcurrency = reconcileConcurrency
	config.WatchNamespace = watchNamespace
	config.LogFile = logFile
	config.LogFileEncoder = logFileEncoder
	config.LogStdoutEncoder = logStdoutEncoder
	config.EnableBatchScheduler = enableBatchScheduler
	config.BatchScheduler = batchScheduler
	config.UseKubernetesProxy = useKubernetesProxy
	config.DeleteRayJobAfterJobFinishes = os.Getenv(utils.DELETE_RAYJOB_CR_AFTER_JOB_FINISHES) == "true"
	config.EnableMetrics = enableMetrics
	config.QPS = &qps
	config.Burst = &burst
	}

kuberay/helm-chart/kuberay-operator/values.yaml

Lines 104 to 107 in 9e8a2c0

	# Configuration for the KubeRay operator.
	configuration:
	# -- Whether to enable the configuration feature. If enabled, a ConfigMap will be created and mounted to the operator.
	enabled: false

kuberay/helm-chart/kuberay-operator/templates/deployment.yaml

Lines 86 to 91 in 9e8a2c0

	args:
	{{- $argList := list -}}
	{{- if .Values.configuration.enabled -}}
	{{- $argList = append $argList "--config" -}}
	{{- $argList = append $argList "/etc/kuberay/config.yaml" -}}
	{{- end -}}

kuberay/helm-chart/kuberay-operator/templates/configmap.yaml

Lines 1 to 20 in 9e8a2c0

	{{- if .Values.configuration.enabled }}
	apiVersion: v1
	kind: ConfigMap
	metadata:
	name: {{ include "kuberay-operator.deployment.name" . }}-config
	namespace: {{ .Release.Namespace }}
	labels:
	{{- include "kuberay-operator.labels" . | nindent 4 }}
	{{- with .Values.labels }}
	{{- toYaml . | nindent 4 }}
	{{- end }}
	data:
	config.yaml: |
	apiVersion: config.ray.io/v1alpha1
	kind: Configuration
	{{- if .Values.configuration.defaultContainerEnvs }}
	defaultContainerEnvs:
	{{- toYaml .Values.configuration.defaultContainerEnvs | nindent 4 }}
	{{- end }}
	{{- end }}

This PR includes all flag-based configuration options in the ConfigMap template, ensuring they are properly read from helm values and injected into the ConfigMap.

• When configuration.enabled = true, the KubeRay Operator would read all flag-based envs from the ConfigMap, and these values would take precedence over the flag-based configuration.

• When configuration.enabled = false and the relevant flag-based options are explicitly set in values.yaml (e.g. metrics.enabled = true), the operator would preserving the current flag-based behavior.

Related issue number

Closes #4268

Checks

• I've made sure the tests are passing.
• Testing Strategy
  • Unit tests
  • Manual tests
  • This PR is not tested :(

[seanlaii]

Thanks! Overall this looks great to me.
Aside from the comment below, I have one minor suggestion: should we consider avoiding setting this flag?

[win5923]

I have one minor suggestion: should we consider avoiding setting this flag?

I think both are fine, This flag is primarily designed for complex structures that cannot be easily passed via command-line flags (like defaultContainerEnvs, headSidecarContainers).

[seanlaii]

I think both are fine, This flag is primarily designed for complex structures that cannot be easily passed via command-line flags (like defaultContainerEnvs, headSidecarContainers).

Sorry for the confusion. What I mean is should we consider avoiding setting the command line flags when they enable the config map mode?
IIUC, the command line flags will still be set when enabling the config map mode.
Maybe the users will be confused when they still see the command line flags while using the config map mode.

[win5923]

Sorry for the confusion. What I mean is should we consider avoiding setting the command line flags when they enable the config map mode?
IIUC, the command line flags will still be set when enabling the config map mode.
Maybe the users will be confused when they still see the command line flags while using the config map mode.

Oh, Thanks for catching this. I think we should consider avoiding setting the command line flags when they enable the config map mode.

[seanlaii]

Thanks! To confirm, featureGates is not supported in config map, is it correct?

[win5923]

yes

[seanlaii]

Thanks! LGTM!