From 59746fbf4fb312a64eb604d9f8140367ff9c7b32 Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Tue, 23 Apr 2024 11:59:08 +0300 Subject: [PATCH] kubeadm: improve performance of unit tests that need a k8s version The function KubernetesReleaseVersion is being called in a number of locations during unit tests but by default it uses a "fetch version from URL" approach. - Update the function to return a placeholder version during unit tests. - Update unit tests for this function. - Update strings / comments in other version_tests.go locations. The improvement is significant: time go test k8s.io/kubernetes/cmd/kubeadm/app/... -count=1 before: real 2m47.733s after: real 0m10.234s --- cmd/kubeadm/app/constants/constants.go | 6 +- cmd/kubeadm/app/constants/constants_test.go | 2 +- cmd/kubeadm/app/util/version.go | 9 ++- cmd/kubeadm/app/util/version_test.go | 78 +++++++++++++++------ 4 files changed, 68 insertions(+), 27 deletions(-) diff --git a/cmd/kubeadm/app/constants/constants.go b/cmd/kubeadm/app/constants/constants.go index 70646c21145..9516ae71731 100644 --- a/cmd/kubeadm/app/constants/constants.go +++ b/cmd/kubeadm/app/constants/constants.go @@ -494,9 +494,9 @@ var ( // the bootstrap tokens to access the kubeadm-certs Secret during the join of a new control-plane KubeadmCertsClusterRoleName = fmt.Sprintf("kubeadm:%s", KubeadmCertsSecret) - // defaultKubernetesPlaceholderVersion is a placeholder version in case the component-base + // DefaultKubernetesPlaceholderVersion is a placeholder version in case the component-base // version was not populated during build. - defaultKubernetesPlaceholderVersion = version.MustParseSemantic("v1.0.0-placeholder-version") + DefaultKubernetesPlaceholderVersion = version.MustParseSemantic("v1.0.0-placeholder-version") ) // getSkewedKubernetesVersion returns the current MAJOR.(MINOR+n).0 Kubernetes version with a skew of 'n' @@ -514,7 +514,7 @@ func getSkewedKubernetesVersionImpl(versionInfo *apimachineryversion.Info, n int // More changes would be required if the kubelet version one day decouples from that of Kubernetes. var ver *version.Version if len(versionInfo.Major) == 0 { - return defaultKubernetesPlaceholderVersion + return DefaultKubernetesPlaceholderVersion } ver = version.MustParseSemantic(versionInfo.GitVersion) // Append the MINOR version skew. diff --git a/cmd/kubeadm/app/constants/constants_test.go b/cmd/kubeadm/app/constants/constants_test.go index bc33346a8d2..505a7572e67 100644 --- a/cmd/kubeadm/app/constants/constants_test.go +++ b/cmd/kubeadm/app/constants/constants_test.go @@ -258,7 +258,7 @@ func TestGetSkewedKubernetesVersionImpl(t *testing.T) { { name: "invalid versionInfo; placeholder version is returned", versionInfo: &apimachineryversion.Info{}, - expectedResult: defaultKubernetesPlaceholderVersion, + expectedResult: DefaultKubernetesPlaceholderVersion, }, { name: "valid skew of -1", diff --git a/cmd/kubeadm/app/util/version.go b/cmd/kubeadm/app/util/version.go index 297a9c81d00..608969f05cc 100644 --- a/cmd/kubeadm/app/util/version.go +++ b/cmd/kubeadm/app/util/version.go @@ -46,7 +46,12 @@ var ( kubeBucketPrefixes = regexp.MustCompile(`^((release|ci)/)?([-\w.+]+)$`) ) -// KubernetesReleaseVersion is helper function that can fetch +// KubernetesReleaseVersion during unit tests equals kubernetesReleaseVersionTest +// and returns a static placeholder version. When not running in unit tests +// it equals kubernetesReleaseVersionDefault. +var KubernetesReleaseVersion = kubernetesReleaseVersionDefault + +// kubernetesReleaseVersionDefault is helper function that can fetch // available version information from release servers based on // label names, like "stable" or "latest". // @@ -64,7 +69,7 @@ var ( // latest (latest release, including alpha/beta) // latest-1 (latest release in 1.x, including alpha/beta) // latest-1.0 (and similarly 1.1, 1.2, 1.3, ...) -func KubernetesReleaseVersion(version string) (string, error) { +func kubernetesReleaseVersionDefault(version string) (string, error) { return kubernetesReleaseVersion(version, fetchFromURL) } diff --git a/cmd/kubeadm/app/util/version_test.go b/cmd/kubeadm/app/util/version_test.go index 8eb749c23a8..b290461180e 100644 --- a/cmd/kubeadm/app/util/version_test.go +++ b/cmd/kubeadm/app/util/version_test.go @@ -18,6 +18,7 @@ package util import ( "fmt" + "os" "path" "strings" "testing" @@ -28,14 +29,49 @@ import ( "k8s.io/kubernetes/cmd/kubeadm/app/constants" ) -func TestEmptyVersion(t *testing.T) { +func TestMain(m *testing.M) { + KubernetesReleaseVersion = kubernetesReleaseVersionTest + os.Exit(m.Run()) +} - ver, err := KubernetesReleaseVersion("") - if err == nil { - t.Error("KubernetesReleaseVersion returned successfully, but error expected") +func kubernetesReleaseVersionTest(version string) (string, error) { + fetcher := func(string, time.Duration) (string, error) { + return constants.DefaultKubernetesPlaceholderVersion.String(), nil } - if ver != "" { - t.Error("KubernetesReleaseVersion returned value, expected only error") + return kubernetesReleaseVersion(version, fetcher) +} + +func TesKubernetesReleaseVersion(t *testing.T) { + tests := []struct { + name string + input string + expectedOutput string + expectedError bool + }{ + { + name: "empty input", + input: "", + expectedOutput: "", + expectedError: true, + }, + { + name: "label as input", + input: "stable", + expectedOutput: constants.DefaultKubernetesPlaceholderVersion.String(), + expectedError: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + output, err := KubernetesReleaseVersion(tc.input) + if (err != nil) != tc.expectedError { + t.Errorf("expected error: %v, got: %v, error: %v", tc.expectedError, err != nil, err) + } + if output != tc.expectedOutput { + t.Errorf("expected output: %s, got: %s", tc.expectedOutput, output) + } + }) } } @@ -57,10 +93,10 @@ func TestValidVersion(t *testing.T) { ver, err := kubernetesReleaseVersion(s, errorFetcher) t.Log("Valid: ", s, ver, err) if err != nil { - t.Errorf("KubernetesReleaseVersion unexpected error for version %q: %v", s, err) + t.Errorf("kubernetesReleaseVersion unexpected error for version %q: %v", s, err) } if ver != s && ver != "v"+s { - t.Errorf("KubernetesReleaseVersion should return same valid version string. %q != %q", s, ver) + t.Errorf("kubernetesReleaseVersion should return same valid version string. %q != %q", s, ver) } }) } @@ -79,10 +115,10 @@ func TestInvalidVersion(t *testing.T) { ver, err := kubernetesReleaseVersion(s, errorFetcher) t.Log("Invalid: ", s, ver, err) if err == nil { - t.Errorf("KubernetesReleaseVersion error expected for version %q, but returned successfully", s) + t.Errorf("kubernetesReleaseVersion error expected for version %q, but returned successfully", s) } if ver != "" { - t.Errorf("KubernetesReleaseVersion should return empty string in case of error. Returned %q for version %q", ver, s) + t.Errorf("kubernetesReleaseVersion should return empty string in case of error. Returned %q for version %q", ver, s) } }) } @@ -99,10 +135,10 @@ func TestValidConvenientForUserVersion(t *testing.T) { ver, err := kubernetesReleaseVersion(s, errorFetcher) t.Log("Valid: ", s, ver, err) if err != nil { - t.Errorf("KubernetesReleaseVersion unexpected error for version %q: %v", s, err) + t.Errorf("kubernetesReleaseVersion unexpected error for version %q: %v", s, err) } if ver != "v"+s { - t.Errorf("KubernetesReleaseVersion should return semantic version string. %q vs. %q", s, ver) + t.Errorf("kubernetesReleaseVersion should return semantic version string. %q vs. %q", s, ver) } }) } @@ -147,11 +183,11 @@ func TestVersionFromNetwork(t *testing.T) { t.Logf("Key: %q. Result: %q, Error: %v", k, ver, err) switch { case err != nil && !v.ErrorExpected: - t.Errorf("KubernetesReleaseVersion: unexpected error for %q. Error: %+v", k, err) + t.Errorf("kubernetesReleaseVersion: unexpected error for %q. Error: %+v", k, err) case err == nil && v.ErrorExpected: - t.Errorf("KubernetesReleaseVersion: error expected for key %q, but result is %q", k, ver) + t.Errorf("kubernetesReleaseVersion: error expected for key %q, but result is %q", k, ver) case ver != v.Expected: - t.Errorf("KubernetesReleaseVersion: unexpected result for key %q. Expected: %q Actual: %q", k, v.Expected, ver) + t.Errorf("kubernetesReleaseVersion: unexpected result for key %q. Expected: %q Actual: %q", k, v.Expected, ver) } }) } @@ -176,7 +212,7 @@ func TestVersionToTag(t *testing.T) { for _, tc := range cases { t.Run(fmt.Sprintf("input:%s/expected:%s", tc.input, tc.expected), func(t *testing.T) { tag := KubernetesVersionToImageTag(tc.input) - t.Logf("KubernetesVersionToImageTag: Input: %q. Result: %q. Expected: %q", tc.input, tag, tc.expected) + t.Logf("kubernetesVersionToImageTag: Input: %q. Result: %q. Expected: %q", tc.input, tag, tc.expected) if tag != tc.expected { t.Errorf("failed KubernetesVersionToImageTag: Input: %q. Result: %q. Expected: %q", tc.input, tag, tc.expected) } @@ -245,7 +281,7 @@ func TestKubernetesIsCIVersion(t *testing.T) { for _, tc := range cases { t.Run(fmt.Sprintf("input:%s/expected:%t", tc.input, tc.expected), func(t *testing.T) { result := KubernetesIsCIVersion(tc.input) - t.Logf("KubernetesIsCIVersion: Input: %q. Result: %v. Expected: %v", tc.input, result, tc.expected) + t.Logf("kubernetesIsCIVersion: Input: %q. Result: %v. Expected: %v", tc.input, result, tc.expected) if result != tc.expected { t.Errorf("failed KubernetesIsCIVersion: Input: %q. Result: %v. Expected: %v", tc.input, result, tc.expected) } @@ -253,7 +289,7 @@ func TestKubernetesIsCIVersion(t *testing.T) { } } -// Validate KubernetesReleaseVersion but with bucket prefixes +// Validate kubernetesReleaseVersion but with bucket prefixes func TestCIBuildVersion(t *testing.T) { type T struct { input string @@ -287,11 +323,11 @@ func TestCIBuildVersion(t *testing.T) { t.Logf("Input: %q. Result: %q, Error: %v", tc.input, ver, err) switch { case err != nil && tc.valid: - t.Errorf("KubernetesReleaseVersion: unexpected error for input %q. Error: %v", tc.input, err) + t.Errorf("kubernetesReleaseVersion: unexpected error for input %q. Error: %v", tc.input, err) case err == nil && !tc.valid: - t.Errorf("KubernetesReleaseVersion: error expected for input %q, but result is %q", tc.input, ver) + t.Errorf("kubernetesReleaseVersion: error expected for input %q, but result is %q", tc.input, ver) case ver != tc.expected: - t.Errorf("KubernetesReleaseVersion: unexpected result for input %q. Expected: %q Actual: %q", tc.input, tc.expected, ver) + t.Errorf("kubernetesReleaseVersion: unexpected result for input %q. Expected: %q Actual: %q", tc.input, tc.expected, ver) } }) }