diff --git a/resource/helpers.go b/resource/helpers.go index 1d83aa8..a8b252c 100644 --- a/resource/helpers.go +++ b/resource/helpers.go @@ -35,8 +35,10 @@ type PodResourcesOptions struct { // Reuse, if provided will be reused to accumulate resources and returned by the PodRequests or PodLimits // functions. All existing values in Reuse will be lost. Reuse v1.ResourceList - // InPlacePodVerticalScalingEnabled indicates that the in-place pod vertical scaling feature gate is enabled. - InPlacePodVerticalScalingEnabled bool + // UseStatusResources indicates whether resources reported by the PodStatus should be considered + // when evaluating the pod resources. This MUST be false if the InPlacePodVerticalScaling + // feature is not enabled. + UseStatusResources bool // ExcludeOverhead controls if pod overhead is excluded from the calculation. ExcludeOverhead bool // ContainerFn is called with the effective resources required for each container within the pod. @@ -54,7 +56,7 @@ func PodRequests(pod *v1.Pod, opts PodResourcesOptions) v1.ResourceList { reqs := reuseOrClearResourceList(opts.Reuse) var containerStatuses map[string]*v1.ContainerStatus - if opts.InPlacePodVerticalScalingEnabled { + if opts.UseStatusResources { containerStatuses = make(map[string]*v1.ContainerStatus, len(pod.Status.ContainerStatuses)) for i := range pod.Status.ContainerStatuses { containerStatuses[pod.Status.ContainerStatuses[i].Name] = &pod.Status.ContainerStatuses[i] @@ -63,13 +65,13 @@ func PodRequests(pod *v1.Pod, opts PodResourcesOptions) v1.ResourceList { for _, container := range pod.Spec.Containers { containerReqs := container.Resources.Requests - if opts.InPlacePodVerticalScalingEnabled { + if opts.UseStatusResources { cs, found := containerStatuses[container.Name] - if found { + if found && cs.Resources != nil { if pod.Status.Resize == v1.PodResizeStatusInfeasible { - containerReqs = cs.AllocatedResources.DeepCopy() + containerReqs = cs.Resources.Requests.DeepCopy() } else { - containerReqs = max(container.Resources.Requests, cs.AllocatedResources) + containerReqs = max(container.Resources.Requests, cs.Resources.Requests) } } } @@ -155,11 +157,31 @@ func PodLimits(pod *v1.Pod, opts PodResourcesOptions) v1.ResourceList { // attempt to reuse the maps if passed, or allocate otherwise limits := reuseOrClearResourceList(opts.Reuse) + var containerStatuses map[string]*v1.ContainerStatus + if opts.UseStatusResources { + containerStatuses = make(map[string]*v1.ContainerStatus, len(pod.Status.ContainerStatuses)) + for i := range pod.Status.ContainerStatuses { + containerStatuses[pod.Status.ContainerStatuses[i].Name] = &pod.Status.ContainerStatuses[i] + } + } + for _, container := range pod.Spec.Containers { + containerLimits := container.Resources.Limits + if opts.UseStatusResources { + cs, found := containerStatuses[container.Name] + if found && cs.Resources != nil { + if pod.Status.Resize == v1.PodResizeStatusInfeasible { + containerLimits = cs.Resources.Limits.DeepCopy() + } else { + containerLimits = max(container.Resources.Limits, cs.Resources.Limits) + } + } + } + if opts.ContainerFn != nil { - opts.ContainerFn(container.Resources.Limits, Containers) + opts.ContainerFn(containerLimits, Containers) } - addResourceList(limits, container.Resources.Limits) + addResourceList(limits, containerLimits) } restartableInitContainerLimits := v1.ResourceList{} diff --git a/resource/helpers_test.go b/resource/helpers_test.go index 9d993f4..f2d984c 100644 --- a/resource/helpers_test.go +++ b/resource/helpers_test.go @@ -432,7 +432,7 @@ func TestPodResourceRequests(t *testing.T) { v1.ResourceCPU: resource.MustParse("2"), }, podResizeStatus: v1.PodResizeStatusInfeasible, - options: PodResourcesOptions{InPlacePodVerticalScalingEnabled: true}, + options: PodResourcesOptions{UseStatusResources: true}, containers: []v1.Container{ { Name: "container-1", @@ -446,8 +446,10 @@ func TestPodResourceRequests(t *testing.T) { containerStatus: []v1.ContainerStatus{ { Name: "container-1", - AllocatedResources: v1.ResourceList{ - v1.ResourceCPU: resource.MustParse("2"), + Resources: &v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("2"), + }, }, }, }, @@ -457,7 +459,7 @@ func TestPodResourceRequests(t *testing.T) { expectedRequests: v1.ResourceList{ v1.ResourceCPU: resource.MustParse("4"), }, - options: PodResourcesOptions{InPlacePodVerticalScalingEnabled: true}, + options: PodResourcesOptions{UseStatusResources: true}, containers: []v1.Container{ { Name: "container-1", @@ -471,19 +473,21 @@ func TestPodResourceRequests(t *testing.T) { containerStatus: []v1.ContainerStatus{ { Name: "container-1", - AllocatedResources: v1.ResourceList{ - v1.ResourceCPU: resource.MustParse("2"), + Resources: &v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("2"), + }, }, }, }, }, { - description: "resized, infeasible, feature gate disabled", + description: "resized, infeasible, but don't use status", expectedRequests: v1.ResourceList{ v1.ResourceCPU: resource.MustParse("4"), }, podResizeStatus: v1.PodResizeStatusInfeasible, - options: PodResourcesOptions{InPlacePodVerticalScalingEnabled: false}, + options: PodResourcesOptions{UseStatusResources: false}, containers: []v1.Container{ { Name: "container-1", @@ -497,8 +501,10 @@ func TestPodResourceRequests(t *testing.T) { containerStatus: []v1.ContainerStatus{ { Name: "container-1", - AllocatedResources: v1.ResourceList{ - v1.ResourceCPU: resource.MustParse("2"), + Resources: &v1.ResourceRequirements{ + Requests: v1.ResourceList{ + v1.ResourceCPU: resource.MustParse("2"), + }, }, }, }, @@ -742,12 +748,13 @@ func TestPodResourceRequestsReuse(t *testing.T) { func TestPodResourceLimits(t *testing.T) { restartAlways := v1.ContainerRestartPolicyAlways testCases := []struct { - description string - options PodResourcesOptions - overhead v1.ResourceList - initContainers []v1.Container - containers []v1.Container - expectedLimits v1.ResourceList + description string + options PodResourcesOptions + overhead v1.ResourceList + initContainers []v1.Container + containers []v1.Container + containerStatuses []v1.ContainerStatus + expectedLimits v1.ResourceList }{ { description: "nil options, larger init container", @@ -1119,6 +1126,87 @@ func TestPodResourceLimits(t *testing.T) { }, }, }, + { + description: "pod scaled up", + expectedLimits: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("2Gi"), + }, + options: PodResourcesOptions{UseStatusResources: true}, + containers: []v1.Container{ + { + Name: "container-1", + Resources: v1.ResourceRequirements{ + Limits: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("2Gi"), + }, + }, + }, + }, + containerStatuses: []v1.ContainerStatus{ + { + Name: "container-1", + Resources: &v1.ResourceRequirements{ + Limits: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + }, + }, + }, + }, + }, + { + description: "pod scaled down", + expectedLimits: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("2Gi"), + }, + options: PodResourcesOptions{UseStatusResources: true}, + containers: []v1.Container{ + { + Name: "container-1", + Resources: v1.ResourceRequirements{ + Limits: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + }, + }, + }, + }, + containerStatuses: []v1.ContainerStatus{ + { + Name: "container-1", + Resources: &v1.ResourceRequirements{ + Limits: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("2Gi"), + }, + }, + }, + }, + }, + { + description: "pod scaled down, don't use status", + expectedLimits: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + }, + options: PodResourcesOptions{UseStatusResources: false}, + containers: []v1.Container{ + { + Name: "container-1", + Resources: v1.ResourceRequirements{ + Limits: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("1Gi"), + }, + }, + }, + }, + containerStatuses: []v1.ContainerStatus{ + { + Name: "container-1", + Resources: &v1.ResourceRequirements{ + Limits: v1.ResourceList{ + v1.ResourceMemory: resource.MustParse("2Gi"), + }, + }, + }, + }, + }, } for _, tc := range testCases { t.Run(tc.description, func(t *testing.T) { @@ -1128,6 +1216,9 @@ func TestPodResourceLimits(t *testing.T) { InitContainers: tc.initContainers, Overhead: tc.overhead, }, + Status: v1.PodStatus{ + ContainerStatuses: tc.containerStatuses, + }, } limits := PodLimits(p, tc.options) if diff := cmp.Diff(limits, tc.expectedLimits); diff != "" {