-
Notifications
You must be signed in to change notification settings - Fork 631
[Feature] Support inject specific env vars to all Ray containers in all RayCluster CRs by ConfigMap #4103
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feature] Support inject specific env vars to all Ray containers in all RayCluster CRs by ConfigMap #4103
Conversation
bb9068f
to
238677a
Compare
…uster CRs Signed-off-by: win5923 <[email protected]>
238677a
to
39c4813
Compare
442f118
to
77048d8
Compare
Hi @kevin85421, PTAL when you have time, thanks! |
77048d8
to
bacdedd
Compare
Signed-off-by: win5923 <[email protected]>
bacdedd
to
3bfb280
Compare
@@ -0,0 +1,15 @@ | |||
{{- if .Values.defaultRayEnvs }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This check could be a problem if we want to add more fields to the configuration in the future. Is it possible that we always generate the configmap?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice suggestion! I've updated to use configuration.enabled
to control whether the ConfigMap should be created.
55852fd
also cc @andrewsykim for review because you implemented the config API. |
Signed-off-by: win5923 <[email protected]>
070b4f7
to
55852fd
Compare
| batchScheduler.enabled | bool | `false` | | | ||
| batchScheduler.name | string | `""` | | | ||
| configuration.enabled | bool | `false` | Whether to enable the configuration feature. If enabled, a ConfigMap will be created and mounted to the operator. | | ||
| configuration.defaultRayEnvs | list | `[]` | Default environment variables to inject into all Ray containers in all RayCluster CRs. This allows user to set feature flags across all Ray pods. Example: defaultRayEnvs: - name: RAY_enable_open_telemetry value: "true" - name: RAY_metric_cardinality_level value: "recommended" | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like RayEnvs is a bit confusing. The envs themselves can be unrelated to Ray. Maybe defaultContainerEnvs is better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change to defaultContainerEnvs. Thanks! 8cf8ba3
Signed-off-by: win5923 <[email protected]>
Why are these changes needed?
Added defaultRayEnvs field to the Configuration API, allowing user to inject environment variables into all Ray containers across all RayCluster CRs.
Also update kuberay-operator helm chart, now generates a
ConfigMap
with the Configuration YAML and mounts it to the operator whendefaultRayEnvs
is set invalues.yaml
.E2E
ray:2.49.0
head pod:


worker pod:
The

WorkerId
tag has been removed for metrics such as ray_tasks.Related issue number
Closes #4098
Checks