Skip to content

Commit

Permalink
Merge pull request #124468 from neolit123/1.31-improve-unit-test-perf…
Browse files Browse the repository at this point in the history 
…ormance-with-test-versions

kubeadm: improve performance of unit tests that need a k8s version
  • Loading branch information
@k8s-ci-robot
k8s-ci-robot committed Apr 24, 2024
2 parents e59ecee + 59746fb commit 5291c11
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 27 deletions.
6 changes: 3 additions & 3 deletions cmd/kubeadm/app/constants/constants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -495,9 +495,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'
Expand All @@ -515,7 +515,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.
Expand Down
2 changes: 1 addition & 1 deletion cmd/kubeadm/app/constants/constants_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
9 changes: 7 additions & 2 deletions cmd/kubeadm/app/util/version.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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".
//
Expand All @@ -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)
}

Expand Down
78 changes: 57 additions & 21 deletions cmd/kubeadm/app/util/version_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package util

import (
"fmt"
"os"
"path"
"strings"
"testing"
Expand All @@ -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())
}

func kubernetesReleaseVersionTest(version string) (string, error) {
fetcher := func(string, time.Duration) (string, error) {
return constants.DefaultKubernetesPlaceholderVersion.String(), nil
}
return kubernetesReleaseVersion(version, fetcher)
}

ver, err := KubernetesReleaseVersion("")
if err == nil {
t.Error("KubernetesReleaseVersion returned successfully, but error expected")
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,
},
}
if ver != "" {
t.Error("KubernetesReleaseVersion returned value, expected only error")

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)
}
})
}
}

Expand All @@ -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)
}
})
}
Expand All @@ -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)
}
})
}
Expand All @@ -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)
}
})
}
Expand Down Expand Up @@ -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)
}
})
}
Expand All @@ -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)
}
Expand Down Expand Up @@ -245,15 +281,15 @@ 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)
}
})
}
}

// Validate KubernetesReleaseVersion but with bucket prefixes
// Validate kubernetesReleaseVersion but with bucket prefixes
func TestCIBuildVersion(t *testing.T) {
type T struct {
input string
Expand Down Expand Up @@ -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)
}
})
}
Expand Down

0 comments on commit 5291c11

Please sign in to comment.