From a0cf017d5c7eac17ac6e1f9e889a8b53682c0d8a Mon Sep 17 00:00:00 2001 From: Max Cao Date: Fri, 21 Mar 2025 18:02:24 -0700 Subject: [PATCH] VPA: Allow admission-controller to validate in-place spec Only allow VPA objects with InPlaceOrRecreate update mode to be created if InPlaceOrRecreate feature gate is enabled. If a VPA object already exists with this mode on, and the feature gate is disabled, this prevents further objects to be created with InPlaceOrRecreate, but this does not prevent the existing InPlaceOrRecreate VPA objects with from being modified. Signed-off-by: Max Cao --- .../resource/vpa/handler.go | 13 +++-- .../resource/vpa/handler_test.go | 52 +++++++++++++++++-- 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go b/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go index 6888efac1aa..b553a236f9d 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler.go @@ -30,15 +30,17 @@ import ( "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource" vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/features" "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/metrics/admission" ) 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.UpdateModeOff: struct{}{}, + vpa_types.UpdateModeInitial: struct{}{}, + vpa_types.UpdateModeRecreate: struct{}{}, + vpa_types.UpdateModeAuto: struct{}{}, + vpa_types.UpdateModeInPlaceOrRecreate: struct{}{}, } possibleScalingModes = map[vpa_types.ContainerScalingMode]interface{}{ @@ -121,6 +123,9 @@ func ValidateVPA(vpa *vpa_types.VerticalPodAutoscaler, isCreate bool) error { if _, found := possibleUpdateModes[*mode]; !found { return fmt.Errorf("unexpected UpdateMode value %s", *mode) } + if (*mode == vpa_types.UpdateModeInPlaceOrRecreate) && !features.Enabled(features.InPlaceOrRecreate) && isCreate { + return fmt.Errorf("in order to use UpdateMode %s, you must enable feature gate %s in the admission-controller args", vpa_types.UpdateModeInPlaceOrRecreate, features.InPlaceOrRecreate) + } if minReplicas := vpa.Spec.UpdatePolicy.MinReplicas; minReplicas != nil && *minReplicas <= 0 { return fmt.Errorf("MinReplicas has to be positive, got %v", *minReplicas) diff --git a/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler_test.go b/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler_test.go index 06efced1e0c..3f11995fccc 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler_test.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa/handler_test.go @@ -24,7 +24,10 @@ import ( apiv1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/resource" + featuregatetesting "k8s.io/component-base/featuregate/testing" + vpa_types "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1" + "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/features" ) const ( @@ -42,11 +45,13 @@ func TestValidateVPA(t *testing.T) { validScalingMode := vpa_types.ContainerScalingModeAuto scalingModeOff := vpa_types.ContainerScalingModeOff controlledValuesRequestsAndLimits := vpa_types.ContainerControlledValuesRequestsAndLimits + inPlaceOrRecreateUpdateMode := vpa_types.UpdateModeInPlaceOrRecreate tests := []struct { - name string - vpa vpa_types.VerticalPodAutoscaler - isCreate bool - expectError error + name string + vpa vpa_types.VerticalPodAutoscaler + isCreate bool + expectError error + inPlaceOrRecreateFeatureGateDisabled bool }{ { name: "empty update", @@ -78,6 +83,42 @@ func TestValidateVPA(t *testing.T) { }, expectError: fmt.Errorf("unexpected UpdateMode value bad"), }, + { + name: "creating VPA with InPlaceOrRecreate update mode not allowed by disabled feature gate", + vpa: vpa_types.VerticalPodAutoscaler{ + Spec: vpa_types.VerticalPodAutoscalerSpec{ + UpdatePolicy: &vpa_types.PodUpdatePolicy{ + UpdateMode: &inPlaceOrRecreateUpdateMode, + }, + }, + }, + isCreate: true, + inPlaceOrRecreateFeatureGateDisabled: true, + expectError: fmt.Errorf("in order to use UpdateMode %s, you must enable feature gate %s in the admission-controller args", vpa_types.UpdateModeInPlaceOrRecreate, features.InPlaceOrRecreate), + }, + { + name: "updating VPA with InPlaceOrRecreate update mode allowed by disabled feature gate", + vpa: vpa_types.VerticalPodAutoscaler{ + Spec: vpa_types.VerticalPodAutoscalerSpec{ + UpdatePolicy: &vpa_types.PodUpdatePolicy{ + UpdateMode: &inPlaceOrRecreateUpdateMode, + }, + }, + }, + isCreate: false, + inPlaceOrRecreateFeatureGateDisabled: true, + expectError: nil, + }, + { + name: "InPlaceOrRecreate update mode enabled by feature gate", + vpa: vpa_types.VerticalPodAutoscaler{ + Spec: vpa_types.VerticalPodAutoscalerSpec{ + UpdatePolicy: &vpa_types.PodUpdatePolicy{ + UpdateMode: &inPlaceOrRecreateUpdateMode, + }, + }, + }, + }, { name: "zero minReplicas", vpa: vpa_types.VerticalPodAutoscaler{ @@ -282,6 +323,9 @@ func TestValidateVPA(t *testing.T) { } for _, tc := range tests { t.Run(fmt.Sprintf("test case: %s", tc.name), func(t *testing.T) { + if !tc.inPlaceOrRecreateFeatureGateDisabled { + featuregatetesting.SetFeatureGateDuringTest(t, features.MutableFeatureGate, features.InPlaceOrRecreate, true) + } err := ValidateVPA(&tc.vpa, tc.isCreate) if tc.expectError == nil { assert.NoError(t, err)