VPA: add InPlaceVerticalScaling feature gate to admission-controller

Adds logic to vpa admission webhook to deny requests creating VPAs with InPlaceOrRecreate update mode without enabling feature gate.
Also adds admission e2e logic to wait for the vpa-webhook to be registered before starting the test.

Signed-off-by: Max Cao <[email protected]>

Author: maxcao13

vertical-pod-autoscaler/deploy/admission-controller-deployment.yaml

@@ -47,15 +47,3 @@ spec:
         - name: tls-certs
           secret:
             secretName: vpa-tls-certs
----
-apiVersion: v1
-kind: Service
-metadata:
-  name: vpa-webhook
-  namespace: kube-system
-spec:
-  ports:
-    - port: 443
-      targetPort: 8000
-  selector:
-    app: vpa-admission-controller

vertical-pod-autoscaler/deploy/admission-controller-service.yaml

@@ -0,0 +1,11 @@
+apiVersion: v1
+kind: Service
+metadata:
+  name: vpa-webhook
+  namespace: kube-system
+spec:
+  ports:
+    - port: 443
+      targetPort: 8000
+  selector:
+    app: vpa-admission-controller

vertical-pod-autoscaler/docs/flags.md

@@ -12,6 +12,7 @@ This document is auto-generated from the flag definitions in the VPA admission-c
 | `--address` | ":8944" |                         The address to expose Prometheus metrics. |
 | `--alsologtostderr` |  |                        log to standard error as well as files (no effect when -logtostderr=true) |
 | `--client-ca-file` | "/etc/tls-certs/caCert.pem" |                  Path to CA PEM file. |
+| `--feature-gates` |  |            A set of key=value pairs that describe feature gates for alpha/experimental features. Options are: |
 | `--ignored-vpa-object-namespaces` |  |   A comma-separated list of namespaces to ignore when searching for VPA objects. Leave empty to avoid ignoring any namespaces. These namespaces will not be cleaned by the garbage collector. |
 | `--kube-api-burst` | 10 |                   QPS burst limit when making requests to Kubernetes apiserver |
 | `--kube-api-qps` | 5 |                     QPS limit when making requests to Kubernetes apiserver |

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

@@ -521,7 +521,7 @@ var _ = ActuationSuiteE2eDescribe("InPlaceVerticalScaling", func() {
 	f.NamespacePodSecurityEnforceLevel = podsecurity.LevelBaseline
 
 	ginkgo.BeforeEach(func() {
-		checkInPlaceVerticalScalingTestsEnabled(f)
+		checkInPlaceVerticalScalingTestsEnabled(f, true, true)
 	})
 
 	ginkgo.It("still applies recommendations on restart when update mode is InPlaceOrRecreate", func() {

vertical-pod-autoscaler/e2e/v1/admission_controller.go

@@ -37,10 +37,19 @@ import (
 	"github.com/onsi/gomega"
 )
 
+const (
+	webhookConfigName = "vpa-webhook-config"
+	webhookName       = "vpa.k8s.io"
+)
+
 var _ = AdmissionControllerE2eDescribe("Admission-controller", func() {
 	f := framework.NewDefaultFramework("vertical-pod-autoscaling")
 	f.NamespacePodSecurityEnforceLevel = podsecurity.LevelBaseline
 
+	ginkgo.BeforeEach(func() {
+		waitForVpaWebhookRegistration(f)
+	})
+
 	ginkgo.It("starts pods with new recommended request", func() {
 		d := NewHamsterDeploymentWithResources(f, ParseQuantityOrDie("100m") /*cpu*/, ParseQuantityOrDie("100Mi") /*memory*/)
 
@@ -909,6 +918,8 @@ var _ = AdmissionControllerE2eDescribe("Admission-controller", func() {
 	})
 
 	ginkgo.It("starts pods with new recommended request with InPlaceOrRecreate mode", func() {
+		checkInPlaceVerticalScalingTestsEnabled(f, true, false)
+
 		d := NewHamsterDeploymentWithResources(f, ParseQuantityOrDie("100m") /*cpu*/, ParseQuantityOrDie("100Mi") /*memory*/)
 
 		ginkgo.By("Setting up a VPA CRD")
@@ -994,3 +1005,17 @@ func startDeploymentPods(f *framework.Framework, deployment *appsv1.Deployment)
 	gomega.Expect(err).NotTo(gomega.HaveOccurred(), "when listing pods after deployment resize")
 	return podList
 }
+
+func waitForVpaWebhookRegistration(f *framework.Framework) {
+	ginkgo.By("Waiting for VPA webhook registration")
+	gomega.Eventually(func() bool {
+		webhook, err := f.ClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations().Get(context.TODO(), webhookConfigName, metav1.GetOptions{})
+		if err != nil {
+			return false
+		}
+		if webhook != nil && len(webhook.Webhooks) > 0 && webhook.Webhooks[0].Name == webhookName {
+			return true
+		}
+		return false
+	}, 3*time.Minute, 5*time.Second).Should(gomega.BeTrue(), "Webhook was not registered in the cluster")
+}

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

@@ -618,7 +618,7 @@ func WaitForPodsUpdatedWithoutEviction(f *framework.Framework, initialPods *apiv
 // checkInPlaceVerticalScalingTestsEnabled check for enabled feature gates in the cluster used for the
 // InPlaceVerticalScaling VPA feature.
 // Use this in a "beforeEach" call before any suites that use InPlaceVerticalScaling featuregate.
-func checkInPlaceVerticalScalingTestsEnabled(f *framework.Framework) {
+func checkInPlaceVerticalScalingTestsEnabled(f *framework.Framework, checkAdmission, checkUpdater bool) {
 	ginkgo.By("Checking InPlacePodVerticalScaling cluster feature gate is on")
 
 	podList, err := f.ClientSet.CoreV1().Pods("kube-system").List(context.TODO(), metav1.ListOptions{
@@ -633,16 +633,32 @@ func checkInPlaceVerticalScalingTestsEnabled(f *framework.Framework) {
 		ginkgo.Skip("Skipping suite: InPlacePodVerticalScaling feature gate is not enabled on the cluster level")
 	}
 
-	ginkgo.By("Checking InPlaceVerticalScaling VPA feature gate is on")
+	if checkUpdater {
+		ginkgo.By("Checking InPlaceVerticalScaling VPA feature gate is on for updater")
 
-	deploy, err := f.ClientSet.AppsV1().Deployments(VpaNamespace).Get(context.TODO(), "vpa-updater", metav1.GetOptions{})
-	gomega.Expect(err).NotTo(gomega.HaveOccurred())
-	gomega.Expect(deploy.Spec.Template.Spec.Containers).To(gomega.HaveLen(1))
-	vpaUpdaterPod := deploy.Spec.Template.Spec.Containers[0]
-	gomega.Expect(vpaUpdaterPod.Name).To(gomega.Equal("updater"))
-	if !anyContainsSubstring(vpaUpdaterPod.Args, string(features.InPlaceVerticalScaling)) {
-		ginkgo.Skip("Skipping suite: InPlaceVerticalScaling feature gate is not enabled on the VPA level")
+		deploy, err := f.ClientSet.AppsV1().Deployments(VpaNamespace).Get(context.TODO(), "vpa-updater", metav1.GetOptions{})
+		gomega.Expect(err).NotTo(gomega.HaveOccurred())
+		gomega.Expect(deploy.Spec.Template.Spec.Containers).To(gomega.HaveLen(1))
+		vpaUpdaterPod := deploy.Spec.Template.Spec.Containers[0]
+		gomega.Expect(vpaUpdaterPod.Name).To(gomega.Equal("updater"))
+		if !anyContainsSubstring(vpaUpdaterPod.Args, string(features.InPlaceVerticalScaling)) {
+			ginkgo.Skip("Skipping suite: InPlaceVerticalScaling feature gate is not enabled on the VPA updater")
+		}
 	}
+
+	if checkAdmission {
+		ginkgo.By("Checking InPlaceVerticalScaling VPA feature gate is on for admission controller")
+
+		deploy, err := f.ClientSet.AppsV1().Deployments(VpaNamespace).Get(context.TODO(), "vpa-admission-controller", metav1.GetOptions{})
+		gomega.Expect(err).NotTo(gomega.HaveOccurred())
+		gomega.Expect(deploy.Spec.Template.Spec.Containers).To(gomega.HaveLen(1))
+		vpaAdmissionPod := deploy.Spec.Template.Spec.Containers[0]
+		gomega.Expect(vpaAdmissionPod.Name).To(gomega.Equal("admission-controller"))
+		if !anyContainsSubstring(vpaAdmissionPod.Args, string(features.InPlaceVerticalScaling)) {
+			ginkgo.Skip("Skipping suite: InPlaceVerticalScaling feature gate is not enabled on VPA admission controller")
+		}
+	}
+
 }
 
 func anyContainsSubstring(arr []string, substr string) bool {

vertical-pod-autoscaler/e2e/v1/updater.go

@@ -145,7 +145,7 @@ var _ = UpdaterE2eDescribe("InPlaceVerticalScaling", func() {
 	f.NamespacePodSecurityEnforceLevel = podsecurity.LevelBaseline
 
 	ginkgo.BeforeEach(func() {
-		checkInPlaceVerticalScalingTestsEnabled(f)
+		checkInPlaceVerticalScalingTestsEnabled(f, true, true)
 	})
 
 	ginkgo.It("In-place update pods when Admission Controller status available", func() {

vertical-pod-autoscaler/hack/deploy-for-e2e-locally.sh

@@ -20,7 +20,7 @@ set -o pipefail
 
 SCRIPT_ROOT=$(dirname ${BASH_SOURCE})/..
 BASE_NAME=$(basename $0)
-FEATURE_GATES=${FEATURE_GATES:-""}
+FEATURE_GATES=${FEATURE_GATES:-""""}
 source "${SCRIPT_ROOT}/hack/lib/util.sh"
 
 ARCH=$(kube::util::host_arch)
@@ -87,6 +87,16 @@ for i in ${COMPONENTS}; do
   kind load docker-image ${REGISTRY}/vpa-${i}:${TAG}
 done
 
+apply_feature_gate() {
+  component=$1
+  if [ "${component}" == "admission-controller" ]; then
+    kubectl patch --type=json --local -p='[{"op": "add", "path": "/spec/template/spec/containers/0/args/-", "value": "--feature-gates='"${FEATURE_GATES}"'"}]' -o yaml -f -
+  elif [ "${component}" == "updater" ]; then
+    kubectl patch --type=json --local -p='[{"op": "add", "path": "/spec/template/spec/containers/0/args", "value": ["--feature-gates='"${FEATURE_GATES}"'"]}]' -o yaml -f -
+  else
+    cat # passthrough
+  fi
+}
 
 for i in ${COMPONENTS}; do
   if [ $i == recommender-externalmetrics ] ; then
@@ -97,19 +107,11 @@ for i in ${COMPONENTS}; do
      kubectl apply -f ${SCRIPT_ROOT}/hack/e2e/metrics-pump.yaml
      kubectl apply -f ${SCRIPT_ROOT}/hack/e2e/${i}-deployment.yaml
   else
-     REGISTRY=${REGISTRY} TAG=${TAG} ${SCRIPT_ROOT}/hack/vpa-process-yaml.sh  ${SCRIPT_ROOT}/deploy/${i}-deployment.yaml | kubectl apply -f -
+    REGISTRY=${REGISTRY} TAG=${TAG} ${SCRIPT_ROOT}/hack/vpa-process-yaml.sh ${SCRIPT_ROOT}/deploy/${i}-deployment.yaml | apply_feature_gate "$i" | kubectl apply -f -
+      if [ $i == admission-controller ] ; then
+        kubectl apply -f ${SCRIPT_ROOT}/deploy/admission-controller-service.yaml
+      fi
   fi
 done
 
-# TODO: come up with some sort of plan for feature gate enablement e2e testing purposes
-# only applies to updater for now
-
-# --feature-gates A set of key=value pairs that describe feature gates for alpha/experimental features. Options are:
-# InPlaceVerticalScaling=true|false (BETA - default=false)
-if [ -n "${FEATURE_GATES}" ]; then
-  echo "Enabling feature gates: ${FEATURE_GATES}"
-  kubectl -n kube-system patch deployment vpa-updater --type=json \
-  -p='[{"op": "add", "path": "/spec/template/spec/containers/0/args", "value": [
-    --feature-gates='"${FEATURE_GATES}"'
-  ]}]'
-fi
+exit

vertical-pod-autoscaler/pkg/admission-controller/main.go

@@ -24,6 +24,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/spf13/pflag"
 	"k8s.io/client-go/informers"
 	kube_client "k8s.io/client-go/kubernetes"
 	kube_flag "k8s.io/component-base/cli/flag"
@@ -36,6 +37,7 @@ import (
 	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource/pod/recommendation"
 	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/resource/vpa"
 	vpa_clientset "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/client/clientset/versioned"
+	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/features"
 	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target"
 	controllerfetcher "k8s.io/autoscaler/vertical-pod-autoscaler/pkg/target/controller_fetcher"
 	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/utils/limitrange"
@@ -81,6 +83,7 @@ func main() {
 	commonFlags := common.InitCommonFlags()
 	klog.InitFlags(nil)
 	common.InitLoggingFlags()
+	features.MutableFeatureGate.AddFlag(pflag.CommandLine)
 	kube_flag.InitFlags()
 	klog.V(1).InfoS("Starting Vertical Pod Autoscaler Admission Controller", "version", common.VerticalPodAutoscalerVersion())
 

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

@@ -30,6 +30,7 @@ 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"
 )
 
@@ -122,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.InPlaceVerticalScaling) {
+			return fmt.Errorf("in order to use UpdateMode %s, you must enable feature gate %s in the admission and updater args", vpa_types.UpdateModeInPlaceOrRecreate, features.InPlaceVerticalScaling)
+		}
 
 		if minReplicas := vpa.Spec.UpdatePolicy.MinReplicas; minReplicas != nil && *minReplicas <= 0 {
 			return fmt.Errorf("MinReplicas has to be positive, got %v", *minReplicas)

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,6 +45,8 @@ func TestValidateVPA(t *testing.T) {
 	validScalingMode := vpa_types.ContainerScalingModeAuto
 	scalingModeOff := vpa_types.ContainerScalingModeOff
 	controlledValuesRequestsAndLimits := vpa_types.ContainerControlledValuesRequestsAndLimits
+	inPlaceOrRecreateUpdateMode := vpa_types.UpdateModeInPlaceOrRecreate
+	inPlaceOrRecreateEnabledTestName := "InPlaceOrRecreate update mode enabled by feature gate"
 	tests := []struct {
 		name        string
 		vpa         vpa_types.VerticalPodAutoscaler
@@ -78,6 +83,27 @@ func TestValidateVPA(t *testing.T) {
 			},
 			expectError: fmt.Errorf("unexpected UpdateMode value bad"),
 		},
+		{
+			name: "InPlaceOrRecreate update mode not enabled by feature gate",
+			vpa: vpa_types.VerticalPodAutoscaler{
+				Spec: vpa_types.VerticalPodAutoscalerSpec{
+					UpdatePolicy: &vpa_types.PodUpdatePolicy{
+						UpdateMode: &inPlaceOrRecreateUpdateMode,
+					},
+				},
+			},
+			expectError: fmt.Errorf("in order to use UpdateMode %s, you must enable feature gate %s in the admission and updater args", vpa_types.UpdateModeInPlaceOrRecreate, features.InPlaceVerticalScaling),
+		},
+		{
+			name: inPlaceOrRecreateEnabledTestName,
+			vpa: vpa_types.VerticalPodAutoscaler{
+				Spec: vpa_types.VerticalPodAutoscalerSpec{
+					UpdatePolicy: &vpa_types.PodUpdatePolicy{
+						UpdateMode: &inPlaceOrRecreateUpdateMode,
+					},
+				},
+			},
+		},
 		{
 			name: "zero minReplicas",
 			vpa: vpa_types.VerticalPodAutoscaler{
@@ -282,6 +308,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.name == inPlaceOrRecreateEnabledTestName {
+				featuregatetesting.SetFeatureGateDuringTest(t, features.MutableFeatureGate, features.InPlaceVerticalScaling, true)
+			}
 			err := ValidateVPA(&tc.vpa, tc.isCreate)
 			if tc.expectError == nil {
 				assert.NoError(t, err)

vertical-pod-autoscaler/pkg/apis/autoscaling.k8s.io/v1/types.go

@@ -174,6 +174,8 @@ const (
 	// UpdateModeInPlaceOrRecreate means that autoscaler tries to assign resources in-place
 	// If this is not possible (e.g., resizing takes too long or is infeasible), it falls back to the
 	// "Recreate" update mode.
+	// Requires VPA level feature gate "InPlaceVerticalScaling" to be enabled
+	// on the admission and updater pods.
 	UpdateModeInPlaceOrRecreate UpdateMode = "InPlaceOrRecreate"
 )