Merge pull request #128377 from tallclair/allocated-status-2

[FG:InPlacePodVerticalScaling] Implement AllocatedResources status changes for Beta Kubernetes-commit: f81a68f4888c9da87f856d17516fb810518a67c0
kubernetes · Nov 6, 2024 · 152c232 · 152c232
2 parents d4567c4 + b14c363
commit 152c232
Show file tree

Hide file tree

Showing 2 changed files with 138 additions and 25 deletions.
diff --git 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
@@ -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 != "" {