From 306e73558990b7ab6d58c94df85e0bcf39cc4c24 Mon Sep 17 00:00:00 2001 From: Adrian Moisey Date: Mon, 10 Jun 2024 17:38:13 +0200 Subject: [PATCH 1/2] VPA: Add tests for selfRegistration() and filterVPAs() I'm working on another PR (https://github.com/kubernetes/autoscaler/pull/6788) and realised that those changes will be touching code that is untested. I decided to go back and add some tests before continuing work in that PR. --- .../pkg/admission-controller/config.go | 2 +- .../pkg/admission-controller/config_test.go | 119 ++++++++++++++++++ .../recommender/input/cluster_feeder_test.go | 49 ++++++++ 3 files changed, 169 insertions(+), 1 deletion(-) create mode 100644 vertical-pod-autoscaler/pkg/admission-controller/config_test.go diff --git a/vertical-pod-autoscaler/pkg/admission-controller/config.go b/vertical-pod-autoscaler/pkg/admission-controller/config.go index 231d7772bfd..c86ad21c0b3 100644 --- a/vertical-pod-autoscaler/pkg/admission-controller/config.go +++ b/vertical-pod-autoscaler/pkg/admission-controller/config.go @@ -75,7 +75,7 @@ func configTLS(serverCert, serverKey []byte, minTlsVersion, ciphers string) *tls // register this webhook admission controller with the kube-apiserver // by creating MutatingWebhookConfiguration. -func selfRegistration(clientset *kubernetes.Clientset, caCert []byte, namespace, serviceName, url string, registerByURL bool, timeoutSeconds int32) { +func selfRegistration(clientset kubernetes.Interface, caCert []byte, namespace, serviceName, url string, registerByURL bool, timeoutSeconds int32) { time.Sleep(10 * time.Second) client := clientset.AdmissionregistrationV1().MutatingWebhookConfigurations() _, err := client.Get(context.TODO(), webhookConfigName, metav1.GetOptions{}) diff --git a/vertical-pod-autoscaler/pkg/admission-controller/config_test.go b/vertical-pod-autoscaler/pkg/admission-controller/config_test.go new file mode 100644 index 00000000000..032987657a1 --- /dev/null +++ b/vertical-pod-autoscaler/pkg/admission-controller/config_test.go @@ -0,0 +1,119 @@ +/* +Copyright 2024 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 main + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + admissionregistration "k8s.io/api/admissionregistration/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/client-go/kubernetes/fake" +) + +func TestSelfRegistrationBase(t *testing.T) { + + testClientSet := fake.NewSimpleClientset() + caCert := []byte("fake") + namespace := "default" + serviceName := "vpa-service" + url := "http://example.com/" + registerByURL := true + timeoutSeconds := int32(32) + + selfRegistration(testClientSet, caCert, namespace, serviceName, url, registerByURL, timeoutSeconds) + + webhookConfigInterface := testClientSet.AdmissionregistrationV1().MutatingWebhookConfigurations() + webhookConfig, err := webhookConfigInterface.Get(context.TODO(), webhookConfigName, metav1.GetOptions{}) + + assert.NoError(t, err, "expected no error fetching webhook configuration") + assert.Equal(t, webhookConfigName, webhookConfig.Name, "expected webhook configuration name to match") + + assert.Len(t, webhookConfig.Webhooks, 1, "expected one webhook configuration") + webhook := webhookConfig.Webhooks[0] + assert.Equal(t, "vpa.k8s.io", webhook.Name, "expected webhook name to match") + + PodRule := webhook.Rules[0] + assert.Equal(t, []admissionregistration.OperationType{admissionregistration.Create}, PodRule.Operations, "expected operations to match") + assert.Equal(t, []string{""}, PodRule.APIGroups, "expected API groups to match") + assert.Equal(t, []string{"v1"}, PodRule.APIVersions, "expected API versions to match") + assert.Equal(t, []string{"pods"}, PodRule.Resources, "expected resources to match") + + VPARule := webhook.Rules[1] + assert.Equal(t, []admissionregistration.OperationType{admissionregistration.Create, admissionregistration.Update}, VPARule.Operations, "expected operations to match") + assert.Equal(t, []string{"autoscaling.k8s.io"}, VPARule.APIGroups, "expected API groups to match") + assert.Equal(t, []string{"*"}, VPARule.APIVersions, "ehook.Rulxpected API versions to match") + assert.Equal(t, []string{"verticalpodautoscalers"}, VPARule.Resources, "expected resources to match") + + assert.Equal(t, admissionregistration.SideEffectClassNone, *webhook.SideEffects, "expected side effects to match") + assert.Equal(t, admissionregistration.Ignore, *webhook.FailurePolicy, "expected failure policy to match") + assert.Equal(t, caCert, webhook.ClientConfig.CABundle, "expected CA bundle to match") + assert.Equal(t, timeoutSeconds, *webhook.TimeoutSeconds, "expected timeout seconds to match") +} + +func TestSelfRegistrationWithURL(t *testing.T) { + + testClientSet := fake.NewSimpleClientset() + caCert := []byte("fake") + namespace := "default" + serviceName := "vpa-service" + url := "http://example.com/" + registerByURL := true + timeoutSeconds := int32(32) + + selfRegistration(testClientSet, caCert, namespace, serviceName, url, registerByURL, timeoutSeconds) + + 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.Nil(t, webhook.ClientConfig.Service, "expected service reference to be nil") + assert.NotNil(t, webhook.ClientConfig.URL, "expected URL to be set") + assert.Equal(t, url, *webhook.ClientConfig.URL, "expected URL to match") +} + +func TestSelfRegistrationWithOutURL(t *testing.T) { + + testClientSet := fake.NewSimpleClientset() + caCert := []byte("fake") + namespace := "default" + serviceName := "vpa-service" + url := "http://example.com/" + registerByURL := false + timeoutSeconds := int32(32) + + selfRegistration(testClientSet, caCert, namespace, serviceName, url, registerByURL, timeoutSeconds) + + 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.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.Nil(t, webhook.ClientConfig.URL, "expected URL to be set") +} diff --git a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go index af431d8b908..530d9bfae1e 100644 --- a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go +++ b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go @@ -523,3 +523,52 @@ func TestClusterStateFeeder_InitFromHistoryProvider(t *testing.T) { } assert.Equal(t, memAmount, containerState.GetMaxMemoryPeak()) } + +func TestFilterVPAs(t *testing.T) { + recommenderName := "test-recommender" + defaultRecommenderName := "default-recommender" + + vpa1 := &vpa_types.VerticalPodAutoscaler{ + Spec: vpa_types.VerticalPodAutoscalerSpec{ + Recommenders: []*vpa_types.VerticalPodAutoscalerRecommenderSelector{ + {Name: defaultRecommenderName}, + }, + }, + } + vpa2 := &vpa_types.VerticalPodAutoscaler{ + Spec: vpa_types.VerticalPodAutoscalerSpec{ + Recommenders: []*vpa_types.VerticalPodAutoscalerRecommenderSelector{ + {Name: recommenderName}, + }, + }, + } + vpa3 := &vpa_types.VerticalPodAutoscaler{ + Spec: vpa_types.VerticalPodAutoscalerSpec{ + Recommenders: []*vpa_types.VerticalPodAutoscalerRecommenderSelector{ + {Name: "another-recommender"}, + }, + }, + } + + allVpaCRDs := []*vpa_types.VerticalPodAutoscaler{vpa1, vpa2, vpa3} + + feeder := &clusterStateFeeder{ + recommenderName: recommenderName, + } + + // Set expected results + expectedResult := []*vpa_types.VerticalPodAutoscaler{vpa2} + + // Run the filterVPAs function + result := filterVPAs(feeder, allVpaCRDs) + + if len(result) != len(expectedResult) { + t.Fatalf("expected %d VPAs, got %d", len(expectedResult), len(result)) + } + + for i, vpa := range result { + if vpa != expectedResult[i] { + t.Errorf("expected VPA %v at index %d, got %v", expectedResult[i], i, vpa) + } + } +} From 9c0941f103db24d59cefd00e6ec20acc62e16431 Mon Sep 17 00:00:00 2001 From: Adrian Moisey Date: Fri, 14 Jun 2024 21:56:41 +0200 Subject: [PATCH 2/2] Simplify test with ElementsMatch https://pkg.go.dev/github.com/stretchr/testify@v1.9.0/assert#ElementsMatch --- .../pkg/recommender/input/cluster_feeder_test.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go index 530d9bfae1e..e2d2a81d17a 100644 --- a/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go +++ b/vertical-pod-autoscaler/pkg/recommender/input/cluster_feeder_test.go @@ -566,9 +566,5 @@ func TestFilterVPAs(t *testing.T) { t.Fatalf("expected %d VPAs, got %d", len(expectedResult), len(result)) } - for i, vpa := range result { - if vpa != expectedResult[i] { - t.Errorf("expected VPA %v at index %d, got %v", expectedResult[i], i, vpa) - } - } + assert.ElementsMatch(t, expectedResult, result) }