-
Notifications
You must be signed in to change notification settings - Fork 364
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
[autoscaler] Make log file mount path more specific. #391
[autoscaler] Make log file mount path more specific. #391
Conversation
Signed-off-by: Dmitri Gekhtman <dmitri.m.gekhtman@gmail.com>
Extra paranoia --- this Ray PR should also solve the issue: ray-project/ray#26748 |
Oh I need to fix the test... |
There was an update to the RayJobs CRD when I ran |
I added some comments to the job CRD and forgot to run |
that's great! |
Why are these changes needed? Together with ray-project/kuberay#391, this should address #26064 Have the autoscaler try to make the Ray log directory before setting up logging. ray-project/kuberay#391 should be good enough for this, but this PR makes things safer in case the KubeRay user overrides log mounts or something like that.
Why are these changes needed? Together with ray-project/kuberay#391, this should address ray-project#26064 Have the autoscaler try to make the Ray log directory before setting up logging. ray-project/kuberay#391 should be good enough for this, but this PR makes things safer in case the KubeRay user overrides log mounts or something like that. Signed-off-by: Rohan138 <rapotdar@purdue.edu>
Why are these changes needed? Together with ray-project/kuberay#391, this should address ray-project#26064 Have the autoscaler try to make the Ray log directory before setting up logging. ray-project/kuberay#391 should be good enough for this, but this PR makes things safer in case the KubeRay user overrides log mounts or something like that. Signed-off-by: Stefan van der Kleij <s.vanderkleij@viroteq.com>
* Fix log path. Signed-off-by: Dmitri Gekhtman <dmitri.m.gekhtman@gmail.com> * Rerun CI. * Fix test. Signed-off-by: Dmitri Gekhtman <dmitri.m.gekhtman@gmail.com> * Rerun CI.
Signed-off-by: Dmitri Gekhtman dmitri.m.gekhtman@gmail.com
Why are these changes needed?
Should resolve ray-project/ray#26064, where users report the autoscaler crash-looping due to a missing log directory.
Gives the shared log mount configured by the KubeRay operator for Ray logs a more specific path.
(This log mount was introduced in #274)
Previously the path was
/tmp/ray
. Instead, we should use/tmp/ray/session_latest/logs
, which is where Ray and the autoscaler would like to write logs.By using the more specific path, we ensure that the path will exist on container startup, preventing the file-not-found issues described in ray-project/ray#26064.
Ray attempts to create this directory with
os.makedirs(path, exist_ok=True)
, so this should not interfere with Ray. (Obviously, this will be confirmed by the tests.)To be extra safe, I'll go ahead and make the autoscaler also
os.makedirs(path, exist_ok=True)
: ray-project/ray#26748Related issue number
Checks