Skip to content

Commit

Permalink
fix tests we broke
Browse files Browse the repository at this point in the history
  • Loading branch information
jkyros committed Mar 14, 2024
1 parent 29fa69c commit 699e6f1
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 13 deletions.
4 changes: 4 additions & 0 deletions vertical-pod-autoscaler/pkg/updater/logic/updater_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
30 changes: 17 additions & 13 deletions vertical-pod-autoscaler/pkg/updater/priority/priority_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions vertical-pod-autoscaler/pkg/utils/test/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 699e6f1

Please sign in to comment.