Merge pull request #7185 from adrianmoisey/allow-failurepolicy

VPA: Add option to set the webhook failurePolicy to Fail

Author: k8s-ci-robot

vertical-pod-autoscaler/README.md

@@ -24,6 +24,7 @@
   - [Using CPU management with static policy](#using-cpu-management-with-static-policy)
   - [Controlling eviction behavior based on scaling direction and resource](#controlling-eviction-behavior-based-on-scaling-direction-and-resource)
   - [Limiting which namespaces are used](#limiting-which-namespaces-are-used)
+  - [Setting the webhook failurePolicy](#setting-the-webhook-failurepolicy)
 - [Known limitations](#known-limitations)
 - [Related links](#related-links)
 
@@ -388,6 +389,11 @@ vpa-post-processor.kubernetes.io/{containerName}_integerCPU=true
 
 These options cannot be used together and are mutually exclusive. 
 
+ ### Setting the webhook failurePolicy
+
+ It is possible to set the failurePolicy of the webhook to `Fail` by passing `--webhook-failure-policy-fail=true` to the VPA admission controller.
+ Please use this option with caution as it may be possible to break Pod creation if there is a failure with the VPA.
+Using it in conjunction with `--ignored-vpa-object-namespaces=kube-system` or `--vpa-object-namespace` to reduce risk.
 
 # Known limitations
 

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

@@ -90,7 +90,7 @@ func configTLS(cfg certsConfig, minTlsVersion, ciphers string, stop <-chan struc
 
 // register this webhook admission controller with the kube-apiserver
 // by creating MutatingWebhookConfiguration.
-func selfRegistration(clientset kubernetes.Interface, caCert []byte, webHookDelay time.Duration, namespace, serviceName, url string, registerByURL bool, timeoutSeconds int32, selectedNamespace string, ignoredNamespaces []string) {
+func selfRegistration(clientset kubernetes.Interface, caCert []byte, webHookDelay time.Duration, namespace, serviceName, url string, registerByURL bool, timeoutSeconds int32, selectedNamespace string, ignoredNamespaces []string, webHookFailurePolicy bool) {
 	time.Sleep(webHookDelay)
 	client := clientset.AdmissionregistrationV1().MutatingWebhookConfigurations()
 	_, err := client.Get(context.TODO(), webhookConfigName, metav1.GetOptions{})
@@ -109,7 +109,14 @@ func selfRegistration(clientset kubernetes.Interface, caCert []byte, webHookDela
 		RegisterClientConfig.URL = &url
 	}
 	sideEffects := admissionregistration.SideEffectClassNone
-	failurePolicy := admissionregistration.Ignore
+
+	var failurePolicy admissionregistration.FailurePolicyType
+	if webHookFailurePolicy {
+		failurePolicy = admissionregistration.Fail
+	} else {
+		failurePolicy = admissionregistration.Ignore
+	}
+
 	RegisterClientConfig.CABundle = caCert
 
 	var namespaceSelector metav1.LabelSelector

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

@@ -40,7 +40,7 @@ func TestSelfRegistrationBase(t *testing.T) {
 	selectedNamespace := ""
 	ignoredNamespaces := []string{}
 
-	selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces)
+	selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false)
 
 	webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations()
 	webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{})
@@ -83,7 +83,7 @@ func TestSelfRegistrationWithURL(t *testing.T) {
 	selectedNamespace := ""
 	ignoredNamespaces := []string{}
 
-	selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces)
+	selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false)
 
 	webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations()
 	webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{})
@@ -111,7 +111,7 @@ func TestSelfRegistrationWithOutURL(t *testing.T) {
 	selectedNamespace := ""
 	ignoredNamespaces := []string{}
 
-	selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces)
+	selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false)
 
 	webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations()
 	webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{})
@@ -141,7 +141,7 @@ func TestSelfRegistrationWithIgnoredNamespaces(t *testing.T) {
 	selectedNamespace := ""
 	ignoredNamespaces := []string{"test"}
 
-	selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces)
+	selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false)
 
 	webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations()
 	webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{})
@@ -172,7 +172,7 @@ func TestSelfRegistrationWithSelectedNamespaces(t *testing.T) {
 	selectedNamespace := "test"
 	ignoredNamespaces := []string{}
 
-	selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces)
+	selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false)
 
 	webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations()
 	webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{})
@@ -190,3 +190,57 @@ func TestSelfRegistrationWithSelectedNamespaces(t *testing.T) {
 	assert.Equal(t, matchExpression.Operator, metav1.LabelSelectorOpIn, "expected namespace operator to be OpIn")
 	assert.Equal(t, matchExpression.Values, []string{selectedNamespace}, "expected namespace selector match expression to be equal")
 }
+
+func TestSelfRegistrationWithFailurePolicy(t *testing.T) {
+
+	testClientSet := fake.NewSimpleClientset()
+	caCert := []byte("fake")
+	webHookDelay := 0 * time.Second
+	namespace := "default"
+	serviceName := "vpa-service"
+	url := "http://example.com/"
+	registerByURL := false
+	timeoutSeconds := int32(32)
+	selectedNamespace := "test"
+	ignoredNamespaces := []string{}
+
+	selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, true)
+
+	webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations()
+	webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{})
+
+	assert.NoError(t, err, "expected no error fetching webhook configuration")
+
+	assert.Len(t, webhookConfig.Webhooks, 1, "expected one webhook configuration")
+	webhook := webhookConfig.Webhooks[0]
+
+	assert.NotNil(t, *webhook.FailurePolicy, "expected failurePolicy not to be nil")
+	assert.Equal(t, *webhook.FailurePolicy, admissionregistration.Fail, "expected failurePolicy to be Fail")
+}
+
+func TestSelfRegistrationWithOutFailurePolicy(t *testing.T) {
+
+	testClientSet := fake.NewSimpleClientset()
+	caCert := []byte("fake")
+	webHookDelay := 0 * time.Second
+	namespace := "default"
+	serviceName := "vpa-service"
+	url := "http://example.com/"
+	registerByURL := false
+	timeoutSeconds := int32(32)
+	selectedNamespace := "test"
+	ignoredNamespaces := []string{}
+
+	selfRegistration(testClientSet, caCert, webHookDelay, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespace, ignoredNamespaces, false)
+
+	webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations()
+	webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{})
+
+	assert.NoError(t, err, "expected no error fetching webhook configuration")
+
+	assert.Len(t, webhookConfig.Webhooks, 1, "expected one webhook configuration")
+	webhook := webhookConfig.Webhooks[0]
+
+	assert.NotNil(t, *webhook.FailurePolicy, "expected namespace selector not to be nil")
+	assert.Equal(t, *webhook.FailurePolicy, admissionregistration.Ignore, "expected failurePolicy to be Ignore")
+}

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

@@ -75,6 +75,7 @@ var (
 	webhookAddress             = flag.String("webhook-address", "", "Address under which webhook is registered. Used when registerByURL is set to true.")
 	webhookPort                = flag.String("webhook-port", "", "Server Port for Webhook")
 	webhookTimeout             = flag.Int("webhook-timeout-seconds", 30, "Timeout in seconds that the API server should wait for this webhook to respond before failing.")
+	webHookFailurePolicy       = flag.Bool("webhook-failure-policy-fail", false, "If set to true, will configure the admission webhook failurePolicy to \"Fail\". Use with caution.")
 	registerWebhook            = flag.Bool("register-webhook", true, "If set to true, admission webhook object will be created on start up to register with the API server.")
 	registerByURL              = flag.Bool("register-by-url", false, "If set to true, admission webhook will be registered by URL (webhookAddress:webhookPort) instead of by service name")
 	vpaObjectNamespace         = flag.String("vpa-object-namespace", apiv1.NamespaceAll, "Namespace to search for VPA objects. Empty means all namespaces will be used. Must not be used if ignored-vpa-object-namespaces is set.")
@@ -146,7 +147,7 @@ func main() {
 	ignoredNamespaces := strings.Split(*ignoredVpaObjectNamespaces, ",")
 	go func() {
 		if *registerWebhook {
-			selfRegistration(kubeClient, readFile(*certsConfiguration.clientCaFile), webHookDelay, namespace, *serviceName, url, *registerByURL, int32(*webhookTimeout), *vpaObjectNamespace, ignoredNamespaces)
+			selfRegistration(kubeClient, readFile(*certsConfiguration.clientCaFile), webHookDelay, namespace, *serviceName, url, *registerByURL, int32(*webhookTimeout), *vpaObjectNamespace, ignoredNamespaces, *webHookFailurePolicy)
 		}
 		// Start status updates after the webhook is initialized.
 		statusUpdater.Run(stopCh)