From f680d1b1fcca11fc57b01f8521af2a7a5a9bf452 Mon Sep 17 00:00:00 2001 From: Nanik T Date: Fri, 1 Nov 2019 16:42:21 +1100 Subject: [PATCH] Fix --kubernetes-version for upgrading cluster to correct version This PR fix the issue of checking kubernetes-version and upgrading it. The fix now will do the following * Not specifying the --kubernetes-version flag means just use the currently deployed version. * If an update is available inform the user that they may use --kubernetes-version=. * When --kubernetes-version is specifically set, upgrade the cluster. Also add integration testing to test upgrading and downgrading --- cmd/minikube/cmd/start.go | 26 ++++++++--- cmd/minikube/cmd/start_test.go | 55 ++++++++++++++++++++++++ test/integration/version_upgrade_test.go | 36 +++++++++++++++- 3 files changed, 108 insertions(+), 9 deletions(-) diff --git a/cmd/minikube/cmd/start.go b/cmd/minikube/cmd/start.go index 1be0db1f1205..c97f28cb4bf7 100644 --- a/cmd/minikube/cmd/start.go +++ b/cmd/minikube/cmd/start.go @@ -178,7 +178,7 @@ func initMinikubeFlags() { // initKubernetesFlags inits the commandline flags for kubernetes related options func initKubernetesFlags() { - startCmd.Flags().String(kubernetesVersion, constants.DefaultKubernetesVersion, "The kubernetes version that the minikube VM will use (ex: v1.2.3)") + startCmd.Flags().String(kubernetesVersion, "", "The kubernetes version that the minikube VM will use (ex: v1.2.3)") startCmd.Flags().Var(&extraOptions, "extra-config", `A set of key=value pairs that describe configuration that may be passed to different components. The key should be '.' separated, and the first part before the dot is the component to apply the configuration to. @@ -1114,15 +1114,20 @@ func tryRegistry(r command.Runner) { // getKubernetesVersion ensures that the requested version is reasonable func getKubernetesVersion(old *cfg.MachineConfig) (string, bool) { - rawVersion := viper.GetString(kubernetesVersion) + paramVersion := viper.GetString(kubernetesVersion) isUpgrade := false - if rawVersion == "" { - rawVersion = constants.DefaultKubernetesVersion + + if paramVersion == "" { // if the user did not specify any version then ... + if old != nil { // .. use the old version from config + paramVersion = old.KubernetesConfig.KubernetesVersion + } else { // .. otherwise use the default version + paramVersion = constants.DefaultKubernetesVersion + } } - nvs, err := semver.Make(strings.TrimPrefix(rawVersion, version.VersionPrefix)) + nvs, err := semver.Make(strings.TrimPrefix(paramVersion, version.VersionPrefix)) if err != nil { - exit.WithCodeT(exit.Data, `Unable to parse "{{.kubernetes_version}}": {{.error}}`, out.V{"kubernetes_version": rawVersion, "error": err}) + exit.WithCodeT(exit.Data, `Unable to parse "{{.kubernetes_version}}": {{.error}}`, out.V{"kubernetes_version": paramVersion, "error": err}) } nv := version.VersionPrefix + nvs.String() @@ -1134,6 +1139,10 @@ func getKubernetesVersion(old *cfg.MachineConfig) (string, bool) { if err != nil { exit.WithCodeT(exit.Data, "Unable to parse oldest Kubernetes version from constants: {{.error}}", out.V{"error": err}) } + defaultVersion, err := semver.Make(strings.TrimPrefix(constants.DefaultKubernetesVersion, version.VersionPrefix)) + if err != nil { + exit.WithCodeT(exit.Data, "Unable to parse default Kubernetes version from constants: {{.error}}", out.V{"error": err}) + } if nvs.LT(oldestVersion) { out.WarningT("Specified Kubernetes version {{.specified}} is less than the oldest supported version: {{.oldest}}", out.V{"specified": nvs, "oldest": constants.OldestKubernetesVersion}) @@ -1162,8 +1171,11 @@ func getKubernetesVersion(old *cfg.MachineConfig) (string, bool) { * Reuse the existing cluster with Kubernetes v{{.old}} or newer: Run "minikube start {{.profile}} --kubernetes-version={{.old}}"`, out.V{"new": nvs, "old": ovs, "profile": profileArg}) } + if defaultVersion.GT(nvs) { + out.T(out.ThumbsUp, "Kubernetes {{.new}} is now available. If you would like to upgrade, specify: --kubernetes-version={{.new}}", out.V{"new": defaultVersion}) + } + if nvs.GT(ovs) { - out.T(out.ThumbsUp, "Upgrading from Kubernetes {{.old}} to {{.new}}", out.V{"old": ovs, "new": nvs}) isUpgrade = true } return nv, isUpgrade diff --git a/cmd/minikube/cmd/start_test.go b/cmd/minikube/cmd/start_test.go index 60c57d76fd44..33de12cc137c 100644 --- a/cmd/minikube/cmd/start_test.go +++ b/cmd/minikube/cmd/start_test.go @@ -22,9 +22,64 @@ import ( "github.com/spf13/cobra" "github.com/spf13/viper" + cfg "k8s.io/minikube/pkg/minikube/config" "k8s.io/minikube/pkg/minikube/constants" ) +func TestGetKuberneterVersion(t *testing.T) { + var tests = []struct { + description string + expectedVersion string + paramVersion string + upgrade bool + cfg *cfg.MachineConfig + }{ + { + description: "kubernetes-version not given, no config", + expectedVersion: constants.DefaultKubernetesVersion, + paramVersion: "", + upgrade: false, + }, + { + description: "kubernetes-version not given, config available", + expectedVersion: "v1.15.0", + paramVersion: "", + upgrade: false, + cfg: &cfg.MachineConfig{KubernetesConfig: cfg.KubernetesConfig{KubernetesVersion: "v1.15.0"}}, + }, + { + description: "kubernetes-version given, no config", + expectedVersion: "v1.15.0", + paramVersion: "v1.15.0", + upgrade: false, + }, + { + description: "kubernetes-version given, config available", + expectedVersion: "v1.16.0", + paramVersion: "v1.16.0", + upgrade: true, + cfg: &cfg.MachineConfig{KubernetesConfig: cfg.KubernetesConfig{KubernetesVersion: "v1.15.0"}}, + }, + } + + for _, test := range tests { + t.Run(test.description, func(t *testing.T) { + viper.SetDefault(kubernetesVersion, test.paramVersion) + version, upgrade := getKubernetesVersion(test.cfg) + + // check whether we are getting the expected version + if version != test.expectedVersion { + t.Fatalf("test failed because the expected version %s is not returned", test.expectedVersion) + } + + // check whether the upgrade flag is correct + if test.upgrade != upgrade { + t.Fatalf("test failed expected upgrade is %t", test.upgrade) + } + }) + } +} + func TestGenerateCfgFromFlagsHTTPProxyHandling(t *testing.T) { viper.SetDefault(memory, defaultMemorySize) viper.SetDefault(humanReadableDiskSize, defaultDiskSize) diff --git a/test/integration/version_upgrade_test.go b/test/integration/version_upgrade_test.go index 78bc9ef71221..cec42dcb9d37 100644 --- a/test/integration/version_upgrade_test.go +++ b/test/integration/version_upgrade_test.go @@ -18,6 +18,7 @@ package integration import ( "context" + "encoding/json" "fmt" "io/ioutil" "os" @@ -28,6 +29,7 @@ import ( "time" "github.com/docker/machine/libmachine/state" + "k8s.io/minikube/pkg/minikube/constants" "k8s.io/minikube/pkg/util/retry" @@ -66,13 +68,13 @@ func TestVersionUpgrade(t *testing.T) { args := append([]string{"start", "-p", profile, fmt.Sprintf("--kubernetes-version=%s", constants.OldestKubernetesVersion), "--alsologtostderr", "-v=1"}, StartArgs()...) rr := &RunResult{} - releaseStart := func() error { + r := func() error { rr, err = Run(t, exec.CommandContext(ctx, tf.Name(), args...)) return err } // Retry to allow flakiness for the previous release - if err := retry.Expo(releaseStart, 1*time.Second, 30*time.Minute, 3); err != nil { + if err := retry.Expo(r, 1*time.Second, 30*time.Minute, 3); err != nil { t.Fatalf("release start failed: %v", err) } @@ -96,4 +98,34 @@ func TestVersionUpgrade(t *testing.T) { if err != nil { t.Errorf("%s failed: %v", rr.Args, err) } + + s, err := Run(t, exec.CommandContext(ctx, "kubectl", "--context", profile, "version", "--output=json")) + if err != nil { + t.Fatalf("error running kubectl: %v", err) + } + cv := struct { + ServerVersion struct { + GitVersion string `json:"gitVersion"` + } `json:"serverVersion"` + }{} + err = json.Unmarshal(s.Stdout.Bytes(), &cv) + + if err != nil { + t.Fatalf("error traversing json output: %v", err) + } + + if cv.ServerVersion.GitVersion != constants.NewestKubernetesVersion { + t.Fatalf("expected server version %s is not the same with latest version %s", cv.ServerVersion.GitVersion, constants.NewestKubernetesVersion) + } + + args = append([]string{"start", "-p", profile, fmt.Sprintf("--kubernetes-version=%s", constants.OldestKubernetesVersion), "--alsologtostderr", "-v=1"}, StartArgs()...) + rr = &RunResult{} + r = func() error { + rr, err = Run(t, exec.CommandContext(ctx, tf.Name(), args...)) + return err + } + + if err := retry.Expo(r, 1*time.Second, 30*time.Minute, 3); err == nil { + t.Fatalf("downgrading kubernetes should not be allowed: %v", err) + } }