Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,6 @@ import (
)

var (
possibleUpdateModes = map[vpa_types.UpdateMode]interface{}{
vpa_types.UpdateModeOff: struct{}{},
vpa_types.UpdateModeInitial: struct{}{},
vpa_types.UpdateModeRecreate: struct{}{},
vpa_types.UpdateModeAuto: struct{}{},
vpa_types.UpdateModeInPlaceOrRecreate: struct{}{},
}

possibleScalingModes = map[vpa_types.ContainerScalingMode]interface{}{
vpa_types.ContainerScalingModeAuto: struct{}{},
vpa_types.ContainerScalingModeOff: struct{}{},
Expand Down Expand Up @@ -120,7 +112,7 @@ func ValidateVPA(vpa *vpa_types.VerticalPodAutoscaler, isCreate bool) error {
if mode == nil {
return fmt.Errorf("updateMode is required if UpdatePolicy is used")
}
if _, found := possibleUpdateModes[*mode]; !found {
if _, found := vpa_types.GetUpdateModes()[*mode]; !found {
return fmt.Errorf("unexpected UpdateMode value %s", *mode)
}
if (*mode == vpa_types.UpdateModeInPlaceOrRecreate) && !features.Enabled(features.InPlaceOrRecreate) && isCreate {
Expand Down
11 changes: 11 additions & 0 deletions vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,17 @@ const (
UpdateModeInPlaceOrRecreate UpdateMode = "InPlaceOrRecreate"
)

// GetUpdateModes returns all supported UpdateModes
func GetUpdateModes() map[UpdateMode]any {
return map[UpdateMode]any{
UpdateModeOff: nil,
UpdateModeInitial: nil,
UpdateModeRecreate: nil,
UpdateModeAuto: nil,
UpdateModeInPlaceOrRecreate: nil,
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at kubernetes, it seems that the pattern is that functions like this live in a helpers.go file.
Could you move it there?


// PodResourcePolicy controls how autoscaler computes the recommended resources
// for containers belonging to the pod. There can be at most one entry for every
// named container and optionally a single wildcard entry with `containerName` = '*',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,6 @@ const (
metricsNamespace = metrics.TopMetricsNamespace + "recommender"
)

var (
// TODO: unify this list with the types defined in the VPA handler to avoid
// drift if one file is changed and the other one is missed.
modes = []string{
string(vpa_types.UpdateModeOff),
string(vpa_types.UpdateModeInitial),
string(vpa_types.UpdateModeRecreate),
string(vpa_types.UpdateModeAuto),
}
)

type apiVersion string

const (
Expand Down Expand Up @@ -156,13 +145,13 @@ func NewObjectCounter() *ObjectCounter {
}

// initialize with empty data so we can clean stale gauge values in Observe
for _, m := range modes {
for m := range vpa_types.GetUpdateModes() {
for _, h := range []bool{false, true} {
for _, api := range []apiVersion{v1beta1, v1beta2, v1} {
for _, mp := range []bool{false, true} {
for _, uc := range []bool{false, true} {
obj.cnt[objectCounterKey{
mode: m,
mode: string(m),
has: h,
apiVersion: api,
matchesPods: mp,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ func TestObjectCounter(t *testing.T) {
updateModeInitial := vpa_types.UpdateModeInitial
updateModeRecreate := vpa_types.UpdateModeRecreate
updateModeAuto := vpa_types.UpdateModeAuto
updateModeInPlaceOrRecreate := vpa_types.UpdateModeInPlaceOrRecreate
// We verify that other update modes are handled correctly as validation
// may not happen if there are issues with the admission controller.
updateModeUserDefined := vpa_types.UpdateMode("userDefined")
Expand Down Expand Up @@ -146,6 +147,18 @@ func TestObjectCounter(t *testing.T) {
"api=v1,has_recommendation=false,matches_pods=true,unsupported_config=false,update_mode=Off,": 1,
},
},
{
name: "report update mode InPlaceOrRecreate",
add: []*model.Vpa{
{
APIVersion: "v1",
UpdateMode: &updateModeInPlaceOrRecreate,
},
},
wantMetrics: map[string]float64{
"api=v1,has_recommendation=false,matches_pods=true,unsupported_config=false,update_mode=InPlaceOrRecreate,": 1,
},
},
{
name: "report update mode user defined",
add: []*model.Vpa{
Expand Down Expand Up @@ -321,3 +334,61 @@ func labelsToKey(labels []*dto.LabelPair) string {
}
return key.String()
}

func TestObjectCounterResetsAllUpdateModes(t *testing.T) {
updatesModes := []vpa_types.UpdateMode{
vpa_types.UpdateModeOff,
vpa_types.UpdateModeInitial,
vpa_types.UpdateModeAuto,
vpa_types.UpdateModeRecreate,
vpa_types.UpdateModeInPlaceOrRecreate,
}

for _, mode := range updatesModes {
t.Run(string(mode), func(t *testing.T) {
t.Cleanup(func() {
vpaObjectCount.Reset()
})

key := "api=v1,has_recommendation=false,matches_pods=true,unsupported_config=false,update_mode=" + string(mode) + ","

// first loop add VPAs to increment the counter
counter1 := NewObjectCounter()
for range 3 {
vpa := model.Vpa{
APIVersion: "v1",
UpdateMode: &mode,
}
counter1.Add(&vpa)
}
counter1.Observe()
collectMetricsAndVerifyCount(t, key, 3)

// next loop no VPAs
counter2 := NewObjectCounter()
counter2.Observe()
collectMetricsAndVerifyCount(t, key, 0)
})
}
}

func collectMetricsAndVerifyCount(t *testing.T, key string, expectedCount float64) {
metrics := make(chan prometheus.Metric)
go func() {
vpaObjectCount.Collect(metrics)
close(metrics)
}()

liveMetrics := make(map[string]float64)
for metric := range metrics {
var metricProto dto.Metric
if err := metric.Write(&metricProto); err != nil {
t.Errorf("failed to write metric: %v", err)
}
liveMetrics[labelsToKey(metricProto.GetLabel())] = *metricProto.GetGauge().Value
}

if actualCount := liveMetrics[key]; actualCount != expectedCount {
t.Errorf("key=%s expectedCount=%v actualCount=%v", key, expectedCount, actualCount)
}
}
Loading