Address comments

Author: laoj2

vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation/recommendation_provider_test.go

@@ -520,17 +520,17 @@ func TestGetContainersResources(t *testing.T) {
 			addAll:           false,
 		},
 		{
-			name:      "Memory only recommendation, request and limits only set in containerStatus, addAll true",
+			name:      "CPU and memory recommendation, request and limits only set in containerStatus, addAll true",
 			container: test.Container().WithName("container").Get(),
 			containerStatus: test.ContainerStatus().WithName("container").
-				WithCPURequest(resource.MustParse("1m")).
+				WithCPURequest(resource.MustParse("1")).
 				WithMemRequest(resource.MustParse("1M")).
-				WithCPULimit(resource.MustParse("40m")).
+				WithCPULimit(resource.MustParse("10")).
 				WithMemLimit(resource.MustParse("3M")).Get(),
-			vpa:              test.VerticalPodAutoscaler().WithContainer("container").WithTargetResource(apiv1.ResourceMemory, "2M").Get(),
-			expectedCPU:      mustParseResourcePointer("1m"),
+			vpa:              test.VerticalPodAutoscaler().WithContainer("container").WithTarget("3", "2M").Get(),
+			expectedCPU:      mustParseResourcePointer("3"),
 			expectedMem:      mustParseResourcePointer("2M"),
-			expectedCPULimit: mustParseResourcePointer("40m"),
+			expectedCPULimit: mustParseResourcePointer("30"),
 			expectedMemLimit: mustParseResourcePointer("6M"),
 			addAll:           true,
 		},

vertical-pod-autoscaler/pkg/updater/priority/scaling_direction_pod_eviction_admission_test.go

@@ -157,12 +157,15 @@ func TestAdmitForSingleContainer(t *testing.T) {
 		assert.Equal(t, true, sdpea.Admit(podWithoutRequests, recommendation))
 	})
 
-	t.Run("it should admit a Pod for eviction if it has non-nil ContainerStatus resources, but with no requests", func(t *testing.T) {
+	t.Run("it should admit a Pod for eviction if recommendation is higher than ContainerStatus requests", func(t *testing.T) {
 		podWithContainerStatus := pod.DeepCopy()
-		podWithContainerStatus.Status.ContainerStatuses = []corev1.ContainerStatus{{
-			Name:      containerName,
-			Resources: &corev1.ResourceRequirements{},
-		}}
+		podWithContainerStatus.Status.ContainerStatuses = []corev1.ContainerStatus{
+			test.ContainerStatus().WithName(containerName).
+				WithCPURequest(resource.MustParse("100m")).
+				WithCPULimit(resource.MustParse("100m")).
+				WithMemRequest(resource.MustParse("1Gi")).
+				WithMemLimit(resource.MustParse("1Gi")).Get()}
+
 		evictionRequirements := map[*corev1.Pod][]*v1.EvictionRequirement{
 			podWithContainerStatus: {
 				{Resources: []corev1.ResourceName{corev1.ResourceCPU, corev1.ResourceMemory},
@@ -172,7 +175,11 @@ func TestAdmitForSingleContainer(t *testing.T) {
 		}
 		sdpea := NewScalingDirectionPodEvictionAdmission()
 		sdpea.(*scalingDirectionPodEvictionAdmission).EvictionRequirements = evictionRequirements
-		recommendation := test.Recommendation().WithContainer(containerName).WithTarget("500m", "10Gi").Get()
+		// Recommendation is higher than ContainerStatus requests, but lower than
+		// PodSpec requests. This should be admitted (eviction requirement is
+		// TargetHigherThanRequests), given that ContainerStatus has priority over
+		// PodSpec.
+		recommendation := test.Recommendation().WithContainer(containerName).WithTarget("400m", "4Gi").Get()
 
 		assert.Equal(t, true, sdpea.Admit(podWithContainerStatus, recommendation))
 	})

vertical-pod-autoscaler/pkg/utils/resources/resourcehelpers.go

@@ -18,19 +18,24 @@ package resourcehelpers
 
 import (
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/klog/v2"
 )
 
 // ContainerRequestsAndLimits returns a copy of the actual resource requests and
 // limits of a given container:
-//   - If in-place pod updates feature is enabled, the actual resource requests
+//
+//   - If in-place pod updates feature [1] is enabled, the actual resource requests
 //     are stored in the container status field.
 //   - Otherwise, fallback to the resource requests defined in the pod spec.
+//
+// [1] https://github.com/kubernetes/enhancements/tree/master/keps/sig-node/1287-in-place-update-pod-resources
 func ContainerRequestsAndLimits(containerName string, pod *v1.Pod) (v1.ResourceList, v1.ResourceList) {
 	cs := containerStatusForContainer(containerName, pod)
 	if cs != nil && cs.Resources != nil {
 		return cs.Resources.Requests.DeepCopy(), cs.Resources.Limits.DeepCopy()
 	}
 
+	klog.V(6).InfoS("Container resources not found in containerStatus for container. Falling back to resources defined in the pod spec. This is expected for clusters with in-place pod updates feature disabled.", "container", containerName, "containerStatus", cs)
 	container := findContainer(containerName, pod)
 	if container != nil {
 		return container.Resources.Requests.DeepCopy(), container.Resources.Limits.DeepCopy()