Skip to content

Commit

Permalink
Pass around comma separate strings rather than lists
Browse files Browse the repository at this point in the history
  • Loading branch information
adrianmoisey committed Jun 26, 2024
1 parent a8a16a4 commit 7a1aea1
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 35 deletions.
9 changes: 6 additions & 3 deletions vertical-pod-autoscaler/pkg/admission-controller/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{})
Expand All @@ -112,14 +112,17 @@ 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{
MatchExpressions: []metav1.LabelSelectorRequirement{
{
Key: "kubernetes.io/metadata.name",
Operator: metav1.LabelSelectorOpNotIn,
Values: ignoredNamespaces,
Values: ignoredNamespacesList,
},
},
}
Expand All @@ -129,7 +132,7 @@ func selfRegistration(clientset kubernetes.Interface, caCert []byte, namespace,
{
Key: "kubernetes.io/metadata.name",
Operator: metav1.LabelSelectorOpIn,
Values: selectedNamespaces,
Values: selectedNamespacesList,
},
},
}
Expand Down
33 changes: 17 additions & 16 deletions vertical-pod-autoscaler/pkg/admission-controller/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package main

import (
"context"
"strings"
"testing"

"github.com/stretchr/testify/assert"
Expand All @@ -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)

Expand Down Expand Up @@ -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)

Expand All @@ -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)

Expand All @@ -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")
}
Expand All @@ -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)

Expand All @@ -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) {
Expand All @@ -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)

Expand All @@ -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")
}
5 changes: 1 addition & 4 deletions vertical-pod-autoscaler/pkg/admission-controller/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"fmt"
"net/http"
"os"
"strings"
"time"

apiv1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"fmt"
"slices"
"strings"
"time"

apiv1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -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.
Expand All @@ -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, ","),
}
}

Expand Down
5 changes: 1 addition & 4 deletions vertical-pod-autoscaler/pkg/recommender/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ package main
import (
"context"
"flag"
"strings"
"time"

resourceclient "k8s.io/metrics/pkg/client/clientset/versioned/typed/metrics/v1beta1"
Expand Down Expand Up @@ -156,8 +155,6 @@ func main() {
source = input_metrics.NewPodMetricsesSource(resourceclient.NewForConfigOrDie(config))
}

ignoredNamespaces := strings.Split(*ignoredVpaObjectNamespaces, ",")

clusterStateFeeder := input.ClusterStateFeederFactory{
PodLister: podLister,
OOMObserver: oomObserver,
Expand All @@ -170,7 +167,7 @@ func main() {
MemorySaveMode: *memorySaver,
ControllerFetcher: controllerFetcher,
RecommenderName: *recommenderName,
IgnoredNamespaces: ignoredNamespaces,
IgnoredNamespaces: *ignoredVpaObjectNamespaces,
}.Make()
controllerFetcher.Start(context.Background(), scaleCacheLoopPeriod)

Expand Down
5 changes: 3 additions & 2 deletions vertical-pod-autoscaler/pkg/updater/logic/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"fmt"
"slices"
"strings"
"time"

"golang.org/x/time/rate"
Expand Down Expand Up @@ -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)
Expand All @@ -110,7 +111,7 @@ func NewUpdater(
status.AdmissionControllerStatusName,
statusNamespace,
),
ignoredNamespaces: ignoredNamespaces,
ignoredNamespaces: strings.Split(ignoredNamespaces, ","),
}, nil
}

Expand Down
5 changes: 1 addition & 4 deletions vertical-pod-autoscaler/pkg/updater/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import (
"context"
"flag"
"os"
"strings"
"time"

apiv1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -108,8 +107,6 @@ func main() {
admissionControllerStatusNamespace = namespace
}

ignoredNamespaces := strings.Split(*ignoredVpaObjectNamespaces, ",")

// TODO: use SharedInformerFactory in updater
updater, err := updater.NewUpdater(
kubeClient,
Expand All @@ -126,7 +123,7 @@ func main() {
controllerFetcher,
priority.NewProcessor(),
*vpaObjectNamespace,
ignoredNamespaces,
*ignoredVpaObjectNamespaces,
)
if err != nil {
klog.Fatalf("Failed to create updater: %v", err)
Expand Down

0 comments on commit 7a1aea1

Please sign in to comment.