From 699e6f1190e0b968e9124508b4bf521744f7de09 Mon Sep 17 00:00:00 2001
From: John Kyros <jkyros@redhat.com>
Date: Thu, 14 Mar 2024 02:57:41 -0500
Subject: [PATCH] fix tests we broke

---
 .../pkg/updater/logic/updater_test.go         |  4 +++
 .../updater/priority/priority_processor.go    | 30 +++++++++++--------
 .../pkg/utils/test/test_utils.go              |  6 ++++
 3 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/vertical-pod-autoscaler/pkg/updater/logic/updater_test.go b/vertical-pod-autoscaler/pkg/updater/logic/updater_test.go
index 9a3f10dcca20..3547551d780d 100644
--- a/vertical-pod-autoscaler/pkg/updater/logic/updater_test.go
+++ b/vertical-pod-autoscaler/pkg/updater/logic/updater_test.go
@@ -157,6 +157,10 @@ func testRunOnceBase(
 			Get()
 
 		pods[i].Labels = labels
+		// We will test in-place separately, but we do need to account for these calls
+		eviction.On("CanInPlaceUpdate", pods[i]).Return(false)
+		eviction.On("IsInPlaceUpdating", pods[i]).Return(false)
+
 		eviction.On("CanEvict", pods[i]).Return(true)
 		eviction.On("Evict", pods[i], nil).Return(nil)
 	}
diff --git a/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go b/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go
index 1eb6aaeb0e20..e92ee209efdf 100644
--- a/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go
+++ b/vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go
@@ -72,20 +72,24 @@ func (*defaultPriorityProcessor) GetUpdatePriority(pod *apiv1.Pod, _ *vpa_types.
 				} else {
 					// TODO(jkyros): For in place VPA, this is gross, but we need this pod to be in the eviction list because it doesn't actually have
 					// the resources it asked for even if the spec is right
-					if statusRequest, hasStatusRequest := pod.Status.ContainerStatuses[num].Resources.Requests[resourceName]; hasStatusRequest {
-						if request.MilliValue() > statusRequest.MilliValue() {
-							// It's okay if we're actually still resizing, but if we can't now or we're stuck, make sure the pod
-							// is still in the list so we can evict it to go live on a fatter node or something
-							if pod.Status.Resize == apiv1.PodResizeStatusDeferred || pod.Status.Resize == apiv1.PodResizeStatusInfeasible {
-								klog.V(4).Infof("Pod %s looks like it's stuck scaling up (%v state), leaving it in for eviction", pod.Name, pod.Status.Resize)
-							} else {
-								klog.V(4).Infof("Pod %s is in the process of scaling up (%v state), leaving it alone", pod.Name, pod.Status.Resize)
+					// TODO(jkyros): Can we have empty container status at this point for real? It's at least failing the tests if we don't check, but
+					// we could just populate the status in the tests
+					if len(pod.Status.ContainerStatuses) > num {
+						if statusRequest, hasStatusRequest := pod.Status.ContainerStatuses[num].Resources.Requests[resourceName]; hasStatusRequest {
+							if request.MilliValue() > statusRequest.MilliValue() {
+								// It's okay if we're actually still resizing, but if we can't now or we're stuck, make sure the pod
+								// is still in the list so we can evict it to go live on a fatter node or something
+								if pod.Status.Resize == apiv1.PodResizeStatusDeferred || pod.Status.Resize == apiv1.PodResizeStatusInfeasible {
+									klog.V(4).Infof("Pod %s looks like it's stuck scaling up (%v state), leaving it in for eviction", pod.Name, pod.Status.Resize)
+								} else {
+									klog.V(4).Infof("Pod %s is in the process of scaling up (%v state), leaving it alone", pod.Name, pod.Status.Resize)
 
-							}
-							// I guess if it's not outside of compliance, it's probably okay it's stuck here?
-							if (hasLowerBound && statusRequest.Cmp(lowerBound) < 0) ||
-								(hasUpperBound && statusRequest.Cmp(upperBound) > 0) {
-								outsideRecommendedRange = true
+								}
+								// I guess if it's not outside of compliance, it's probably okay it's stuck here?
+								if (hasLowerBound && statusRequest.Cmp(lowerBound) < 0) ||
+									(hasUpperBound && statusRequest.Cmp(upperBound) > 0) {
+									outsideRecommendedRange = true
+								}
 							}
 						}
 					}
diff --git a/vertical-pod-autoscaler/pkg/utils/test/test_utils.go b/vertical-pod-autoscaler/pkg/utils/test/test_utils.go
index 94b15e6c7aec..38aa5d3ece0e 100644
--- a/vertical-pod-autoscaler/pkg/utils/test/test_utils.go
+++ b/vertical-pod-autoscaler/pkg/utils/test/test_utils.go
@@ -133,6 +133,12 @@ func (m *PodsEvictionRestrictionMock) CanInPlaceUpdate(pod *apiv1.Pod) bool {
 	return args.Bool(0)
 }
 
+// IsInPlaceUpdating is a mock implementation of PodsEvictionRestriction.IsInPlaceUpdating
+func (m *PodsEvictionRestrictionMock) IsInPlaceUpdating(pod *apiv1.Pod) bool {
+	args := m.Called(pod)
+	return args.Bool(0)
+}
+
 // PodListerMock is a mock of PodLister
 type PodListerMock struct {
 	mock.Mock