Skip to content

Commit c30bcc6

Browse files
committed
VPA: (squash) Fix updater test for in-place
So in-place VPA checks happen before eviction checks do, and we fall back to eviction if in-place VPA isn't possible. In-place VPA happens immediatly (rather than the delay with eviction) so we seem to be able to catch early "0" recommendations from the VPA when it hasn't recommended anything yet unless we wait for the VPA's recommendation to be provided. The problem is, the updater test was not set up to accommidate that, it expects eviction immediately which would seem to be wrong and probably not what we want (it's possible something else before in-place made that "not wrong" but for now I'm operating under the assumption that the "eviction delay" was saving us from having any trouble with this in the real world). So until I get to the bottom of how we should be behaving when the VPA hasn't made a recommendation yet, I'm going to adjust the test suite so it allows for in-place's "wait until we have a recommendation" logic. And I need to come back here and have proper tests for in-place with simulating the InPlacePodVerticalScaling behavior being enabled.
1 parent a1c5830 commit c30bcc6

File tree

1 file changed

+16
-7
lines changed

1 file changed

+16
-7
lines changed

vertical-pod-autoscaler/pkg/updater/logic/updater_test.go

Lines changed: 16 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,12 @@ import (
2222
"testing"
2323
"time"
2424

25-
"golang.org/x/time/rate"
26-
v1 "k8s.io/api/autoscaling/v1"
27-
2825
"github.com/golang/mock/gomock"
2926
"github.com/stretchr/testify/assert"
27+
"golang.org/x/time/rate"
28+
v1 "k8s.io/api/autoscaling/v1"
3029
apiv1 "k8s.io/api/core/v1"
30+
corev1 "k8s.io/api/core/v1"
3131
"k8s.io/apimachinery/pkg/api/resource"
3232
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
3333
"k8s.io/apimachinery/pkg/labels"
@@ -160,7 +160,6 @@ func testRunOnceBase(
160160
// We will test in-place separately, but we do need to account for these calls
161161
eviction.On("CanInPlaceUpdate", pods[i]).Return(false)
162162
eviction.On("IsInPlaceUpdating", pods[i]).Return(false)
163-
164163
eviction.On("CanEvict", pods[i]).Return(true)
165164
eviction.On("Evict", pods[i], nil).Return(nil)
166165
}
@@ -175,12 +174,17 @@ func testRunOnceBase(
175174
Name: rc.Name,
176175
APIVersion: rc.APIVersion,
177176
}
177+
// TOD0(jkyros): I added the recommendationProvided condition here because in-place needs to wait for a
178+
// recommendation to scale, causing this test to fail (because in-place checks before eviction, and in-place will
179+
// wait to scale -- and not fall back to eviction -- until the VPA has made a recommendation)
180+
178181
vpaObj := test.VerticalPodAutoscaler().
179182
WithContainer(containerName).
180183
WithTarget("2", "200M").
181184
WithMinAllowed(containerName, "1", "100M").
182185
WithMaxAllowed(containerName, "3", "1G").
183-
WithTargetRef(targetRef).Get()
186+
WithTargetRef(targetRef).
187+
AppendCondition(vpa_types.RecommendationProvided, corev1.ConditionTrue, "reason", "msg", time.Unix(0, 0)).Get()
184188

185189
vpaObj.Spec.UpdatePolicy = &vpa_types.PodUpdatePolicy{UpdateMode: &updateMode}
186190
vpaLister.On("List").Return([]*vpa_types.VerticalPodAutoscaler{vpaObj}, nil).Once()
@@ -312,14 +316,19 @@ func TestRunOnceIgnoreNamespaceMatchingPods(t *testing.T) {
312316
Name: rc.Name,
313317
APIVersion: rc.APIVersion,
314318
}
319+
320+
// TOD0(jkyros): I added the recommendationProvided condition here because in-place needs to wait for a
321+
// recommendation to scale, causing this test to fail (because in-place checks before eviction, and in-place will
322+
// wait to scale -- and not fall back to eviction -- until the VPA has made a recommendation)
315323
vpaObj := test.VerticalPodAutoscaler().
316324
WithNamespace("default").
317325
WithContainer(containerName).
318326
WithTarget("2", "200M").
319327
WithMinAllowed(containerName, "1", "100M").
320328
WithMaxAllowed(containerName, "3", "1G").
321-
WithTargetRef(targetRef).Get()
322-
329+
WithTargetRef(targetRef).
330+
AppendCondition(vpa_types.RecommendationProvided, corev1.ConditionTrue, "reason", "msg", time.Unix(0, 0)).
331+
Get()
323332
vpaLister.On("List").Return([]*vpa_types.VerticalPodAutoscaler{vpaObj}, nil).Once()
324333

325334
mockSelectorFetcher := target_mock.NewMockVpaTargetSelectorFetcher(ctrl)

0 commit comments

Comments
 (0)