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=<version>.
* When --kubernetes-version is specifically set, upgrade the cluster.

Author: nanikjava

cmd/minikube/cmd/start.go

@@ -179,7 +179,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.
@@ -1057,15 +1057,20 @@ Suggested workarounds:
 
 // getKubernetesVersion ensures that the requested version is reasonable
 func getKubernetesVersion(old *cfg.Config) (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()
 
@@ -1077,6 +1082,10 @@ func getKubernetesVersion(old *cfg.Config) (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})
@@ -1105,8 +1114,11 @@ func getKubernetesVersion(old *cfg.Config) (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

cmd/minikube/cmd/start_test.go

@@ -17,14 +17,95 @@ limitations under the License.
 package cmd
 
 import (
+	"io/ioutil"
 	"os"
+	"strings"
 	"testing"
 
 	"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
+		stderrmsg       string
+		cfg             *cfg.Config
+	}{
+		{
+			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.Config{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,
+			stderrmsg:       "Kubernetes 1.16.2 is now available",
+			cfg:             &cfg.Config{KubernetesConfig: cfg.KubernetesConfig{KubernetesVersion: "v1.15.0"}},
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.description, func(t *testing.T) {
+			oldStderr := os.Stderr
+			reader, writer, err := os.Pipe()
+			if err != nil {
+				t.Fatalf("something is wrong when trying os.Pipe")
+			}
+
+			os.Stderr = writer
+
+			viper.SetDefault(kubernetesVersion, test.paramVersion)
+			version, upgrade := getKubernetesVersion(test.cfg)
+
+			writer.Close()
+			// read the output printed on stdErr
+			out, err := ioutil.ReadAll(reader)
+			if err != nil {
+				t.Fatalf("error reading from the piped stderr")
+			}
+			os.Stderr = oldStderr
+			reader.Close()
+
+			// check whether the printed output is the same
+			if !strings.Contains(string(out), test.stderrmsg) {
+				t.Fatalf("test failed returned result does not contain the string %s", test.stderrmsg)
+			}
+
+			// 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)