-
Notifications
You must be signed in to change notification settings - Fork 403
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 for overwriting the generated ray start command with a user-specified container command #1704
Conversation
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.
Code and tests look good!
@@ -0,0 +1,60 @@ | |||
# This example config is used to describe how does the annotation "ray.io/overwrite-container-cmd" work. | |||
# See kuberay/docs/guidance/pod-command.md for more details. |
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.
# See kuberay/docs/guidance/pod-command.md for more details. | |
# See kuberay/docs/guidance/pod-command.md for more details. |
Is this link still accurate? More generally, what's the plan to document this feature? (I guess we'll add a section in the doc in the Ray repo?)
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.
Good catch!
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.
More generally, what's the plan to document this feature? (I guess we'll add a section in the doc in the Ray repo?)
Yes, I will update the doc later.
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.
Updated 5ad6b08.
# Because the annotation "ray.io/overwrite-container-cmd" is set to "true", | ||
# KubeRay will overwrite the generated container command with `command` and | ||
# `args` in the following. Hence, you need to specify the `ulimit` command | ||
# by yourself to avoid Ray scalability issues. |
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 think it will be helpful for the user if we remind them what $KUBERAY_GEN_RAY_START_CMD means here, even though you already described it in the code.
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.
Updated 5ad6b08.
Update doc for ray-project/kuberay#1704. --------- Signed-off-by: Kai-Hsun Chen <[email protected]> Signed-off-by: Kai-Hsun Chen <[email protected]> Co-authored-by: Archit Kulkarni <[email protected]>
Why are these changes needed?
KUBERAY_GEN_RAY_START_CMD
for both head and worker Pods to store theray start
command generated by KubeRay.ray.io/overwrite-container-cmd: "true"
to RayCluster, KubeRay will respect the container command/args provided by users.ulimit -n 65536
and["/bin/bash", "-lc", "--"]
by themselves.Related issue number
Closes #1560
Checks
ray-cluster.overwrite-command.yaml
.Command
,Args
, and environment variableKUBERAY_GEN_RAY_START_CMD
.Command
,Args
, and environment variableKUBERAY_GEN_RAY_START_CMD
.