From c2927727107cc8123c2688ea571f464650df6b2b Mon Sep 17 00:00:00 2001 From: Tim Allclair Date: Mon, 17 Mar 2025 21:23:32 -0700 Subject: [PATCH] Consider AllocatableResources when computing pod requests --- .../component-helpers/resource/helpers.go | 25 ++----- .../resource/helpers_test.go | 74 +++++++++++++++++++ .../kubectl/pkg/util/resource/resource.go | 25 ++----- 3 files changed, 88 insertions(+), 36 deletions(-) diff --git a/staging/src/k8s.io/component-helpers/resource/helpers.go b/staging/src/k8s.io/component-helpers/resource/helpers.go index b848603cb58..6977201c7aa 100644 --- a/staging/src/k8s.io/component-helpers/resource/helpers.go +++ b/staging/src/k8s.io/component-helpers/resource/helpers.go @@ -224,9 +224,9 @@ func AggregateContainerRequests(pod *v1.Pod, opts PodResourcesOptions) v1.Resour // determineContainerReqs will return a copy of the container requests based on if resizing is feasible or not. func determineContainerReqs(pod *v1.Pod, container *v1.Container, cs *v1.ContainerStatus) v1.ResourceList { if IsPodResizeInfeasible(pod) { - return cs.Resources.Requests.DeepCopy() + return max(cs.Resources.Requests, cs.AllocatedResources) } - return max(container.Resources.Requests, cs.Resources.Requests) + return max(container.Resources.Requests, cs.Resources.Requests, cs.AllocatedResources) } // determineContainerLimits will return a copy of the container limits based on if resizing is feasible or not. @@ -399,23 +399,12 @@ func maxResourceList(list, newList v1.ResourceList) { } } -// max returns the result of max(a, b) for each named resource and is only used if we can't +// max returns the result of max(a, b...) for each named resource and is only used if we can't // accumulate into an existing resource list -func max(a v1.ResourceList, b v1.ResourceList) v1.ResourceList { - result := v1.ResourceList{} - for key, value := range a { - if other, found := b[key]; found { - if value.Cmp(other) <= 0 { - result[key] = other.DeepCopy() - continue - } - } - result[key] = value.DeepCopy() - } - for key, value := range b { - if _, found := result[key]; !found { - result[key] = value.DeepCopy() - } +func max(a v1.ResourceList, b ...v1.ResourceList) v1.ResourceList { + result := a.DeepCopy() + for _, other := range b { + maxResourceList(result, other) } return result } diff --git a/staging/src/k8s.io/component-helpers/resource/helpers_test.go b/staging/src/k8s.io/component-helpers/resource/helpers_test.go index b1717f1e328..ba18bb23d07 100644 --- a/staging/src/k8s.io/component-helpers/resource/helpers_test.go +++ b/staging/src/k8s.io/component-helpers/resource/helpers_test.go @@ -459,6 +459,41 @@ func TestPodResourceRequests(t *testing.T) { }, }, }, + { + description: "resized, infeasible & in-progress", + expectedRequests: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("4"), + }, + podResizeStatus: []v1.PodCondition{{ + Type: v1.PodResizePending, + Status: v1.ConditionTrue, + Reason: v1.PodReasonInfeasible, + }}, + options: PodResourcesOptions{UseStatusResources: true}, + containers: []v1.Container{ + { + Name: "container-1", + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("6"), + }, + }, + }, + }, + containerStatus: []v1.ContainerStatus{ + { + Name: "container-1", + AllocatedResources: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("4"), + }, + Resources: &v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("2"), + }, + }, + }, + }, + }, { description: "resized, no resize status", expectedRequests: v1.ResourceList{ @@ -486,6 +521,45 @@ func TestPodResourceRequests(t *testing.T) { }, }, }, + { + description: "resized: per-resource 3-way maximum", + expectedRequests: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("30m"), + v1.ResourceMemory: resource.MustParse("30M"), + // Note: EphemeralStorage is not resizable, but that doesn't matter for the purposes of this test. + v1.ResourceEphemeralStorage: resource.MustParse("30G"), + }, + options: PodResourcesOptions{UseStatusResources: true}, + containers: []v1.Container{ + { + Name: "container-1", + Resources: v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("30m"), + v1.ResourceMemory: resource.MustParse("20M"), + v1.ResourceEphemeralStorage: resource.MustParse("10G"), + }, + }, + }, + }, + containerStatus: []v1.ContainerStatus{ + { + Name: "container-1", + AllocatedResources: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("20m"), + v1.ResourceMemory: resource.MustParse("10M"), + v1.ResourceEphemeralStorage: resource.MustParse("30G"), + }, + Resources: &v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("10m"), + v1.ResourceMemory: resource.MustParse("30M"), + v1.ResourceEphemeralStorage: resource.MustParse("20G"), + }, + }, + }, + }, + }, { description: "resized, infeasible, but don't use status", expectedRequests: v1.ResourceList{ diff --git a/staging/src/k8s.io/kubectl/pkg/util/resource/resource.go b/staging/src/k8s.io/kubectl/pkg/util/resource/resource.go index 376c02dd05e..46530d482f4 100644 --- a/staging/src/k8s.io/kubectl/pkg/util/resource/resource.go +++ b/staging/src/k8s.io/kubectl/pkg/util/resource/resource.go @@ -144,28 +144,17 @@ func podLimits(pod *corev1.Pod) corev1.ResourceList { // determineContainerReqs will return a copy of the container requests based on if resizing is feasible or not. func determineContainerReqs(pod *corev1.Pod, container *corev1.Container, cs *corev1.ContainerStatus) corev1.ResourceList { if helpers.IsPodResizeInfeasible(pod) { - return cs.Resources.Requests.DeepCopy() + return max(cs.Resources.Requests, cs.AllocatedResources) } - return max(container.Resources.Requests, cs.Resources.Requests) + return max(container.Resources.Requests, cs.Resources.Requests, cs.AllocatedResources) } -// max returns the result of max(a, b) for each named resource and is only used if we can't +// max returns the result of max(a, b...) for each named resource and is only used if we can't // accumulate into an existing resource list -func max(a corev1.ResourceList, b corev1.ResourceList) corev1.ResourceList { - result := corev1.ResourceList{} - for key, value := range a { - if other, found := b[key]; found { - if value.Cmp(other) <= 0 { - result[key] = other.DeepCopy() - continue - } - } - result[key] = value.DeepCopy() - } - for key, value := range b { - if _, found := result[key]; !found { - result[key] = value.DeepCopy() - } +func max(a corev1.ResourceList, b ...corev1.ResourceList) corev1.ResourceList { + result := a.DeepCopy() + for _, other := range b { + maxResourceList(result, other) } return result }