VPA: Refactor how flags are set

vertical-pod-autoscaler/common/flags.go

@@ -35,18 +35,38 @@ type CommonFlags struct {
 	IgnoredVpaObjectNamespaces string
 }
 
+// DefaultCommonConfig returns the default values for common flags
+func DefaultCommonConfig() *CommonFlags {
+	return &CommonFlags{
+		KubeConfig:                 "",
+		KubeApiQps:                 50.0,
+		KubeApiBurst:               100.0,
+		EnableProfiling:            false,
+		VpaObjectNamespace:         corev1.NamespaceAll,
+		IgnoredVpaObjectNamespaces: "",
+	}
+}
+
 // InitCommonFlags initializes the common flags
 func InitCommonFlags() *CommonFlags {
-	cf := &CommonFlags{}
-	flag.StringVar(&cf.KubeConfig, "kubeconfig", "", "Path to a kubeconfig. Only required if out-of-cluster.")
-	flag.Float64Var(&cf.KubeApiQps, "kube-api-qps", 50.0, "QPS limit when making requests to Kubernetes apiserver")
-	flag.Float64Var(&cf.KubeApiBurst, "kube-api-burst", 100.0, "QPS burst limit when making requests to Kubernetes apiserver")
-	flag.BoolVar(&cf.EnableProfiling, "profiling", false, "Is debug/pprof endpoint enabled")
-	flag.StringVar(&cf.VpaObjectNamespace, "vpa-object-namespace", corev1.NamespaceAll, "Specifies the namespace to search for VPA objects. Leave empty to include all namespaces. If provided, the garbage collector will only clean this namespace.")
-	flag.StringVar(&cf.IgnoredVpaObjectNamespaces, "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.")
+	cf := DefaultCommonConfig()
+	flag.StringVar(&cf.KubeConfig, "kubeconfig", cf.KubeConfig, "Path to a kubeconfig. Only required if out-of-cluster.")
+	flag.Float64Var(&cf.KubeApiQps, "kube-api-qps", cf.KubeApiQps, "QPS limit when making requests to Kubernetes apiserver")
+	flag.Float64Var(&cf.KubeApiBurst, "kube-api-burst", cf.KubeApiBurst, "QPS burst limit when making requests to Kubernetes apiserver")
+	flag.BoolVar(&cf.EnableProfiling, "profiling", cf.EnableProfiling, "Is debug/pprof endpoint enabled")
+	flag.StringVar(&cf.VpaObjectNamespace, "vpa-object-namespace", cf.VpaObjectNamespace, "Specifies the namespace to search for VPA objects. Leave empty to include all namespaces. If provided, the garbage collector will only clean this namespace.")
+	flag.StringVar(&cf.IgnoredVpaObjectNamespaces, "ignored-vpa-object-namespaces", cf.IgnoredVpaObjectNamespaces, "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.")
 	return cf
 }
 
+// ValidateCommonConfig performs validation of the common flags
+func ValidateCommonConfig(config *CommonFlags) {
+	if len(config.VpaObjectNamespace) > 0 && len(config.IgnoredVpaObjectNamespaces) > 0 {
+		klog.ErrorS(nil, "--vpa-object-namespace and --ignored-vpa-object-namespaces are mutually exclusive and can't be set together.")
+		klog.FlushAndExit(klog.ExitFlushTimeout, 1)
+	}
+}
+
 // InitLoggingFlags initializes the logging flags
 func InitLoggingFlags() {
 	// Set the default log level to 4 (info)

vertical-pod-autoscaler/docs/flags.md

@@ -149,7 +149,7 @@ This document is auto-generated from the flag definitions in the VPA updater cod
 | `alsologtostderr` |  |  | log to standard error as well as files (no effect when -logtostderr=true) |
 | `evict-after-oom-threshold` |  |  10m0s | duration                              The default duration to evict pods that have OOMed in less than evict-after-oom-threshold since start.  |
 | `eviction-rate-burst` | int |  1 | Burst of pods that can be evicted.  |
-| `eviction-rate-limit` | float |  | Number of pods that can be evicted per seconds. A rate limit set to 0 or -1 will disable<br>the rate limiter. (default -1) |
+| `eviction-rate-limit` | float |  -1 | Number of pods that can be evicted per seconds. A rate limit set to 0 or -1 will disable the rate limiter.  |
 | `eviction-tolerance` | float |  0.5 | Fraction of replica count that can be evicted for update, if more than one pod can be evicted.  |
 | `feature-gates` | mapStringBool |  | A set of key=value pairs that describe feature gates for alpha/experimental features. Options are:<br>AllAlpha=true\|false (ALPHA - default=false)<br>AllBeta=true\|false (BETA - default=false)<br>CPUStartupBoost=true\|false (ALPHA - default=false)<br>PerVPAConfig=true\|false (ALPHA - default=false) |
 | `ignored-vpa-object-namespaces` | string |  | 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. |

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

@@ -32,11 +32,6 @@ import (
 	"k8s.io/klog/v2"
 )
 
-type certsConfig struct {
-	clientCaFile, tlsCertFile, tlsPrivateKey *string
-	reload                                   *bool
-}
-
 func readFile(filePath string) []byte {
 	res, err := os.ReadFile(filePath)
 	if err != nil {

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

@@ -28,6 +28,8 @@ import (
 	"k8s.io/client-go/kubernetes"
 	typedadmregv1 "k8s.io/client-go/kubernetes/typed/admissionregistration/v1"
 	"k8s.io/klog/v2"
+
+	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/admission-controller/config"
 )
 
 const (
@@ -36,7 +38,7 @@ const (
 )
 
 // MutatingWebhookConfigurationInterface
-func configTLS(cfg certsConfig, minTlsVersion, ciphers string, stop <-chan struct{}, mutatingWebhookClient typedadmregv1.MutatingWebhookConfigurationInterface) *tls.Config {
+func configTLS(cfg config.CertsConfig, minTlsVersion, ciphers string, stop <-chan struct{}, mutatingWebhookClient typedadmregv1.MutatingWebhookConfigurationInterface) *tls.Config {
 	var tlsVersion uint16
 	var ciphersuites []uint16
 	reverseCipherMap := make(map[string]uint16)
@@ -67,11 +69,11 @@ func configTLS(cfg certsConfig, minTlsVersion, ciphers string, stop <-chan struc
 		MinVersion:   tlsVersion,
 		CipherSuites: ciphersuites,
 	}
-	if *cfg.reload {
+	if cfg.Reload {
 		cr := certReloader{
-			tlsCertPath:           *cfg.tlsCertFile,
-			tlsKeyPath:            *cfg.tlsPrivateKey,
-			clientCaPath:          *cfg.clientCaFile,
+			tlsCertPath:           cfg.TlsCertFile,
+			tlsKeyPath:            cfg.TlsPrivateKey,
+			clientCaPath:          cfg.ClientCaFile,
 			mutatingWebhookClient: mutatingWebhookClient,
 		}
 		if err := cr.load(); err != nil {
@@ -82,7 +84,7 @@ func configTLS(cfg certsConfig, minTlsVersion, ciphers string, stop <-chan struc
 		}
 		config.GetCertificate = cr.getCertificate
 	} else {
-		cert, err := tls.LoadX509KeyPair(*cfg.tlsCertFile, *cfg.tlsPrivateKey)
+		cert, err := tls.LoadX509KeyPair(cfg.TlsCertFile, cfg.TlsPrivateKey)
 		if err != nil {
 			klog.Fatal(err)
 		}

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

@@ -0,0 +1,133 @@
+/*
+Copyright The Kubernetes Authors.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package config
+
+import (
+	"flag"
+	"os"
+
+	"github.com/spf13/pflag"
+	"k8s.io/apimachinery/pkg/api/resource"
+	kube_flag "k8s.io/component-base/cli/flag"
+	"k8s.io/klog/v2"
+
+	"k8s.io/autoscaler/vertical-pod-autoscaler/common"
+	"k8s.io/autoscaler/vertical-pod-autoscaler/pkg/features"
+)
+
+// CertsConfig holds configuration related to TLS certificates
+type CertsConfig struct {
+	ClientCaFile  string
+	TlsCertFile   string
+	TlsPrivateKey string
+	Reload        bool
+}
+
+// AdmissionControllerConfig holds all configuration for the admission controller component
+type AdmissionControllerConfig struct {
+	// Common flags
+	CommonFlags *common.CommonFlags
+
+	CertsConfiguration *CertsConfig
+
+	Ciphers              string
+	MinTlsVersion        string
+	Port                 int
+	Address              string
+	Namespace            string
+	ServiceName          string
+	WebhookAddress       string
+	WebhookPort          string
+	WebhookTimeout       int
+	WebhookFailurePolicy bool
+	RegisterWebhook      bool
+	WebhookLabels        string
+	RegisterByURL        bool
+
+	MaxAllowedCPUBoost resource.QuantityValue
+}
+
+// DefaultAdmissionControllerConfig returns a AdmissionControllerConfig with default values
+func DefaultAdmissionControllerConfig() *AdmissionControllerConfig {
+	return &AdmissionControllerConfig{
+		CommonFlags: common.DefaultCommonConfig(),
+		CertsConfiguration: &CertsConfig{
+			ClientCaFile:  "/etc/tls-certs/caCert.pem",
+			TlsCertFile:   "/etc/tls-certs/serverCert.pem",
+			TlsPrivateKey: "/etc/tls-certs/serverKey.pem",
+			Reload:        false,
+		},
+		Ciphers:              "",
+		MinTlsVersion:        "tls1_2",
+		Port:                 8000,
+		Address:              ":8944",
+		Namespace:            os.Getenv("NAMESPACE"),
+		ServiceName:          "vpa-webhook",
+		WebhookAddress:       "",
+		WebhookPort:          "",
+		WebhookTimeout:       30,
+		WebhookFailurePolicy: false,
+		RegisterWebhook:      true,
+		WebhookLabels:        "",
+		RegisterByURL:        false,
+
+		MaxAllowedCPUBoost: resource.QuantityValue{},
+	}
+}
+
+// InitAdmissionControllerFlags initializes the flags for the admission controller component
+func InitAdmissionControllerFlags() *AdmissionControllerConfig {
+	config := DefaultAdmissionControllerConfig()
+	config.CommonFlags = common.InitCommonFlags()
+
+	flag.StringVar(&config.CertsConfiguration.ClientCaFile, "client-ca-file", config.CertsConfiguration.ClientCaFile, "Path to CA PEM file.")
+	flag.StringVar(&config.CertsConfiguration.TlsCertFile, "tls-cert-file", config.CertsConfiguration.TlsCertFile, "Path to server certificate PEM file.")
+	flag.StringVar(&config.CertsConfiguration.TlsPrivateKey, "tls-private-key", config.CertsConfiguration.TlsPrivateKey, "Path to server certificate key PEM file.")
+	flag.BoolVar(&config.CertsConfiguration.Reload, "reload-cert", config.CertsConfiguration.Reload, "If set to true, reload leaf and CA certificates when changed.")
+
+	flag.StringVar(&config.Ciphers, "tls-ciphers", config.Ciphers, "A comma-separated or colon-separated list of ciphers to accept.  Only works when min-tls-version is set to tls1_2.")
+	flag.StringVar(&config.MinTlsVersion, "min-tls-version", config.MinTlsVersion, "The minimum TLS version to accept.  Must be set to either tls1_2 (default) or tls1_3.")
+	flag.IntVar(&config.Port, "port", config.Port, "The port to listen on.")
+	flag.StringVar(&config.Address, "address", config.Address, "The address to expose Prometheus metrics.")
+	flag.StringVar(&config.ServiceName, "webhook-service", config.ServiceName, "Kubernetes service under which webhook is registered. Used when registerByURL is set to false.")
+	flag.StringVar(&config.WebhookAddress, "webhook-address", config.WebhookAddress, "Address under which webhook is registered. Used when registerByURL is set to true.")
+	flag.StringVar(&config.WebhookPort, "webhook-port", config.WebhookPort, "Server Port for Webhook")
+	flag.IntVar(&config.WebhookTimeout, "webhook-timeout-seconds", config.WebhookTimeout, "Timeout in seconds that the API server should wait for this webhook to respond before failing.")
+	flag.BoolVar(&config.WebhookFailurePolicy, "webhook-failure-policy-fail", config.WebhookFailurePolicy, "If set to true, will configure the admission webhook failurePolicy to \"Fail\". Use with caution.")
+	flag.BoolVar(&config.RegisterWebhook, "register-webhook", config.RegisterWebhook, "If set to true, admission webhook object will be created on start up to register with the API server.")
+	flag.StringVar(&config.WebhookLabels, "webhook-labels", config.WebhookLabels, "Comma separated list of labels to add to the webhook object. Format: key1:value1,key2:value2")
+	flag.BoolVar(&config.RegisterByURL, "register-by-url", config.RegisterByURL, "If set to true, admission webhook will be registered by URL (webhookAddress:webhookPort) instead of by service name")
+
+	flag.Var(&config.MaxAllowedCPUBoost, "max-allowed-cpu-boost", "Maximum amount of CPU that will be applied for a container with boost.")
+
+	// These need to happen last. kube_flag.InitFlags() synchronizes and parses
+	// flags from the flag package to pflag, so feature gates must be added to
+	// pflag before InitFlags() is called.
+	klog.InitFlags(nil)
+	common.InitLoggingFlags()
+	features.MutableFeatureGate.AddFlag(pflag.CommandLine)
+	kube_flag.InitFlags()
+
+	ValidateAdmissionControllerConfig(config)
+
+	return config
+}
+
+// ValidateAdmissionControllerConfig performs validation of the admission-controller flags
+func ValidateAdmissionControllerConfig(config *AdmissionControllerConfig) {
+	common.ValidateCommonConfig(config.CommonFlags)
+}