-
Notifications
You must be signed in to change notification settings - Fork 294
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
Default requests to limits in Workload #317
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: alculquicondor 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 |
if the requests are not set Change-Id: I9f59d9e87f48d51afc1c48082943cc3233a2fd1b
1d4b168
to
3fd046a
Compare
func setContainersDefaults(containers []corev1.Container) { | ||
for i := range containers { | ||
c := &containers[i] | ||
if c.Resources.Limits != nil { |
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.
Not related to this PR, we currently leverage the requests in resource quota, should we also consider the limits.
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.
Are you talking about kubernetes' ResourceQuotas? If so, I would imagine they run after defaults are applied, but I don't have expertise to know for sure.
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 mean the resources.Limits in podSpec.
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.
It was the intention that we only look at requests, similar to the NodeResources Filter plugin in kube-scheduler.
/lgtm |
In kubernetes-sigs#317, we will default workload resource requests to limits if requests not specified. But this will introduce a bug for in job controller reconciling, job will be judged as not equal to workload, so workload will be deleted and re-create indefinitely Signed-off-by: Kante Yin <[email protected]>
In kubernetes-sigs#317, we will default workload resource requests to limits if requests not specified. But this will introduce a bug for in job controller reconciling, job will be judged as not equal to workload, so workload will be deleted and re-create indefinitely Signed-off-by: Kante Yin <[email protected]>
In kubernetes-sigs#317, we will default workload resource requests to limits if requests not specified. But this will introduce a bug for in job controller reconciling, job will be judged as not equal to workload, so workload will be deleted and re-create indefinitely Signed-off-by: Kante Yin <[email protected]>
In kubernetes-sigs#317, we will default workload resource requests to limits if requests not specified. But this will introduce a bug for in job controller reconciling, job will be judged as not equal to workload, so workload will be deleted and re-create indefinitely Signed-off-by: Kante Yin <[email protected]>
In kubernetes-sigs#317, we will default workload resource requests to limits if requests not specified. But this will introduce a bug for in job controller reconciling, job will be judged as not equal to workload, so workload will be deleted and re-create indefinitely Signed-off-by: Kante Yin <[email protected]>
What type of PR is this?
/kind feature
What this PR does / why we need it:
Default requests to limits if requests are not set.
This matches the internal defaulting for k8s Pods.
Which issue(s) this PR fixes:
Fixes #316
Special notes for your reviewer: