From 7a1aea10f2c52df29c10459ae28108cb70f728cf Mon Sep 17 00:00:00 2001 From: Adrian Moisey Date: Wed, 26 Jun 2024 20:08:08 +0200 Subject: [PATCH] Pass around comma separate strings rather than lists As per https://github.com/kubernetes/autoscaler/pull/6788/files#r1624695422 --- .../pkg/admission-controller/config.go | 9 +++-- .../pkg/admission-controller/config_test.go | 33 ++++++++++--------- .../pkg/admission-controller/main.go | 5 +-- .../pkg/recommender/input/cluster_feeder.go | 5 +-- .../pkg/recommender/main.go | 5 +-- .../pkg/updater/logic/updater.go | 5 +-- vertical-pod-autoscaler/pkg/updater/main.go | 5 +-- 7 files changed, 32 insertions(+), 35 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/admission-controller/config.go b/vertical-pod-autoscaler/pkg/admission-controller/config.go index 3fa04a09fce7..472187ab46f9 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/config.go +++ b/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, namespace, serviceName, url string, registerByURL bool, timeoutSeconds int32, selectedNamespaces []string, ignoredNamespaces []string) { +func selfRegistration(clientset kubernetes.Interface, caCert []byte, namespace, serviceName, url string, registerByURL bool, timeoutSeconds int32, selectedNamespaces string, ignoredNamespaces string) { time.Sleep(10 * time.Second) client := clientset.AdmissionregistrationV1().MutatingWebhookConfigurations() _, err := client.Get(context.TODO(), webhookConfigName, metav1.GetOptions{}) @@ -112,6 +112,9 @@ func selfRegistration(clientset kubernetes.Interface, caCert []byte, namespace, failurePolicy := admissionregistration.Ignore RegisterClientConfig.CABundle = caCert + selectedNamespacesList := strings.Split(selectedNamespaces, ",") + ignoredNamespacesList := strings.Split(ignoredNamespaces, ",") + var namespaceSelector metav1.LabelSelector if len(ignoredNamespaces) > 0 { namespaceSelector = metav1.LabelSelector{ @@ -119,7 +122,7 @@ func selfRegistration(clientset kubernetes.Interface, caCert []byte, namespace, { Key: "kubernetes.io/metadata.name", Operator: metav1.LabelSelectorOpNotIn, - Values: ignoredNamespaces, + Values: ignoredNamespacesList, }, }, } @@ -129,7 +132,7 @@ func selfRegistration(clientset kubernetes.Interface, caCert []byte, namespace, { Key: "kubernetes.io/metadata.name", Operator: metav1.LabelSelectorOpIn, - Values: selectedNamespaces, + Values: selectedNamespacesList, }, }, } diff --git a/vertical-pod-autoscaler/pkg/admission-controller/config_test.go b/vertical-pod-autoscaler/pkg/admission-controller/config_test.go index d19c382152f8..a2a9cf119a17 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/config_test.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/config_test.go @@ -18,6 +18,7 @@ package main import ( "context" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -35,8 +36,8 @@ func TestSelfRegistrationBase(t *testing.T) { url := "http://example.com/" registerByURL := true timeoutSeconds := int32(32) - selectedNamespaces := []string{} - ignoredNamespaces := []string{} + selectedNamespaces := "" + ignoredNamespaces := "" selfRegistration(testClientSet, caCert, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespaces, ignoredNamespaces) @@ -77,8 +78,8 @@ func TestSelfRegistrationWithURL(t *testing.T) { url := "http://example.com/" registerByURL := true timeoutSeconds := int32(32) - selectedNamespaces := []string{} - ignoredNamespaces := []string{} + selectedNamespaces := "" + ignoredNamespaces := "" selfRegistration(testClientSet, caCert, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespaces, ignoredNamespaces) @@ -104,8 +105,8 @@ func TestSelfRegistrationWithOutURL(t *testing.T) { url := "http://example.com/" registerByURL := false timeoutSeconds := int32(32) - selectedNamespaces := []string{} - ignoredNamespaces := []string{} + selectedNamespaces := "" + ignoredNamespaces := "" selfRegistration(testClientSet, caCert, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespaces, ignoredNamespaces) @@ -118,8 +119,8 @@ func TestSelfRegistrationWithOutURL(t *testing.T) { webhook := webhookConfig.Webhooks[0] assert.NotNil(t, webhook.ClientConfig.Service, "expected service reference to be nil") - assert.Equal(t, webhook.ClientConfig.Service.Name, serviceName, "expected service name to be equal") - assert.Equal(t, webhook.ClientConfig.Service.Namespace, namespace, "expected service namespace to be equal") + assert.Equal(t, serviceName, webhook.ClientConfig.Service.Name, "expected service name to be equal") + assert.Equal(t, namespace, webhook.ClientConfig.Service.Namespace, "expected service namespace to be equal") assert.Nil(t, webhook.ClientConfig.URL, "expected URL to be set") } @@ -133,8 +134,8 @@ func TestSelfRegistrationWithIgnoredNamespaces(t *testing.T) { url := "http://example.com/" registerByURL := false timeoutSeconds := int32(32) - selectedNamespaces := []string{} - ignoredNamespaces := []string{"test"} + selectedNamespaces := "" + ignoredNamespaces := "test" selfRegistration(testClientSet, caCert, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespaces, ignoredNamespaces) @@ -150,8 +151,8 @@ func TestSelfRegistrationWithIgnoredNamespaces(t *testing.T) { assert.Len(t, webhook.NamespaceSelector.MatchExpressions, 1, "expected one match expression") matchExpression := webhook.NamespaceSelector.MatchExpressions[0] - assert.Equal(t, matchExpression.Operator, metav1.LabelSelectorOpNotIn, "expected namespace operator to be OpNotIn") - assert.Equal(t, matchExpression.Values, ignoredNamespaces, "expected namespace selector match expression to be equal") + assert.Equal(t, metav1.LabelSelectorOpNotIn, matchExpression.Operator, "expected namespace operator to be OpNotIn") + assert.Equal(t, strings.Split(ignoredNamespaces, ","), matchExpression.Values, "expected namespace selector match expression to be equal") } func TestSelfRegistrationWithSelectedNamespaces(t *testing.T) { @@ -163,8 +164,8 @@ func TestSelfRegistrationWithSelectedNamespaces(t *testing.T) { url := "http://example.com/" registerByURL := false timeoutSeconds := int32(32) - selectedNamespaces := []string{"test"} - ignoredNamespaces := []string{} + selectedNamespaces := "test,namespaces" + ignoredNamespaces := "" selfRegistration(testClientSet, caCert, namespace, serviceName, url, registerByURL, timeoutSeconds, selectedNamespaces, ignoredNamespaces) @@ -180,6 +181,6 @@ func TestSelfRegistrationWithSelectedNamespaces(t *testing.T) { assert.Len(t, webhook.NamespaceSelector.MatchExpressions, 1, "expected one match expression") matchExpression := webhook.NamespaceSelector.MatchExpressions[0] - assert.Equal(t, matchExpression.Operator, metav1.LabelSelectorOpIn, "expected namespace operator to be OpIn") - assert.Equal(t, matchExpression.Values, selectedNamespaces, "expected namespace selector match expression to be equal") + assert.Equal(t, metav1.LabelSelectorOpIn, matchExpression.Operator, "expected namespace operator to be OpIn") + assert.Equal(t, strings.Split(selectedNamespaces, ","), matchExpression.Values, "expected namespace selector match expression to be equal") } diff --git a/vertical-pod-autoscaler/pkg/admission-controller/main.go b/vertical-pod-autoscaler/pkg/admission-controller/main.go index bd2b891450b1..8093160602f1 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/main.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/main.go @@ -21,7 +21,6 @@ import ( "fmt" "net/http" "os" - "strings" "time" apiv1 "k8s.io/api/core/v1" @@ -142,11 +141,9 @@ func main() { TLSConfig: configTLS(*certsConfiguration, *minTlsVersion, *ciphers, stopCh), } url := fmt.Sprintf("%v:%v", *webhookAddress, *webhookPort) - selectedNamespaces := strings.Split(*vpaObjectNamespace, ",") - ignoredNamespaces := strings.Split(*ignoredVpaObjectNamespaces, ",") go func() { if *registerWebhook { - selfRegistration(kubeClient, readFile(*certsConfiguration.clientCaFile), namespace, *serviceName, url, *registerByURL, int32(*webhookTimeout), selectedNamespaces, ignoredNamespaces) + selfRegistration(kubeClient, readFile(*certsConfiguration.clientCaFile), namespace, *serviceName, url, *registerByURL, int32(*webhookTimeout), *vpaObjectNamespace, *ignoredVpaObjectNamespaces) } // Start status updates after the webhook is initialized. statusUpdater.Run(stopCh) diff --git a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go index f9ba5e6d90e3..28ea191018d8 100644 --- a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go +++ b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "slices" + "strings" "time" apiv1 "k8s.io/api/core/v1" @@ -88,7 +89,7 @@ type ClusterStateFeederFactory struct { MemorySaveMode bool ControllerFetcher controllerfetcher.ControllerFetcher RecommenderName string - IgnoredNamespaces []string + IgnoredNamespaces string } // Make creates new ClusterStateFeeder with internal data providers, based on kube client. @@ -105,7 +106,7 @@ func (m ClusterStateFeederFactory) Make() *clusterStateFeeder { memorySaveMode: m.MemorySaveMode, controllerFetcher: m.ControllerFetcher, recommenderName: m.RecommenderName, - ignoredNamespaces: m.IgnoredNamespaces, + ignoredNamespaces: strings.Split(m.IgnoredNamespaces, ","), } } diff --git a/vertical-pod-autoscaler/pkg/recommender/main.go b/vertical-pod-autoscaler/pkg/recommender/main.go index a7fd11af86d8..53d061ae3d39 100644 --- a/vertical-pod-autoscaler/pkg/recommender/main.go +++ b/vertical-pod-autoscaler/pkg/recommender/main.go @@ -19,7 +19,6 @@ package main import ( "context" "flag" - "strings" "time" resourceclient "k8s.io/metrics/pkg/client/clientset/versioned/typed/metrics/v1beta1" @@ -156,8 +155,6 @@ func main() { source = input_metrics.NewPodMetricsesSource(resourceclient.NewForConfigOrDie(config)) } - ignoredNamespaces := strings.Split(*ignoredVpaObjectNamespaces, ",") - clusterStateFeeder := input.ClusterStateFeederFactory{ PodLister: podLister, OOMObserver: oomObserver, @@ -170,7 +167,7 @@ func main() { MemorySaveMode: *memorySaver, ControllerFetcher: controllerFetcher, RecommenderName: *recommenderName, - IgnoredNamespaces: ignoredNamespaces, + IgnoredNamespaces: *ignoredVpaObjectNamespaces, }.Make() controllerFetcher.Start(context.Background(), scaleCacheLoopPeriod) diff --git a/vertical-pod-autoscaler/pkg/updater/logic/updater.go b/vertical-pod-autoscaler/pkg/updater/logic/updater.go index 605eab683200..80a9f5c641e6 100644 --- a/vertical-pod-autoscaler/pkg/updater/logic/updater.go +++ b/vertical-pod-autoscaler/pkg/updater/logic/updater.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "slices" + "strings" "time" "golang.org/x/time/rate" @@ -86,7 +87,7 @@ func NewUpdater( controllerFetcher controllerfetcher.ControllerFetcher, priorityProcessor priority.PriorityProcessor, namespace string, - ignoredNamespaces []string, + ignoredNamespaces string, ) (Updater, error) { evictionRateLimiter := getRateLimiter(evictionRateLimit, evictionRateBurst) factory, err := eviction.NewPodsEvictionRestrictionFactory(kubeClient, minReplicasForEvicition, evictionToleranceFraction) @@ -110,7 +111,7 @@ func NewUpdater( status.AdmissionControllerStatusName, statusNamespace, ), - ignoredNamespaces: ignoredNamespaces, + ignoredNamespaces: strings.Split(ignoredNamespaces, ","), }, nil } diff --git a/vertical-pod-autoscaler/pkg/updater/main.go b/vertical-pod-autoscaler/pkg/updater/main.go index c274f188c1f4..d751c1e8601c 100644 --- a/vertical-pod-autoscaler/pkg/updater/main.go +++ b/vertical-pod-autoscaler/pkg/updater/main.go @@ -20,7 +20,6 @@ import ( "context" "flag" "os" - "strings" "time" apiv1 "k8s.io/api/core/v1" @@ -108,8 +107,6 @@ func main() { admissionControllerStatusNamespace = namespace } - ignoredNamespaces := strings.Split(*ignoredVpaObjectNamespaces, ",") - // TODO: use SharedInformerFactory in updater updater, err := updater.NewUpdater( kubeClient, @@ -126,7 +123,7 @@ func main() { controllerFetcher, priority.NewProcessor(), *vpaObjectNamespace, - ignoredNamespaces, + *ignoredVpaObjectNamespaces, ) if err != nil { klog.Fatalf("Failed to create updater: %v", err)