Skip to content

Commit 818639e

Browse files
cmtlyadrianmoisey
andauthored
fix: vpa_recommender_vpa_objects_count initialization (#8750)
* fix: ensure vpa_recommender_vpa_objects_count UpdateModeInPlaceOrRecreate is reset * move GetUpdateModes() to helpers.go * update copyright Co-authored-by: Adrian Moisey <[email protected]> --------- Co-authored-by: Adrian Moisey <[email protected]>
1 parent 5d19bdf commit 818639e

File tree

4 files changed

+102
-22
lines changed

4 files changed

+102
-22
lines changed

vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,14 +34,6 @@ import (
3434
)
3535

3636
var (
37-
possibleUpdateModes = map[vpa_types.UpdateMode]interface{}{
38-
vpa_types.UpdateModeOff: struct{}{},
39-
vpa_types.UpdateModeInitial: struct{}{},
40-
vpa_types.UpdateModeRecreate: struct{}{},
41-
vpa_types.UpdateModeAuto: struct{}{},
42-
vpa_types.UpdateModeInPlaceOrRecreate: struct{}{},
43-
}
44-
4537
possibleScalingModes = map[vpa_types.ContainerScalingMode]interface{}{
4638
vpa_types.ContainerScalingModeAuto: struct{}{},
4739
vpa_types.ContainerScalingModeOff: struct{}{},
@@ -120,7 +112,7 @@ func ValidateVPA(vpa *vpa_types.VerticalPodAutoscaler, isCreate bool) error {
120112
if mode == nil {
121113
return fmt.Errorf("updateMode is required if UpdatePolicy is used")
122114
}
123-
if _, found := possibleUpdateModes[*mode]; !found {
115+
if _, found := vpa_types.GetUpdateModes()[*mode]; !found {
124116
return fmt.Errorf("unexpected UpdateMode value %s", *mode)
125117
}
126118
if (*mode == vpa_types.UpdateModeInPlaceOrRecreate) && !features.Enabled(features.InPlaceOrRecreate) && isCreate {
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/*
2+
Copyright 2025 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package v1
18+
19+
// GetUpdateModes returns all supported UpdateModes
20+
func GetUpdateModes() map[UpdateMode]any {
21+
return map[UpdateMode]any{
22+
UpdateModeOff: nil,
23+
UpdateModeInitial: nil,
24+
UpdateModeRecreate: nil,
25+
UpdateModeAuto: nil,
26+
UpdateModeInPlaceOrRecreate: nil,
27+
}
28+
}

vertical-pod-autoscaler/pkg/utils/metrics/recommender/recommender.go

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,6 @@ const (
3636
metricsNamespace = metrics.TopMetricsNamespace + "recommender"
3737
)
3838

39-
var (
40-
// TODO: unify this list with the types defined in the VPA handler to avoid
41-
// drift if one file is changed and the other one is missed.
42-
modes = []string{
43-
string(vpa_types.UpdateModeOff),
44-
string(vpa_types.UpdateModeInitial),
45-
string(vpa_types.UpdateModeRecreate),
46-
string(vpa_types.UpdateModeAuto),
47-
}
48-
)
49-
5039
type apiVersion string
5140

5241
const (
@@ -156,13 +145,13 @@ func NewObjectCounter() *ObjectCounter {
156145
}
157146

158147
// initialize with empty data so we can clean stale gauge values in Observe
159-
for _, m := range modes {
148+
for m := range vpa_types.GetUpdateModes() {
160149
for _, h := range []bool{false, true} {
161150
for _, api := range []apiVersion{v1beta1, v1beta2, v1} {
162151
for _, mp := range []bool{false, true} {
163152
for _, uc := range []bool{false, true} {
164153
obj.cnt[objectCounterKey{
165-
mode: m,
154+
mode: string(m),
166155
has: h,
167156
apiVersion: api,
168157
matchesPods: mp,

vertical-pod-autoscaler/pkg/utils/metrics/recommender/recommender_test.go

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ func TestObjectCounter(t *testing.T) {
3333
updateModeInitial := vpa_types.UpdateModeInitial
3434
updateModeRecreate := vpa_types.UpdateModeRecreate
3535
updateModeAuto := vpa_types.UpdateModeAuto
36+
updateModeInPlaceOrRecreate := vpa_types.UpdateModeInPlaceOrRecreate
3637
// We verify that other update modes are handled correctly as validation
3738
// may not happen if there are issues with the admission controller.
3839
updateModeUserDefined := vpa_types.UpdateMode("userDefined")
@@ -146,6 +147,18 @@ func TestObjectCounter(t *testing.T) {
146147
"api=v1,has_recommendation=false,matches_pods=true,unsupported_config=false,update_mode=Off,": 1,
147148
},
148149
},
150+
{
151+
name: "report update mode InPlaceOrRecreate",
152+
add: []*model.Vpa{
153+
{
154+
APIVersion: "v1",
155+
UpdateMode: &updateModeInPlaceOrRecreate,
156+
},
157+
},
158+
wantMetrics: map[string]float64{
159+
"api=v1,has_recommendation=false,matches_pods=true,unsupported_config=false,update_mode=InPlaceOrRecreate,": 1,
160+
},
161+
},
149162
{
150163
name: "report update mode user defined",
151164
add: []*model.Vpa{
@@ -321,3 +334,61 @@ func labelsToKey(labels []*dto.LabelPair) string {
321334
}
322335
return key.String()
323336
}
337+
338+
func TestObjectCounterResetsAllUpdateModes(t *testing.T) {
339+
updatesModes := []vpa_types.UpdateMode{
340+
vpa_types.UpdateModeOff,
341+
vpa_types.UpdateModeInitial,
342+
vpa_types.UpdateModeAuto,
343+
vpa_types.UpdateModeRecreate,
344+
vpa_types.UpdateModeInPlaceOrRecreate,
345+
}
346+
347+
for _, mode := range updatesModes {
348+
t.Run(string(mode), func(t *testing.T) {
349+
t.Cleanup(func() {
350+
vpaObjectCount.Reset()
351+
})
352+
353+
key := "api=v1,has_recommendation=false,matches_pods=true,unsupported_config=false,update_mode=" + string(mode) + ","
354+
355+
// first loop add VPAs to increment the counter
356+
counter1 := NewObjectCounter()
357+
for range 3 {
358+
vpa := model.Vpa{
359+
APIVersion: "v1",
360+
UpdateMode: &mode,
361+
}
362+
counter1.Add(&vpa)
363+
}
364+
counter1.Observe()
365+
collectMetricsAndVerifyCount(t, key, 3)
366+
367+
// next loop no VPAs
368+
counter2 := NewObjectCounter()
369+
counter2.Observe()
370+
collectMetricsAndVerifyCount(t, key, 0)
371+
})
372+
}
373+
}
374+
375+
func collectMetricsAndVerifyCount(t *testing.T, key string, expectedCount float64) {
376+
metrics := make(chan prometheus.Metric)
377+
go func() {
378+
vpaObjectCount.Collect(metrics)
379+
close(metrics)
380+
}()
381+
382+
liveMetrics := make(map[string]float64)
383+
for metric := range metrics {
384+
var metricProto dto.Metric
385+
if err := metric.Write(&metricProto); err != nil {
386+
t.Errorf("failed to write metric: %v", err)
387+
}
388+
liveMetrics[labelsToKey(metricProto.GetLabel())] = *metricProto.GetGauge().Value
389+
}
390+
391+
if actualCount := liveMetrics[key]; actualCount != expectedCount {
392+
t.Errorf("key=%s expectedCount=%v actualCount=%v", key, expectedCount, actualCount)
393+
}
394+
}

0 commit comments

Comments
 (0)