Consider AllocatableResources when computing pod requests

Author: tallclair

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
 }

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{

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
 }