From aad2b573c63c726ad98ee3e920f2787f8271479a Mon Sep 17 00:00:00 2001 From: Marek Counts Date: Thu, 16 May 2019 22:47:42 -0400 Subject: [PATCH] seperation of network calls when getting version updated the network calls to be package local so tests could pass their own implementation. A public interface was not provided as it would not be likely this would ever be needed or wanted. --- cmd/kubeadm/app/util/version.go | 14 +++++---- cmd/kubeadm/app/util/version_test.go | 45 ++++++++++++++++++---------- 2 files changed, 39 insertions(+), 20 deletions(-) diff --git a/cmd/kubeadm/app/util/version.go b/cmd/kubeadm/app/util/version.go index 4e90b27dda6..ddc139941dd 100644 --- a/cmd/kubeadm/app/util/version.go +++ b/cmd/kubeadm/app/util/version.go @@ -62,6 +62,10 @@ var ( // 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) { + return kubernetesReleaseVersion(version, fetchFromURL) +} + +func kubernetesReleaseVersion(version string, fetcher func(string, time.Duration) (string, error)) (string, error) { ver := normalizedBuildVersion(version) if len(ver) != 0 { return ver, nil @@ -87,7 +91,7 @@ func KubernetesReleaseVersion(version string) (string, error) { clientVersion, clientVersionErr := kubeadmVersion(pkgversion.Get().String()) // Fetch version from the internet. url := fmt.Sprintf("%s/%s.txt", bucketURL, versionLabel) - body, err := fetchFromURL(url, getReleaseVersionTimeout) + body, err := fetcher(url, getReleaseVersionTimeout) if err != nil { // If the network operaton was successful but the server did not reply with StatusOK if body != "" { @@ -97,18 +101,18 @@ func KubernetesReleaseVersion(version string) (string, error) { // Handle air-gapped environments by falling back to the client version. klog.Warningf("could not fetch a Kubernetes version from the internet: %v", err) klog.Warningf("falling back to the local client version: %s", clientVersion) - return KubernetesReleaseVersion(clientVersion) + return kubernetesReleaseVersion(clientVersion, fetcher) } } if clientVersionErr != nil { if err != nil { klog.Warningf("could not obtain neither client nor remote version; fall back to: %s", constants.CurrentKubernetesVersion) - return KubernetesReleaseVersion(constants.CurrentKubernetesVersion.String()) + return kubernetesReleaseVersion(constants.CurrentKubernetesVersion.String(), fetcher) } klog.Warningf("could not obtain client version; using remote version: %s", body) - return KubernetesReleaseVersion(body) + return kubernetesReleaseVersion(body, fetcher) } // both the client and the remote version are obtained; validate them and pick a stable version @@ -117,7 +121,7 @@ func KubernetesReleaseVersion(version string) (string, error) { return "", err } // Re-validate received version and return. - return KubernetesReleaseVersion(body) + return kubernetesReleaseVersion(body, fetcher) } return "", errors.Errorf("version %q doesn't match patterns for neither semantic version nor labels (stable, latest, ...)", version) } diff --git a/cmd/kubeadm/app/util/version_test.go b/cmd/kubeadm/app/util/version_test.go index f40f261338b..39de4bd1e45 100644 --- a/cmd/kubeadm/app/util/version_test.go +++ b/cmd/kubeadm/app/util/version_test.go @@ -17,12 +17,13 @@ limitations under the License. package util import ( + "errors" "fmt" "net/http" - "net/http/httptest" "path" "strings" "testing" + "time" ) func TestEmptyVersion(t *testing.T) { @@ -50,7 +51,10 @@ func TestValidVersion(t *testing.T) { } for _, s := range validVersions { t.Run(s, func(t *testing.T) { - ver, err := KubernetesReleaseVersion(s) + fileFetcher := func(url string, timeout time.Duration) (string, error) { + return "", errors.New("Should not make internet call") + } + ver, err := kubernetesReleaseVersion(s, fileFetcher) t.Log("Valid: ", s, ver, err) if err != nil { t.Errorf("KubernetesReleaseVersion unexpected error for version %q: %v", s, err) @@ -72,7 +76,10 @@ func TestInvalidVersion(t *testing.T) { } for _, s := range invalidVersions { t.Run(s, func(t *testing.T) { - ver, err := KubernetesReleaseVersion(s) + fileFetcher := func(url string, timeout time.Duration) (string, error) { + return "", errors.New("Should not make internet call") + } + ver, err := kubernetesReleaseVersion(s, fileFetcher) t.Log("Invalid: ", s, ver, err) if err == nil { t.Errorf("KubernetesReleaseVersion error expected for version %q, but returned successfully", s) @@ -92,7 +99,10 @@ func TestValidConvenientForUserVersion(t *testing.T) { } for _, s := range validVersions { t.Run(s, func(t *testing.T) { - ver, err := KubernetesReleaseVersion(s) + fileFetcher := func(url string, timeout time.Duration) (string, error) { + return "", errors.New("Should not make internet call") + } + ver, err := kubernetesReleaseVersion(s, fileFetcher) t.Log("Valid: ", s, ver, err) if err != nil { t.Errorf("KubernetesReleaseVersion unexpected error for version %q: %v", s, err) @@ -121,22 +131,19 @@ func TestVersionFromNetwork(t *testing.T) { "garbage": {"NoSuchKeyThe specified key does not exist.", http.StatusOK, "", true}, "unknown": {"The requested URL was not found on this server.", http.StatusNotFound, "", true}, } - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - key := strings.TrimSuffix(path.Base(r.URL.Path), ".txt") + + fileFetcher := func(url string, timeout time.Duration) (string, error) { + key := strings.TrimSuffix(path.Base(url), ".txt") res, found := cases[key] if found { - http.Error(w, res.Content, res.Status) - } else { - http.Error(w, "Unknown test case key!", http.StatusNotFound) + return res.Content, nil } - })) - defer server.Close() - - kubeReleaseBucketURL = server.URL + return "Unknown test case key!", errors.New("unknown test case key") + } for k, v := range cases { t.Run(k, func(t *testing.T) { - ver, err := KubernetesReleaseVersion(k) + ver, err := kubernetesReleaseVersion(k, fileFetcher) t.Logf("Key: %q. Result: %q, Error: %v", k, ver, err) switch { case err != nil && !v.ErrorExpected: @@ -272,7 +279,15 @@ func TestCIBuildVersion(t *testing.T) { for _, tc := range cases { t.Run(fmt.Sprintf("input:%s/expected:%s", tc.input, tc.expected), func(t *testing.T) { - ver, err := KubernetesReleaseVersion(tc.input) + + fileFetcher := func(url string, timeout time.Duration) (string, error) { + if tc.valid { + return tc.expected, nil + } + return "Unknown test case key!", fmt.Errorf("unknown test case key") + } + + ver, err := kubernetesReleaseVersion(tc.input, fileFetcher) t.Logf("Input: %q. Result: %q, Error: %v", tc.input, ver, err) switch { case err != nil && tc.valid: