From 4b6d91c2c312ed0424537ed5e8632055feccc3c7 Mon Sep 17 00:00:00 2001 From: Dmitry Rozhkov Date: Fri, 14 Dec 2018 12:23:54 +0200 Subject: [PATCH 1/2] kubeadm: print stack trace in case of unexpected error --- cmd/kubeadm/app/util/version_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/kubeadm/app/util/version_test.go b/cmd/kubeadm/app/util/version_test.go index 3444d6c0a2..ef02ced79e 100644 --- a/cmd/kubeadm/app/util/version_test.go +++ b/cmd/kubeadm/app/util/version_test.go @@ -132,7 +132,7 @@ 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) case ver != v.Expected: From b9c2139ccc57a7ca2632dc39fd0f27b5f5a46776 Mon Sep 17 00:00:00 2001 From: Dmitry Rozhkov Date: Fri, 14 Dec 2018 12:26:46 +0200 Subject: [PATCH 2/2] kubeadm: refactor validateStableVersion() Currently the function `cmd/kubeadm/app/util.validateStableVersion()` doesn't validate remote versions in the special case when the client version is empty. This makes the code more difficult to reason about, because the function may successfully return a string which isn't a valid version. Move handling the special case outside of the function to the place where its meaning is more obvious. --- cmd/kubeadm/app/util/version.go | 17 ++++++++++------- cmd/kubeadm/app/util/version_test.go | 4 ++-- 2 files changed, 12 insertions(+), 9 deletions(-) diff --git a/cmd/kubeadm/app/util/version.go b/cmd/kubeadm/app/util/version.go index 4da6ce48ae..85825e30ba 100644 --- a/cmd/kubeadm/app/util/version.go +++ b/cmd/kubeadm/app/util/version.go @@ -79,9 +79,11 @@ func KubernetesReleaseVersion(version string) (string, error) { // kubeReleaseLabelRegex matches labels such as: latest, latest-1, latest-1.10 if kubeReleaseLabelRegex.MatchString(versionLabel) { - var clientVersion string // Try to obtain a client version. - clientVersion, _ = kubeadmVersion(pkgversion.Get().String()) + // pkgversion.Get().String() should always return a correct version added by the golang + // linker and the build system. The version can still be missing when doing unit tests + // on individual packages. + clientVersion, clientVersionErr := kubeadmVersion(pkgversion.Get().String()) // Fetch version from the internet. url := fmt.Sprintf("%s/%s.txt", bucketURL, versionLabel) body, err := fetchFromURL(url, getReleaseVersionTimeout) @@ -95,6 +97,12 @@ func KubernetesReleaseVersion(version string) (string, error) { klog.Infof("falling back to the local client version: %s", clientVersion) return KubernetesReleaseVersion(clientVersion) } + + if clientVersionErr != nil { + klog.Warningf("could not obtain client version; using remote version: %s", body) + return KubernetesReleaseVersion(body) + } + // both the client and the remote version are obtained; validate them and pick a stable version body, err = validateStableVersion(body, clientVersion) if err != nil { @@ -216,11 +224,6 @@ func kubeadmVersion(info string) (string, error) { // This is done to conform with "stable-X" and only allow remote versions from // the same Patch level release. func validateStableVersion(remoteVersion, clientVersion string) (string, error) { - if clientVersion == "" { - klog.Infof("could not obtain client version; using remote version: %s", remoteVersion) - return remoteVersion, nil - } - verRemote, err := versionutil.ParseGeneric(remoteVersion) if err != nil { return "", pkgerrors.Wrap(err, "remote version error") diff --git a/cmd/kubeadm/app/util/version_test.go b/cmd/kubeadm/app/util/version_test.go index ef02ced79e..33b6fccdcb 100644 --- a/cmd/kubeadm/app/util/version_test.go +++ b/cmd/kubeadm/app/util/version_test.go @@ -421,10 +421,10 @@ func TestValidateStableVersion(t *testing.T) { output: "v1.11.0", }, { - name: "valid: client version is empty; use remote version", + name: "invalid: client version is empty", remoteVersion: "v1.12.1", clientVersion: "", - output: "v1.12.1", + expectedError: true, }, { name: "invalid: error parsing the remote version",