-
Notifications
You must be signed in to change notification settings - Fork 67
Add container resource caps enforcement for workspace containers #1545
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
Changes from 5 commits
70d7d33
bf8aa66
55a9d3a
ea9d22f
5155e99
dca26b2
6f633cb
34713b8
e3ea44f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
@tolusha, if a container does not specify limits / requests the maximums from DWOC will be used, right?
This is an important nuance that should be documented at the specification level, I believe.
@cgruver could you confirm that this is a requested behaviour?
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.
Semantically when I hear
ContainerResourceCaps, to me it sounds like only an upper limit, and not a default if limits / requests are not defined in the devfile.If we want defaults applied when limits/requests are not defined in the devfile, IMHO the
DefaultContainerResourcesshould be the field for thatThere 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 would rather not have this functionality, since right now,
DefaultContainerResourceswill be used instead which IMHO makes more senseThere 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.
@dkwon17
Updated