Skip to content
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

Kubectl: take pod overhead into account for node info #81929

Merged
merged 1 commit into from Aug 31, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 15 additions & 1 deletion staging/src/k8s.io/kubectl/pkg/util/resource/resource.go
Expand Up @@ -28,7 +28,9 @@ import (
)

// PodRequestsAndLimits returns a dictionary of all defined resources summed up for all
// containers of the pod.
// containers of the pod. If pod overhead is non-nil, the pod overhead is added to the
// total container resource requests and to the total container limits which have a
// non-zero quantity.
func PodRequestsAndLimits(pod *corev1.Pod) (reqs, limits corev1.ResourceList) {
reqs, limits = corev1.ResourceList{}, corev1.ResourceList{}
for _, container := range pod.Spec.Containers {
Expand All @@ -40,6 +42,18 @@ func PodRequestsAndLimits(pod *corev1.Pod) (reqs, limits corev1.ResourceList) {
maxResourceList(reqs, container.Resources.Requests)
maxResourceList(limits, container.Resources.Limits)
}

// Add overhead for running a pod to the sum of requests and to non-zero limits:
if pod.Spec.Overhead != nil {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the feature gate is disabled in apiserver, we have logic setting it to nil, right?

Copy link
Contributor Author

@egernst egernst Aug 28, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No.

If someone is explicitly setting this in the Pod.Spec when the feature gate is not enabled, the value will persist. if the feature gate is enabled, the RuntimeClass admission controller will validate the Pod.Spec to make sure the defined overhead matches what is defined in a given RuntimeClass. If it doesn't match, the pod is rejected.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are dropping it server-side if the feature gate is disabled: https://github.com/kubernetes/kubernetes/blob/master/pkg/api/pod/util.go#L399-L402

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense then.

addResourceList(reqs, pod.Spec.Overhead)

for name, quantity := range pod.Spec.Overhead {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems the rule here is:

  • add overhead to reqs no matter each container has req defined or not.
  • add overhead to limits only when at least one container has limit defined.

Is it the case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct.

if value, ok := limits[name]; ok && !value.IsZero() {
value.Add(quantity)
limits[name] = value
}
}
}
return
}

Expand Down