Skip to content

Commit 5d67d4d

Browse files
maxcao13maxcao13
committed
VPA: upgrade InPlacePodVerticalScaling internal logic to k8s 1.33
Signed-off-by: Max Cao <[email protected]>
1 parent b0903ae commit 5d67d4d

File tree

8 files changed

+136
-58
lines changed

8 files changed

+136
-58
lines changed

‎vertical-pod-autoscaler/e2e/v1/actuation.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ import (
3636
"k8s.io/autoscaler/vertical-pod-autoscaler/e2e/utils"
3737
vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1"
3838
restriction "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/restriction"
39+
updaterutils "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/updater/utils"
3940
"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/annotations"
4041
clientset "k8s.io/client-go/kubernetes"
4142
"k8s.io/kubernetes/test/e2e/framework"
@@ -252,8 +253,11 @@ var _ = ActuationSuiteE2eDescribe("Actuation", ginkgo.Label("FG:InPlaceOrRecreat
252253
return err
253254
}
254255
for _, pod := range updatedPodList.Items {
255-
if pod.Status.Resize == apiv1.PodResizeStatusDeferred {
256-
return nil
256+
cond, ok := updaterutils.GetPodCondition(&pod, apiv1.PodResizePending)
257+
if ok {
258+
if cond.Reason == apiv1.PodReasonDeferred && cond.Status == apiv1.ConditionTrue {
259+
return nil
260+
}
257261
}
258262
}
259263
return fmt.Errorf("status not deferred")

‎vertical-pod-autoscaler/e2e/v1/common.go‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,7 @@ func InstallLimitRangeWithMin(f *framework.Framework, minCpuLimit, minMemoryLimi
563563

564564
// WaitForPodsUpdatedWithoutEviction waits for pods to be updated without any evictions taking place over the polling
565565
// interval.
566+
// TODO: Use events to track in-place resizes instead of polling when ready: https://github.com/kubernetes/kubernetes/issues/127172
566567
func WaitForPodsUpdatedWithoutEviction(f *framework.Framework, initialPods *apiv1.PodList) error {
567568
framework.Logf("waiting for at least one pod to be updated without eviction")
568569
err := wait.PollUntilContextTimeout(context.TODO(), pollInterval, VpaInPlaceTimeout, false, func(context.Context) (bool, error) {
@@ -618,18 +619,6 @@ func WaitForPodsUpdatedWithoutEviction(f *framework.Framework, initialPods *apiv
618619
func checkInPlaceOrRecreateTestsEnabled(f *framework.Framework, checkAdmission, checkUpdater bool) {
619620
ginkgo.By("Checking InPlacePodVerticalScaling cluster feature gate is on")
620621

621-
podList, err := f.ClientSet.CoreV1().Pods("kube-system").List(context.TODO(), metav1.ListOptions{
622-
LabelSelector: "component=kube-apiserver",
623-
})
624-
gomega.Expect(err).NotTo(gomega.HaveOccurred())
625-
apiServerPod := podList.Items[0]
626-
gomega.Expect(apiServerPod.Spec.Containers).To(gomega.HaveLen(1))
627-
apiServerContainer := apiServerPod.Spec.Containers[0]
628-
gomega.Expect(apiServerContainer.Name).To(gomega.Equal("kube-apiserver"))
629-
if !anyContainsSubstring(apiServerContainer.Command, "InPlacePodVerticalScaling=true") {
630-
ginkgo.Skip("Skipping suite: InPlacePodVerticalScaling feature gate is not enabled on the cluster level")
631-
}
632-
633622
if checkUpdater {
634623
ginkgo.By("Checking InPlaceOrRecreate VPA feature gate is enabled for updater")
635624

‎vertical-pod-autoscaler/pkg/updater/restriction/pods_inplace_restriction.go‎

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -188,28 +188,40 @@ func CanEvictInPlacingPod(pod *apiv1.Pod, singleGroupStats singleGroupStats, las
188188
}
189189

190190
if singleGroupStats.isPodDisruptable() {
191-
// TODO(maxcao13): fix this after 1.33 KEP changes
192191
// if currently inPlaceUpdating, we should only fallback to eviction if the update has failed. i.e: one of the following conditions:
193-
// 1. .status.resize: Infeasible
194-
// 2. .status.resize: Deferred + more than 5 minutes has elapsed since the lastInPlaceUpdateTime
195-
// 3. .status.resize: InProgress + more than 1 hour has elapsed since the lastInPlaceUpdateTime
196-
switch pod.Status.Resize {
197-
case apiv1.PodResizeStatusDeferred:
198-
if clock.Since(lastUpdate) > DeferredResizeUpdateTimeout {
199-
klog.V(4).InfoS(fmt.Sprintf("In-place update deferred for more than %v, falling back to eviction", DeferredResizeUpdateTimeout), "pod", klog.KObj(pod))
192+
// - Infeasible
193+
// - Deferred + more than 5 minutes has elapsed since the lastInPlaceUpdateTime
194+
// - InProgress + more than 1 hour has elapsed since the lastInPlaceUpdateTime
195+
resizePendingCondition, ok := utils.GetPodCondition(pod, apiv1.PodResizePending)
196+
if ok {
197+
if resizePendingCondition.Reason == apiv1.PodReasonDeferred {
198+
if clock.Since(lastUpdate) > DeferredResizeUpdateTimeout {
199+
klog.V(4).InfoS(fmt.Sprintf("In-place update deferred for more than %v, falling back to eviction", DeferredResizeUpdateTimeout), "pod", klog.KObj(pod))
200+
return true
201+
}
202+
} else if resizePendingCondition.Reason == apiv1.PodReasonInfeasible {
203+
klog.V(4).InfoS("In-place update infeasible, falling back to eviction", "pod", klog.KObj(pod))
200204
return true
201-
}
202-
case apiv1.PodResizeStatusInProgress:
203-
if clock.Since(lastUpdate) > InProgressResizeUpdateTimeout {
204-
klog.V(4).InfoS(fmt.Sprintf("In-place update in progress for more than %v, falling back to eviction", InProgressResizeUpdateTimeout), "pod", klog.KObj(pod))
205+
} else {
206+
klog.V(4).InfoS("In-place update condition unknown, falling back to eviction", "pod", klog.KObj(pod), "condition", resizePendingCondition)
205207
return true
206208
}
207-
case apiv1.PodResizeStatusInfeasible:
208-
klog.V(4).InfoS("In-place update infeasible, falling back to eviction", "pod", klog.KObj(pod))
209-
return true
210-
default:
211-
klog.V(4).InfoS("In-place update status unknown, falling back to eviction", "pod", klog.KObj(pod))
212-
return true
209+
} else {
210+
resizeInProgressCondition, ok := utils.GetPodCondition(pod, apiv1.PodResizeInProgress)
211+
if ok {
212+
if resizeInProgressCondition.Reason == "" && resizeInProgressCondition.Message == "" {
213+
if clock.Since(lastUpdate) > InProgressResizeUpdateTimeout {
214+
klog.V(4).InfoS(fmt.Sprintf("In-place update in progress for more than %v, falling back to eviction", InProgressResizeUpdateTimeout), "pod", klog.KObj(pod))
215+
return true
216+
}
217+
} else if resizeInProgressCondition.Reason == apiv1.PodReasonError {
218+
klog.V(4).InfoS("In-place update error, falling back to eviction", "pod", klog.KObj(pod), "message", resizeInProgressCondition.Message)
219+
return true
220+
} else {
221+
klog.V(4).InfoS("In-place update condition unknown, falling back to eviction", "pod", klog.KObj(pod), "condition", resizeInProgressCondition)
222+
return true
223+
}
224+
}
213225
}
214226
return false
215227
}

‎vertical-pod-autoscaler/pkg/updater/restriction/pods_inplace_restriction_test.go‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,13 @@ func TestCanInPlaceUpdate(t *testing.T) {
113113
{
114114
name: "CanInPlaceUpdate=InPlaceDeferred - resize Deferred, conditions not met to fallback",
115115
pods: []*apiv1.Pod{
116-
generatePod().WithResizeStatus(apiv1.PodResizeStatusDeferred).Get(),
116+
generatePod().WithPodConditions([]apiv1.PodCondition{
117+
{
118+
Type: apiv1.PodResizePending,
119+
Status: apiv1.ConditionTrue,
120+
Reason: apiv1.PodReasonDeferred,
121+
},
122+
}).Get(),
117123
generatePod().Get(),
118124
generatePod().Get(),
119125
},
@@ -125,7 +131,12 @@ func TestCanInPlaceUpdate(t *testing.T) {
125131
{
126132
name: ("CanInPlaceUpdate=InPlaceEvict - resize inProgress for more too long"),
127133
pods: []*apiv1.Pod{
128-
generatePod().WithResizeStatus(apiv1.PodResizeStatusInProgress).Get(),
134+
generatePod().WithPodConditions([]apiv1.PodCondition{
135+
{
136+
Type: apiv1.PodResizeInProgress,
137+
Status: apiv1.ConditionTrue,
138+
},
139+
}).Get(),
129140
generatePod().Get(),
130141
generatePod().Get(),
131142
},
@@ -137,7 +148,12 @@ func TestCanInPlaceUpdate(t *testing.T) {
137148
{
138149
name: "CanInPlaceUpdate=InPlaceDeferred - resize InProgress, conditions not met to fallback",
139150
pods: []*apiv1.Pod{
140-
generatePod().WithResizeStatus(apiv1.PodResizeStatusInProgress).Get(),
151+
generatePod().WithPodConditions([]apiv1.PodCondition{
152+
{
153+
Type: apiv1.PodResizeInProgress,
154+
Status: apiv1.ConditionTrue,
155+
},
156+
}).Get(),
141157
generatePod().Get(),
142158
generatePod().Get(),
143159
},
@@ -149,7 +165,13 @@ func TestCanInPlaceUpdate(t *testing.T) {
149165
{
150166
name: "CanInPlaceUpdate=InPlaceEvict - infeasible",
151167
pods: []*apiv1.Pod{
152-
generatePod().WithResizeStatus(apiv1.PodResizeStatusInfeasible).Get(),
168+
generatePod().WithPodConditions([]apiv1.PodCondition{
169+
{
170+
Type: apiv1.PodResizePending,
171+
Status: apiv1.ConditionTrue,
172+
Reason: apiv1.PodReasonInfeasible,
173+
},
174+
}).Get(),
153175
generatePod().Get(),
154176
generatePod().Get(),
155177
},

‎vertical-pod-autoscaler/pkg/updater/restriction/pods_restriction_factory.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -348,5 +348,10 @@ func (s *singleGroupStats) isPodDisruptable() bool {
348348

349349
// isInPlaceUpdating checks whether or not the given pod is currently in the middle of an in-place update
350350
func isInPlaceUpdating(podToCheck *apiv1.Pod) bool {
351-
return podToCheck.Status.Resize != ""
351+
for _, c := range podToCheck.Status.Conditions {
352+
if c.Type == apiv1.PodResizePending || c.Type == apiv1.PodResizeInProgress {
353+
return c.Status == apiv1.ConditionTrue
354+
}
355+
}
356+
return false
352357
}

‎vertical-pod-autoscaler/pkg/updater/restriction/pods_restriction_factory_test.go‎

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -342,12 +342,23 @@ func TestDisruptReplicatedByController(t *testing.T) {
342342
vpa: getIPORVpa(),
343343
pods: []podWithExpectations{
344344
{
345-
pod: generatePod().WithResizeStatus(apiv1.PodResizeStatusInfeasible).Get(),
345+
pod: generatePod().WithPodConditions([]apiv1.PodCondition{
346+
{
347+
Type: apiv1.PodResizePending,
348+
Status: apiv1.ConditionTrue,
349+
Reason: apiv1.PodReasonInfeasible,
350+
},
351+
}).Get(),
346352
canInPlaceUpdate: utils.InPlaceEvict,
347353
inPlaceUpdateSuccess: false,
348354
},
349355
{
350-
pod: generatePod().WithResizeStatus(apiv1.PodResizeStatusInProgress).Get(),
356+
pod: generatePod().WithPodConditions([]apiv1.PodCondition{
357+
{
358+
Type: apiv1.PodResizeInProgress,
359+
Status: apiv1.ConditionTrue,
360+
},
361+
}).Get(),
351362
canInPlaceUpdate: utils.InPlaceDeferred,
352363
inPlaceUpdateSuccess: false,
353364
},
@@ -400,12 +411,24 @@ func TestDisruptReplicatedByController(t *testing.T) {
400411
evictionSuccess: true,
401412
},
402413
{
403-
pod: generatePod().WithResizeStatus(apiv1.PodResizeStatusInfeasible).Get(),
414+
pod: generatePod().WithPodConditions([]apiv1.PodCondition{
415+
{
416+
Type: apiv1.PodResizePending,
417+
Status: apiv1.ConditionTrue,
418+
Reason: apiv1.PodReasonInfeasible,
419+
},
420+
}).Get(),
404421
canEvict: true,
405422
evictionSuccess: false,
406423
},
407424
{
408-
pod: generatePod().WithResizeStatus(apiv1.PodResizeStatusInfeasible).Get(),
425+
pod: generatePod().WithPodConditions([]apiv1.PodCondition{
426+
{
427+
Type: apiv1.PodResizePending,
428+
Status: apiv1.ConditionTrue,
429+
Reason: apiv1.PodReasonInfeasible,
430+
},
431+
}).Get(),
409432
canEvict: true,
410433
evictionSuccess: false,
411434
},
@@ -423,7 +446,13 @@ func TestDisruptReplicatedByController(t *testing.T) {
423446
evictionSuccess: false,
424447
},
425448
{
426-
pod: generatePod().WithResizeStatus(apiv1.PodResizeStatusInfeasible).Get(),
449+
pod: generatePod().WithPodConditions([]apiv1.PodCondition{
450+
{
451+
Type: apiv1.PodResizePending,
452+
Status: apiv1.ConditionTrue,
453+
Reason: apiv1.PodReasonInfeasible,
454+
},
455+
}).Get(),
427456
canEvict: false,
428457
evictionSuccess: false,
429458
},
@@ -446,7 +475,13 @@ func TestDisruptReplicatedByController(t *testing.T) {
446475
evictionSuccess: true,
447476
},
448477
{
449-
pod: generatePod().WithResizeStatus(apiv1.PodResizeStatusInfeasible).Get(),
478+
pod: generatePod().WithPodConditions([]apiv1.PodCondition{
479+
{
480+
Type: apiv1.PodResizePending,
481+
Status: apiv1.ConditionTrue,
482+
Reason: apiv1.PodReasonInfeasible,
483+
},
484+
}).Get(),
450485
canEvict: true,
451486
evictionSuccess: true,
452487
},
@@ -477,25 +512,25 @@ func TestDisruptReplicatedByController(t *testing.T) {
477512
updateMode := vpa_api_util.GetUpdateMode(testCase.vpa)
478513
for i, p := range testCase.pods {
479514
if updateMode == vpa_types.UpdateModeInPlaceOrRecreate {
480-
assert.Equalf(t, p.canInPlaceUpdate, inplace.CanInPlaceUpdate(p.pod), "TC %v - unexpected CanInPlaceUpdate result for pod-%v %#v", testCase.name, i, p.pod)
515+
assert.Equalf(t, p.canInPlaceUpdate, inplace.CanInPlaceUpdate(p.pod), "unexpected CanInPlaceUpdate result for pod-%v %#v", testCase.name, i, p.pod)
481516
} else {
482-
assert.Equalf(t, p.canEvict, eviction.CanEvict(p.pod), "TC %v - unexpected CanEvict result for pod-%v %#v", testCase.name, i, p.pod)
517+
assert.Equalf(t, p.canEvict, eviction.CanEvict(p.pod), "unexpected CanEvict result for pod-%v %#v", i, p.pod)
483518
}
484519
}
485520
for i, p := range testCase.pods {
486521
if updateMode == vpa_types.UpdateModeInPlaceOrRecreate {
487522
err := inplace.InPlaceUpdate(p.pod, testCase.vpa, test.FakeEventRecorder())
488523
if p.inPlaceUpdateSuccess {
489-
assert.NoErrorf(t, err, "TC %v - unexpected InPlaceUpdate result for pod-%v %#v", testCase.name, i, p.pod)
524+
assert.NoErrorf(t, err, "unexpected InPlaceUpdate result for pod-%v %#v", i, p.pod)
490525
} else {
491-
assert.Errorf(t, err, "TC %v - unexpected InPlaceUpdate result for pod-%v %#v", testCase.name, i, p.pod)
526+
assert.Errorf(t, err, "unexpected InPlaceUpdate result for pod-%v %#v", i, p.pod)
492527
}
493528
} else {
494529
err := eviction.Evict(p.pod, testCase.vpa, test.FakeEventRecorder())
495530
if p.evictionSuccess {
496-
assert.NoErrorf(t, err, "TC %v - unexpected Evict result for pod-%v %#v", testCase.name, i, p.pod)
531+
assert.NoErrorf(t, err, "unexpected Evict result for pod-%v %#v", i, p.pod)
497532
} else {
498-
assert.Errorf(t, err, "TC %v - unexpected Evict result for pod-%v %#v", testCase.name, i, p.pod)
533+
assert.Errorf(t, err, "unexpected Evict result for pod-%v %#v", i, p.pod)
499534
}
500535
}
501536
}

‎vertical-pod-autoscaler/pkg/updater/utils/types.go‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@ limitations under the License.
1616

1717
package utils
1818

19+
import (
20+
apiv1 "k8s.io/api/core/v1"
21+
)
22+
1923
// InPlaceDecision is the type of decision that can be made for a pod.
2024
type InPlaceDecision string
2125

@@ -27,3 +31,13 @@ const (
2731
// InPlaceEvict means we will attempt to evict the pod.
2832
InPlaceEvict InPlaceDecision = "InPlaceEvict"
2933
)
34+
35+
// GetPodCondition will get Pod's condition.
36+
func GetPodCondition(pod *apiv1.Pod, conditionType apiv1.PodConditionType) (apiv1.PodCondition, bool) {
37+
for _, cond := range pod.Status.Conditions {
38+
if cond.Type == conditionType {
39+
return cond, true
40+
}
41+
}
42+
return apiv1.PodCondition{}, false
43+
}

‎vertical-pod-autoscaler/pkg/utils/test/test_pod.go‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ type PodBuilder interface {
3333
WithAnnotations(annotations map[string]string) PodBuilder
3434
WithPhase(phase apiv1.PodPhase) PodBuilder
3535
WithQOSClass(class apiv1.PodQOSClass) PodBuilder
36-
WithResizeStatus(resizeStatus apiv1.PodResizeStatus) PodBuilder
36+
WithPodConditions(conditions []apiv1.PodCondition) PodBuilder
3737
Get() *apiv1.Pod
3838
}
3939

@@ -57,7 +57,7 @@ type podBuilderImpl struct {
5757
containerStatuses []apiv1.ContainerStatus
5858
initContainerStatuses []apiv1.ContainerStatus
5959
qosClass apiv1.PodQOSClass
60-
resizeStatus apiv1.PodResizeStatus
60+
conditions []apiv1.PodCondition
6161
}
6262

6363
func (pb *podBuilderImpl) WithLabels(labels map[string]string) PodBuilder {
@@ -121,9 +121,9 @@ func (pb *podBuilderImpl) WithQOSClass(class apiv1.PodQOSClass) PodBuilder {
121121
return &r
122122
}
123123

124-
func (pb *podBuilderImpl) WithResizeStatus(resizeStatus apiv1.PodResizeStatus) PodBuilder {
124+
func (pb *podBuilderImpl) WithPodConditions(conditions []apiv1.PodCondition) PodBuilder {
125125
r := *pb
126-
r.resizeStatus = resizeStatus
126+
r.conditions = conditions
127127
return &r
128128
}
129129

@@ -141,7 +141,8 @@ func (pb *podBuilderImpl) Get() *apiv1.Pod {
141141
InitContainers: pb.initContainers,
142142
},
143143
Status: apiv1.PodStatus{
144-
StartTime: &startTime,
144+
StartTime: &startTime,
145+
Conditions: pb.conditions,
145146
},
146147
}
147148

@@ -171,10 +172,6 @@ func (pb *podBuilderImpl) Get() *apiv1.Pod {
171172
if pb.qosClass != "" {
172173
pod.Status.QOSClass = pb.qosClass
173174
}
174-
if pb.resizeStatus != "" {
175-
pod.Status.Resize = pb.resizeStatus
176-
}
177-
178175
if pb.containerStatuses != nil {
179176
pod.Status.ContainerStatuses = pb.containerStatuses
180177
}

0 commit comments

Comments
 (0)