-
Notifications
You must be signed in to change notification settings - Fork 44
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
Split head memory and cpu requests/limits #579
Split head memory and cpu requests/limits #579
Conversation
96e3a8e
to
7ccd52d
Compare
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.
There is one thing to note, perhaps unrelated to this PR, and is that a user can basically input ANY value in the ClusterConfiguration parameters.
I.e., I can set to head_cpu_requests=True
, including head_gpus=True
, and that is reflected in the yaml as a bool. I believe this is not the expected behaviour. - Note that this was tested on KinD as my OpenShift cluster isn't working at the moment.
@ChristianZaccaria |
@Bobbins228 I couldn't get further, but I suppose maybe |
@ChristianZaccaria This is insane! It seems you can pretty much set any of the variables to whatever type you like. |
7ccd52d
to
49df838
Compare
Applied do not merge label until RHOAIENG-9259 is a priority again. |
49df838
to
2db9016
Compare
2db9016
to
38b9022
Compare
38b9022
to
ce100ff
Compare
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 looks good to me, just some docs changes
ce100ff
to
0d59774
Compare
Signed-off-by: Bobbins228 <[email protected]>
Signed-off-by: Bobbins228 <[email protected]>
Signed-off-by: Bobbins228 <[email protected]>
Signed-off-by: Bobbins228 <[email protected]>
0d59774
to
c740864
Compare
/retest |
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: KPostOffice The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
1235fc8
into
project-codeflare:main
Issue link
Closes: RHOAIENG-9259
What changes have been made
head_cpus
andhead_memory
get_cluster
methodVerification steps
Setup
Notebook server ODH/RHOAI/Local
git clone https://github.com/project-codeflare/codeflare-sdk.git
poetry build
- install if needed (pip install poetry
)pip install --force-reinstall dist/codeflare_sdk-0.0.0.dev0-py3-none-any.whl
Testing
Testing the depreciating args
head_cpus
andhead_memory
Follow through the basic Ray demo. Set the
head_cpus
andhead_memory
parameters to a value of your choosing.You should get a warning that the parameters are being depreciated and to use the new ones.
The head cpu requests and limits should both equate the values you entered for the above.
Testing the new requests/limits args
In the ClusterConfiguration add the parameters
Set them to values of your choosing and the head pod of the Ray Cluster should reflect these values.
Checks