From b4232f09de39f35320837a8dee0d0e8c9cd73e74 Mon Sep 17 00:00:00 2001 From: Ming Zhao Date: Thu, 30 Oct 2025 09:36:22 -0700 Subject: [PATCH] envtest: respect pre-configured binary paths in ControlPlane This commit fixes an issue where Environment.Start() would ignore pre-configured binary paths (APIServer.Path, Etcd.Path, KubectlPath) set in ControlPlane when DownloadBinaryAssets is false. Changes: - Extract path configuration logic into configureBinaryPaths() method for better testability and separation of concerns - Only auto-configure binary paths when they are empty (not pre-set) - When DownloadBinaryAssets is true, downloaded paths are still used (preserving existing behavior) - Update ControlPlane field documentation to clarify path behavior - Add tests in envtest_test.go to verify path handling logic --- pkg/envtest/envtest_test.go | 52 ++++++++++++++++++++++++++++++ pkg/envtest/server.go | 63 ++++++++++++++++++++++++++----------- 2 files changed, 96 insertions(+), 19 deletions(-) diff --git a/pkg/envtest/envtest_test.go b/pkg/envtest/envtest_test.go index ce3e9a4d3f..806c9f43cc 100644 --- a/pkg/envtest/envtest_test.go +++ b/pkg/envtest/envtest_test.go @@ -963,4 +963,56 @@ var _ = Describe("Test", func() { Expect(env.WebhookInstallOptions.LocalServingCertDir).ShouldNot(BeADirectory()) }) }) + + Describe("Binary Path Handling", func() { + It("should respect pre-configured binary paths when not downloading", func() { + // Setup custom paths + customAPIServerPath := "/custom/path/to/kube-apiserver" + customEtcdPath := "/custom/path/to/etcd" + customKubectlPath := "/custom/path/to/kubectl" + + // Create an environment with pre-configured paths + testEnv := &Environment{} + testEnv.ControlPlane.GetAPIServer().Path = customAPIServerPath + testEnv.ControlPlane.Etcd = &Etcd{} + testEnv.ControlPlane.Etcd.Path = customEtcdPath + testEnv.ControlPlane.KubectlPath = customKubectlPath + + // Set BinaryAssetsDirectory to ensure it's not using defaults + testEnv.BinaryAssetsDirectory = "/should/not/be/used" + testEnv.DownloadBinaryAssets = false + + // Call configureBinaryPaths to test the path configuration logic + err := testEnv.configureBinaryPaths() + Expect(err).NotTo(HaveOccurred()) + + // Verify paths were preserved (not overwritten) + apiServer := testEnv.ControlPlane.GetAPIServer() + Expect(apiServer.Path).To(Equal(customAPIServerPath)) + Expect(testEnv.ControlPlane.Etcd.Path).To(Equal(customEtcdPath)) + Expect(testEnv.ControlPlane.KubectlPath).To(Equal(customKubectlPath)) + }) + + It("should auto-configure binary paths when not pre-configured", func() { + // Create an environment without pre-configured paths + testEnv := &Environment{} + testEnv.BinaryAssetsDirectory = "/test/assets" + testEnv.DownloadBinaryAssets = false + + // Call configureBinaryPaths + err := testEnv.configureBinaryPaths() + Expect(err).NotTo(HaveOccurred()) + + // Verify paths were set using BinPathFinder + apiServer := testEnv.ControlPlane.GetAPIServer() + Expect(apiServer.Path).NotTo(BeEmpty()) + Expect(testEnv.ControlPlane.Etcd.Path).NotTo(BeEmpty()) + Expect(testEnv.ControlPlane.KubectlPath).NotTo(BeEmpty()) + + // Verify the paths contain the binary names + Expect(apiServer.Path).To(ContainSubstring("kube-apiserver")) + Expect(testEnv.ControlPlane.Etcd.Path).To(ContainSubstring("etcd")) + Expect(testEnv.ControlPlane.KubectlPath).To(ContainSubstring("kubectl")) + }) + }) }) diff --git a/pkg/envtest/server.go b/pkg/envtest/server.go index 9bb81ed2ab..c9f19da977 100644 --- a/pkg/envtest/server.go +++ b/pkg/envtest/server.go @@ -109,7 +109,11 @@ var ( // Environment creates a Kubernetes test environment that will start / stop the Kubernetes control plane and // install extension APIs. type Environment struct { - // ControlPlane is the ControlPlane including the apiserver and etcd + // ControlPlane is the ControlPlane including the apiserver and etcd. + // Binary paths (APIServer.Path, Etcd.Path, KubectlPath) can be pre-configured in ControlPlane. + // If DownloadBinaryAssets is true, the downloaded paths will always be used. + // If DownloadBinaryAssets is false and paths are not pre-configured (default is empty), they will be + // automatically resolved using BinaryAssetsDirectory. ControlPlane controlplane.ControlPlane // Scheme is used to determine if conversion webhooks should be enabled @@ -211,6 +215,40 @@ func (te *Environment) Stop() error { return te.ControlPlane.Stop() } +// configureBinaryPaths configures the binary paths for the API server, etcd, and kubectl. +// If DownloadBinaryAssets is true, it downloads and uses those paths. +// If DownloadBinaryAssets is false, it only sets paths that are not already configured (empty). +func (te *Environment) configureBinaryPaths() error { + apiServer := te.ControlPlane.GetAPIServer() + + if te.ControlPlane.Etcd == nil { + te.ControlPlane.Etcd = &controlplane.Etcd{} + } + + if te.DownloadBinaryAssets { + apiServerPath, etcdPath, kubectlPath, err := downloadBinaryAssets(context.TODO(), + te.BinaryAssetsDirectory, te.DownloadBinaryAssetsVersion, te.DownloadBinaryAssetsIndexURL) + if err != nil { + return err + } + + apiServer.Path = apiServerPath + te.ControlPlane.Etcd.Path = etcdPath + te.ControlPlane.KubectlPath = kubectlPath + } else { + if apiServer.Path == "" { + apiServer.Path = process.BinPathFinder("kube-apiserver", te.BinaryAssetsDirectory) + } + if te.ControlPlane.Etcd.Path == "" { + te.ControlPlane.Etcd.Path = process.BinPathFinder("etcd", te.BinaryAssetsDirectory) + } + if te.ControlPlane.KubectlPath == "" { + te.ControlPlane.KubectlPath = process.BinPathFinder("kubectl", te.BinaryAssetsDirectory) + } + } + return nil +} + // Start starts a local Kubernetes server and updates te.ApiserverPort with the port it is listening on. func (te *Environment) Start() (*rest.Config, error) { if te.useExistingCluster() { @@ -229,10 +267,6 @@ func (te *Environment) Start() (*rest.Config, error) { } else { apiServer := te.ControlPlane.GetAPIServer() - if te.ControlPlane.Etcd == nil { - te.ControlPlane.Etcd = &controlplane.Etcd{} - } - if os.Getenv(envAttachOutput) == "true" { te.AttachControlPlaneOutput = true } @@ -243,6 +277,9 @@ func (te *Environment) Start() (*rest.Config, error) { if apiServer.Err == nil { apiServer.Err = os.Stderr } + if te.ControlPlane.Etcd == nil { + te.ControlPlane.Etcd = &controlplane.Etcd{} + } if te.ControlPlane.Etcd.Out == nil { te.ControlPlane.Etcd.Out = os.Stdout } @@ -251,20 +288,8 @@ func (te *Environment) Start() (*rest.Config, error) { } } - if te.DownloadBinaryAssets { - apiServerPath, etcdPath, kubectlPath, err := downloadBinaryAssets(context.TODO(), - te.BinaryAssetsDirectory, te.DownloadBinaryAssetsVersion, te.DownloadBinaryAssetsIndexURL) - if err != nil { - return nil, err - } - - apiServer.Path = apiServerPath - te.ControlPlane.Etcd.Path = etcdPath - te.ControlPlane.KubectlPath = kubectlPath - } else { - apiServer.Path = process.BinPathFinder("kube-apiserver", te.BinaryAssetsDirectory) - te.ControlPlane.Etcd.Path = process.BinPathFinder("etcd", te.BinaryAssetsDirectory) - te.ControlPlane.KubectlPath = process.BinPathFinder("kubectl", te.BinaryAssetsDirectory) + if err := te.configureBinaryPaths(); err != nil { + return nil, fmt.Errorf("failed to configure binary paths: %w", err) } if err := te.defaultTimeouts(); err != nil {