From b5e4a4555a5326186b4dcb6455684ecd2617a526 Mon Sep 17 00:00:00 2001 From: Bartosz Majsak Date: Wed, 30 Jul 2025 12:37:46 +0200 Subject: [PATCH] fix(envtest): ensures consistent env test setup This PR simplifies EnvTest setup and ensures webhooks and controller are configured and deployed together. Signed-off-by: Bartosz Majsak --- pkg/controller/llmisvc/controller_int_test.go | 9 +- pkg/controller/llmisvc/fixture/envtest.go | 95 +++++++++++++++++++ pkg/controller/llmisvc/suite_test.go | 40 +------- .../llmisvc/webhook/webhook_suite_test.go | 48 +--------- pkg/testing/config.go | 12 ++- 5 files changed, 115 insertions(+), 89 deletions(-) create mode 100644 pkg/controller/llmisvc/fixture/envtest.go diff --git a/pkg/controller/llmisvc/controller_int_test.go b/pkg/controller/llmisvc/controller_int_test.go index dff75425f9b..6ae78b408eb 100644 --- a/pkg/controller/llmisvc/controller_int_test.go +++ b/pkg/controller/llmisvc/controller_int_test.go @@ -302,7 +302,14 @@ var _ = Describe("LLMInferenceService Controller", func() { Route: &v1alpha1.GatewayRoutesSpec{ HTTP: &v1alpha1.HTTPRouteSpec{}, }, - Gateway: &v1alpha1.GatewaySpec{}, + Gateway: &v1alpha1.GatewaySpec{ + Refs: []v1alpha1.UntypedObjectReference{ + { + Name: "my-ingress-gateway", + Namespace: gatewayapi.Namespace(nsName), + }, + }, + }, }, }, } diff --git a/pkg/controller/llmisvc/fixture/envtest.go b/pkg/controller/llmisvc/fixture/envtest.go new file mode 100644 index 00000000000..deea5fd30b8 --- /dev/null +++ b/pkg/controller/llmisvc/fixture/envtest.go @@ -0,0 +1,95 @@ +/* +Copyright 2025 The KServe 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 fixture + +import ( + "context" + "path/filepath" + "time" + + "github.com/onsi/ginkgo/v2" + "github.com/onsi/gomega" + corev1 "k8s.io/api/core/v1" + "k8s.io/client-go/kubernetes" + "k8s.io/client-go/rest" + "k8s.io/client-go/tools/record" + + "github.com/kserve/kserve/pkg/constants" + "github.com/kserve/kserve/pkg/controller/llmisvc" + "github.com/kserve/kserve/pkg/controller/llmisvc/webhook" + "github.com/kserve/kserve/pkg/testing" + pkgtest "github.com/kserve/kserve/pkg/testing" + + ctrl "sigs.k8s.io/controller-runtime" +) + +func SetupTestEnv() *pkgtest.Client { + duration, err := time.ParseDuration(constants.GetEnvOrDefault("ENVTEST_DEFAULT_TIMEOUT", "10s")) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + gomega.SetDefaultEventuallyTimeout(duration) + gomega.SetDefaultEventuallyPollingInterval(250 * time.Millisecond) + + ginkgo.By("Setting up the test environment") + systemNs := constants.KServeNamespace + + ctx, cancel := context.WithCancel(context.Background()) + + llmCtrlFunc := func(cfg *rest.Config, mgr ctrl.Manager) error { + eventBroadcaster := record.NewBroadcaster() + clientSet, err := kubernetes.NewForConfig(cfg) + gomega.Expect(err).NotTo(gomega.HaveOccurred()) + + llmCtrl := llmisvc.LLMInferenceServiceReconciler{ + Client: mgr.GetClient(), + Clientset: clientSet, + // TODO fix it to be set up similar to main.go, for now it's stub + EventRecorder: eventBroadcaster.NewRecorder(mgr.GetScheme(), corev1.EventSource{Component: "v1beta1Controllers"}), + } + return llmCtrl.SetupWithManager(mgr) + } + + webhookManifests := pkgtest.WithWebhookManifests(filepath.Join(pkgtest.ProjectRoot(), "test", "webhooks")) + webhooks := func(cfg *rest.Config, mgr ctrl.Manager) error { + clientSet, err := kubernetes.NewForConfig(cfg) + if err != nil { + return err + } + llmInferenceServiceConfigValidator := webhook.LLMInferenceServiceConfigValidator{ + ClientSet: clientSet, + } + if err := llmInferenceServiceConfigValidator.SetupWithManager(mgr); err != nil { + return err + } + + llmInferenceServiceValidator := webhook.LLMInferenceServiceValidator{} + return llmInferenceServiceValidator.SetupWithManager(mgr) + } + + envTest := testing.NewEnvTest(webhookManifests). + WithWebhooks(webhooks). + WithControllers(llmCtrlFunc). + Start(ctx) + + ginkgo.DeferCleanup(func() { + cancel() + gomega.Expect(envTest.Stop()).To(gomega.Succeed()) + }) + + RequiredResources(context.Background(), envTest.Client, systemNs) + + return envTest +} diff --git a/pkg/controller/llmisvc/suite_test.go b/pkg/controller/llmisvc/suite_test.go index 6e845fe1f49..e7899ac6d71 100644 --- a/pkg/controller/llmisvc/suite_test.go +++ b/pkg/controller/llmisvc/suite_test.go @@ -17,20 +17,11 @@ limitations under the License. package llmisvc_test import ( - "context" "testing" - "time" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" - corev1 "k8s.io/api/core/v1" - "k8s.io/client-go/kubernetes" - "k8s.io/client-go/rest" - "k8s.io/client-go/tools/record" - ctrl "sigs.k8s.io/controller-runtime" - "github.com/kserve/kserve/pkg/constants" - "github.com/kserve/kserve/pkg/controller/llmisvc" "github.com/kserve/kserve/pkg/controller/llmisvc/fixture" pkgtest "github.com/kserve/kserve/pkg/testing" ) @@ -43,34 +34,5 @@ func TestLLMInferenceServiceController(t *testing.T) { var envTest *pkgtest.Client var _ = SynchronizedBeforeSuite(func() { - duration, err := time.ParseDuration(constants.GetEnvOrDefault("ENVTEST_DEFAULT_TIMEOUT", "10s")) - Expect(err).NotTo(HaveOccurred()) - SetDefaultEventuallyTimeout(duration) - SetDefaultEventuallyPollingInterval(250 * time.Millisecond) - - By("Setting up the test environment") - systemNs := constants.KServeNamespace - - llmCtrlFunc := func(cfg *rest.Config, mgr ctrl.Manager) error { - eventBroadcaster := record.NewBroadcaster() - clientSet, err := kubernetes.NewForConfig(cfg) - Expect(err).NotTo(HaveOccurred()) - - llmCtrl := llmisvc.LLMInferenceServiceReconciler{ - Client: mgr.GetClient(), - Clientset: clientSet, - // TODO fix it to be set up similar to main.go, for now it's stub - EventRecorder: eventBroadcaster.NewRecorder(mgr.GetScheme(), corev1.EventSource{Component: "v1beta1Controllers"}), - } - return llmCtrl.SetupWithManager(mgr) - } - - ctx, cancel := context.WithCancel(context.Background()) - envTest = pkgtest.NewEnvTest().WithControllers(llmCtrlFunc).Start(ctx) - DeferCleanup(func() { - cancel() - Expect(envTest.Stop()).To(Succeed()) - }) - - fixture.RequiredResources(context.Background(), envTest.Client, systemNs) + envTest = fixture.SetupTestEnv() }, func() {}) diff --git a/pkg/controller/llmisvc/webhook/webhook_suite_test.go b/pkg/controller/llmisvc/webhook/webhook_suite_test.go index 13727b4408f..402109273a7 100644 --- a/pkg/controller/llmisvc/webhook/webhook_suite_test.go +++ b/pkg/controller/llmisvc/webhook/webhook_suite_test.go @@ -17,23 +17,10 @@ limitations under the License. package webhook_test import ( - "context" - "path/filepath" "testing" - "time" - - "k8s.io/client-go/kubernetes" - - "github.com/kserve/kserve/pkg/controller/llmisvc/webhook" "github.com/kserve/kserve/pkg/controller/llmisvc/fixture" - "k8s.io/client-go/rest" - - "github.com/kserve/kserve/pkg/constants" - - ctrl "sigs.k8s.io/controller-runtime" - pkgtest "github.com/kserve/kserve/pkg/testing" . "github.com/onsi/ginkgo/v2" @@ -48,38 +35,5 @@ func TestWebhooks(t *testing.T) { var envTest *pkgtest.Client var _ = SynchronizedBeforeSuite(func() { - duration, err := time.ParseDuration(constants.GetEnvOrDefault("ENVTEST_DEFAULT_TIMEOUT", "10s")) - Expect(err).NotTo(HaveOccurred()) - SetDefaultEventuallyTimeout(duration) - SetDefaultEventuallyPollingInterval(250 * time.Millisecond) - - By("Setting up the test environment") - systemNs := constants.KServeNamespace - - ctx, cancel := context.WithCancel(context.Background()) - envTest = pkgtest.NewEnvTest( - pkgtest.WithWebhookManifests(filepath.Join(pkgtest.ProjectRoot(), "test", "webhooks")), - ). - WithWebhooks(func(cfg *rest.Config, mgr ctrl.Manager) error { - clientSet, err := kubernetes.NewForConfig(cfg) - if err != nil { - return err - } - llmInferenceServiceConfigValidator := webhook.LLMInferenceServiceConfigValidator{ - ClientSet: clientSet, - } - if err := llmInferenceServiceConfigValidator.SetupWithManager(mgr); err != nil { - return err - } - - llmInferenceServiceValidator := webhook.LLMInferenceServiceValidator{} - return llmInferenceServiceValidator.SetupWithManager(mgr) - }). - Start(ctx) - DeferCleanup(func() { - cancel() - Expect(envTest.Stop()).To(Succeed()) - }) - - fixture.RequiredResources(context.Background(), envTest.Client, systemNs) + envTest = fixture.SetupTestEnv() }, func() {}) diff --git a/pkg/testing/config.go b/pkg/testing/config.go index 60fc194e118..8a4ad7a7011 100644 --- a/pkg/testing/config.go +++ b/pkg/testing/config.go @@ -83,7 +83,7 @@ func (e *Config) WithControllers(setupFunc ...SetupFunc) *Config { return e } -// WithWebhookManifests register webhooks under tests required for the test suite. +// WithWebhooks register webhooks under tests required for the test suite. func (e *Config) WithWebhooks(setupFunc ...SetupFunc) *Config { e.webhooksSetupFuncs = append(e.webhooksSetupFuncs, setupFunc...) @@ -187,7 +187,15 @@ func WithCRDs(paths ...string) Option { // WithWebhookManifests adds CRDs to the test environment using paths. func WithWebhookManifests(paths ...string) Option { return func(target *envtest.Environment) { - target.WebhookInstallOptions.Paths = append(target.WebhookInstallOptions.Paths, paths...) + seen := make(map[string]bool) + for _, p := range target.WebhookInstallOptions.Paths { + seen[p] = true + } + for _, p := range paths { + if !seen[p] { + target.WebhookInstallOptions.Paths = append(target.WebhookInstallOptions.Paths, p) + } + } } }