-
Notifications
You must be signed in to change notification settings - Fork 6
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
feat: add requested resources info to servers endpoint #261
Conversation
dacb8a5
to
a6f0d71
Compare
Thanks @lorenzo-cavazzi! I'm wondering if it would make sense to show both resource request and limit? otoh it might make the most sense to enforce both the memory and cpu requests as limits. At the moment, only the memory is enforced. Then we only need to show a single number, like you have done here. |
We don't get any |
yes, that was my point - maybe we should. What can happen is that one user can starve a node of CPU. CPU is a compressible resource, meaning the OS can decide to throttle it or not. If you don't enforce CPU limits, a single greedy process can take over all of the CPU on the node adversely affecting all others. |
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.
Looks good! We can decide if we need to change the CPU policy at a later point.
I was going to look into enforcing the CPU limit since your argument is convincing. Should I open a different issue to keep track of that? |
You just need to add the CPU limit to the pod in the spawner in the same way it's done for the memory. But maybe it warrants a discussion with the others - although the decision is either: a) we hope that the current way doesn't make too many people unhappy or b) we enforce it and then document the limitations |
Add the requested resources info to the
GET /servers
API response. This is an example, the new part is in the attributeresources
UPDATE: It's possible to test the change in
lorenzotest
deplyoment: https://lorenzotest.dev.renku.ch/fix #223