Merge pull request #5840 from tstromberg/better-existing-start

Health check previously configured driver & exit if not installed

cmd/minikube/cmd/start.go

@@ -283,13 +283,14 @@ func runStart(cmd *cobra.Command, args []string) {
 		registryMirror = viper.GetStringSlice("registry_mirror")
 	}
 
-	oldConfig, err := cfg.Load()
+	existing, err := cfg.Load()
 	if err != nil && !os.IsNotExist(err) {
 		exit.WithCodeT(exit.Data, "Unable to load config: {{.error}}", out.V{"error": err})
 	}
 
-	driverName := selectDriver(oldConfig)
-	glog.Infof("selected: %v", driverName)
+	driverName := selectDriver(existing)
+	glog.Infof("selected driver: %s", driverName)
+	validateDriver(driverName, existing)
 	err = autoSetDriverOptions(cmd, driverName)
 	if err != nil {
 		glog.Errorf("Error autoSetOptions : %v", err)
@@ -303,7 +304,7 @@ func runStart(cmd *cobra.Command, args []string) {
 		updateDriver(driverName)
 	}
 
-	k8sVersion, isUpgrade := getKubernetesVersion(oldConfig)
+	k8sVersion, isUpgrade := getKubernetesVersion(existing)
 	config, err := generateCfgFromFlags(cmd, k8sVersion, driverName)
 	if err != nil {
 		exit.WithError("Failed to generate config", err)
@@ -552,67 +553,83 @@ func showKubectlInfo(kcs *kubeconfig.Settings, k8sVersion string) error {
 	return nil
 }
 
-func selectDriver(oldConfig *cfg.Config) string {
+// selectDriver returns which driver to choose based on flags, existing configs, and hypervisor detection
+func selectDriver(existing *cfg.Config) string {
 	name := viper.GetString("vm-driver")
-	glog.Infof("selectDriver: flag=%q, old=%v", name, oldConfig)
-	if name == "" {
-		// By default, the driver is whatever we used last time
-		if oldConfig != nil {
-			return oldConfig.MachineConfig.VMDriver
-		}
-		options := driver.Choices()
-		pick, alts := driver.Choose(options)
-		if len(options) > 1 {
-			out.T(out.Sparkle, `Automatically selected the '{{.driver}}' driver (alternates: {{.alternates}})`, out.V{"driver": pick.Name, "alternates": alts})
-		} else {
-			out.T(out.Sparkle, `Automatically selected the '{{.driver}}' driver`, out.V{"driver": pick.Name})
-		}
+	glog.Infof("selectDriver: flag=%q, old=%v", name, existing)
+	options := driver.Choices()
+	pick, alts := driver.Choose(name, options)
 
-		if pick.Name == "" {
-			exit.WithCodeT(exit.Config, "Unable to determine a default driver to use. Try specifying --vm-driver, or see https://minikube.sigs.k8s.io/docs/start/")
-		}
+	if name != "" {
+		out.T(out.Sparkle, `Selecting '{{.driver}}' driver from user configuration (alternates: {{.alternates}})`, out.V{"driver": name, "alternates": alts})
+		return name
+	}
+
+	// By default, the driver is whatever we used last time
+	if existing != nil {
+		pick, alts := driver.Choose(existing.MachineConfig.VMDriver, options)
+		out.T(out.Sparkle, `Selecting '{{.driver}}' driver from existing profile (alternates: {{.alternates}})`, out.V{"driver": existing.MachineConfig.VMDriver, "alternates": alts})
+		return pick.Name
+	}
 
-		name = pick.Name
+	if len(options) > 1 {
+		out.T(out.Sparkle, `Automatically selected the '{{.driver}}' driver (alternates: {{.alternates}})`, out.V{"driver": pick.Name, "alternates": alts})
+	} else {
+		out.T(out.Sparkle, `Automatically selected the '{{.driver}}' driver`, out.V{"driver": pick.Name})
+	}
+
+	if pick.Name == "" {
+		exit.WithCodeT(exit.Config, "Unable to determine a default driver to use. Try specifying --vm-driver, or see https://minikube.sigs.k8s.io/docs/start/")
 	}
+	return pick.Name
+}
+
+// validateDriver validates that the selected driver appears sane, exits if not
+func validateDriver(name string, existing *cfg.Config) {
+	glog.Infof("validating driver %q against %+v", name, existing)
 	if !driver.Supported(name) {
-		exit.WithCodeT(exit.Failure, "The driver '{{.driver}}' is not supported on {{.os}}", out.V{"driver": name, "os": runtime.GOOS})
+		exit.WithCodeT(exit.Unavailable, "The driver '{{.driver}}' is not supported on {{.os}}", out.V{"driver": name, "os": runtime.GOOS})
 	}
 
 	st := driver.Status(name)
+	glog.Infof("status for %s: %+v", name, st)
+
 	if st.Error != nil {
 		out.ErrLn("")
-		out.WarningT("'{{.driver}}' driver reported a possible issue: {{.error}}", out.V{"driver": name, "error": st.Error, "fix": st.Fix})
+
+		out.WarningT("'{{.driver}}' driver reported an issue: {{.error}}", out.V{"driver": name, "error": st.Error})
 		out.ErrT(out.Tip, "Suggestion: {{.fix}}", out.V{"fix": translate.T(st.Fix)})
 		if st.Doc != "" {
 			out.ErrT(out.Documentation, "Documentation: {{.url}}", out.V{"url": st.Doc})
 		}
 		out.ErrLn("")
+
+		if !st.Installed && !viper.GetBool(force) {
+			if existing != nil && name == existing.MachineConfig.VMDriver {
+				exit.WithCodeT(exit.Unavailable, "{{.driver}} does not appear to be installed, but is specified by an existing profile. Please run 'minikube delete' or install {{.driver}}", out.V{"driver": name})
+			}
+			exit.WithCodeT(exit.Unavailable, "{{.driver}} does not appear to be installed", out.V{"driver": name})
+		}
 	}
 
-	// Detect if our driver conflicts with a previously created VM. If we run into any errors, just move on.
-	api, err := machine.NewAPIClient()
-	if err != nil {
-		glog.Infof("selectDriver NewAPIClient: %v", err)
-		return name
+	if existing == nil {
+		return
 	}
 
-	exists, err := api.Exists(cfg.GetMachineName())
+	api, err := machine.NewAPIClient()
 	if err != nil {
-		glog.Infof("selectDriver api.Exists: %v", err)
-		return name
-	}
-	if !exists {
-		return name
+		glog.Warningf("selectDriver NewAPIClient: %v", err)
+		return
 	}
 
 	h, err := api.Load(cfg.GetMachineName())
 	if err != nil {
-		glog.Infof("selectDriver api.Load: %v", err)
-		return name
+		glog.Warningf("selectDriver api.Load: %v", err)
+		return
 	}
 
-	if h.Driver.DriverName() == name || h.Driver.DriverName() == "not-found" {
-		return name
+	if h.Driver.DriverName() == name {
+		return
 	}
 
 	out.ErrT(out.Conflict, `The existing "{{.profile_name}}" cluster was created using the "{{.old_driver}}" driver, and cannot be started using the "{{.driver}}" driver.`,
@@ -628,7 +645,6 @@ func selectDriver(oldConfig *cfg.Config) string {
 	`, out.V{"command": minikubeCmd(), "old_driver": h.Driver.DriverName(), "profile_name": cfg.GetMachineName()})
 
 	exit.WithCodeT(exit.Config, "Exiting.")
-	return ""
 }
 
 func selectImageRepository(mirrorCountry string, k8sVersion string) (bool, string, error) {

go.mod

@@ -21,14 +21,11 @@ require (
 	github.com/docker/machine v0.7.1-0.20190718054102-a555e4f7a8f5 // version is 0.7.1 to pin to a555e4f7a8f5
 	github.com/elazarl/goproxy v0.0.0-20190421051319-9d40249d3c2f
 	github.com/elazarl/goproxy/ext v0.0.0-20190421051319-9d40249d3c2f // indirect
-	github.com/ghodss/yaml v1.0.0 // indirect
 	github.com/go-ole/go-ole v1.2.4 // indirect
 	github.com/golang-collections/collections v0.0.0-20130729185459-604e922904d3
 	github.com/golang/glog v0.0.0-20160126235308-23def4e6c14b
-	github.com/google/btree v1.0.0 // indirect
 	github.com/google/go-cmp v0.3.0
 	github.com/gorilla/mux v1.7.1 // indirect
-	github.com/grpc-ecosystem/grpc-gateway v1.5.0 // indirect
 	github.com/hashicorp/errwrap v0.0.0-20141028054710-7554cd9344ce // indirect
 	github.com/hashicorp/go-getter v1.4.0
 	github.com/hashicorp/go-multierror v0.0.0-20160811015721-8c5f0ad93604 // indirect

pkg/minikube/driver/driver.go

@@ -95,14 +95,7 @@ func FlagDefaults(name string) FlagHints {
 
 // Choices returns a list of drivers which are possible on this system
 func Choices() []registry.DriverState {
-	options := []registry.DriverState{}
-	for _, ds := range registry.Installed() {
-		if !ds.State.Healthy {
-			glog.Warningf("%q is installed, but unhealthy: %v", ds.Name, ds.State.Error)
-			continue
-		}
-		options = append(options, ds)
-	}
+	options := registry.Available()
 
 	// Descending priority for predictability and appearance
 	sort.Slice(options, func(i, j int) bool {
@@ -112,9 +105,25 @@ func Choices() []registry.DriverState {
 }
 
 // Choose returns a suggested driver from a set of options
-func Choose(options []registry.DriverState) (registry.DriverState, []registry.DriverState) {
+func Choose(requested string, options []registry.DriverState) (registry.DriverState, []registry.DriverState) {
+	glog.Infof("requested: %q", requested)
 	pick := registry.DriverState{}
 	for _, ds := range options {
+		if ds.Name == requested {
+			glog.Infof("choosing %q because it was requested", ds.Name)
+			pick = ds
+			continue
+		}
+
+		if !ds.State.Installed {
+			continue
+		}
+
+		if !ds.State.Healthy {
+			glog.Infof("not recommending %q due to health: %v", ds.Name, ds.State.Error)
+			continue
+		}
+
 		if ds.Priority <= registry.Discouraged {
 			glog.Infof("not recommending %q due to priority: %d", ds.Name, ds.Priority)
 			continue
@@ -128,6 +137,9 @@ func Choose(options []registry.DriverState) (registry.DriverState, []registry.Dr
 	alternates := []registry.DriverState{}
 	for _, ds := range options {
 		if ds != pick {
+			if !ds.State.Healthy || !ds.State.Installed {
+				continue
+			}
 			alternates = append(alternates, ds)
 		}
 	}

pkg/minikube/driver/driver_test.go

@@ -83,18 +83,19 @@ func TestFlagDefaults(t *testing.T) {
 func TestChoices(t *testing.T) {
 
 	tests := []struct {
-		def     registry.DriverDef
-		choices []string
-		pick    string
-		alts    []string
+		def       registry.DriverDef
+		choices   []string
+		pick      string
+		alts      []string
+		requested string
 	}{
 		{
 			def: registry.DriverDef{
 				Name:     "unhealthy",
 				Priority: registry.Default,
 				Status:   func() registry.State { return registry.State{Installed: true, Healthy: false} },
 			},
-			choices: []string{},
+			choices: []string{"unhealthy"},
 			pick:    "",
 			alts:    []string{},
 		},
@@ -104,7 +105,7 @@ func TestChoices(t *testing.T) {
 				Priority: registry.Discouraged,
 				Status:   func() registry.State { return registry.State{Installed: true, Healthy: true} },
 			},
-			choices: []string{"discouraged"},
+			choices: []string{"discouraged", "unhealthy"},
 			pick:    "",
 			alts:    []string{"discouraged"},
 		},
@@ -114,7 +115,7 @@ func TestChoices(t *testing.T) {
 				Priority: registry.Default,
 				Status:   func() registry.State { return registry.State{Installed: true, Healthy: true} },
 			},
-			choices: []string{"default", "discouraged"},
+			choices: []string{"default", "discouraged", "unhealthy"},
 			pick:    "default",
 			alts:    []string{"discouraged"},
 		},
@@ -124,15 +125,23 @@ func TestChoices(t *testing.T) {
 				Priority: registry.Preferred,
 				Status:   func() registry.State { return registry.State{Installed: true, Healthy: true} },
 			},
-			choices: []string{"preferred", "default", "discouraged"},
+			choices: []string{"preferred", "default", "discouraged", "unhealthy"},
 			pick:    "preferred",
 			alts:    []string{"default", "discouraged"},
 		},
+		{
+			requested: "unhealthy",
+			choices:   []string{"preferred", "default", "discouraged", "unhealthy"},
+			pick:      "unhealthy",
+			alts:      []string{"preferred", "default", "discouraged"},
+		},
 	}
 	for _, tc := range tests {
 		t.Run(tc.def.Name, func(t *testing.T) {
-			if err := registry.Register(tc.def); err != nil {
-				t.Errorf("register returned error: %v", err)
+			if tc.def.Name != "" {
+				if err := registry.Register(tc.def); err != nil {
+					t.Errorf("register returned error: %v", err)
+				}
 			}
 
 			got := Choices()
@@ -145,7 +154,7 @@ func TestChoices(t *testing.T) {
 				t.Errorf("choices mismatch (-want +got):\n%s", diff)
 			}
 
-			pick, alts := Choose(got)
+			pick, alts := Choose(tc.requested, got)
 			if pick.Name != tc.pick {
 				t.Errorf("pick = %q, expected %q", pick.Name, tc.pick)
 			}

pkg/minikube/registry/global.go

@@ -18,6 +18,7 @@ package registry
 
 import (
 	"os"
+	"sort"
 
 	"github.com/golang/glog"
 )
@@ -53,8 +54,8 @@ func Driver(name string) DriverDef {
 	return globalRegistry.Driver(name)
 }
 
-// Installed returns a list of installed drivers in the global registry
-func Installed() []DriverState {
+// Available returns a list of available drivers in the global registry
+func Available() []DriverState {
 	sts := []DriverState{}
 	glog.Infof("Querying for installed drivers using PATH=%s", os.Getenv("PATH"))
 
@@ -66,12 +67,18 @@ func Installed() []DriverState {
 		s := d.Status()
 		glog.Infof("%s priority: %d, state: %+v", d.Name, d.Priority, s)
 
-		if !s.Installed {
-			glog.Infof("%q not installed: %v", d.Name, s.Error)
-			continue
+		priority := d.Priority
+		if !s.Healthy {
+			priority = Unhealthy
 		}
-		sts = append(sts, DriverState{Name: d.Name, Priority: d.Priority, State: s})
+
+		sts = append(sts, DriverState{Name: d.Name, Priority: priority, State: s})
 	}
+
+	// Descending priority for predictability
+	sort.Slice(sts, func(i, j int) bool {
+		return sts[i].Priority > sts[j].Priority
+	})
 	return sts
 }
 

pkg/minikube/registry/global_test.go

@@ -64,34 +64,46 @@ func TestGlobalList(t *testing.T) {
 	}
 }
 
-func TestGlobalInstalled(t *testing.T) {
+func TestGlobalAvailable(t *testing.T) {
 	globalRegistry = newRegistry()
 
 	if err := Register(DriverDef{Name: "foo"}); err != nil {
 		t.Errorf("register returned error: %v", err)
 	}
 
 	bar := DriverDef{
-		Name:     "bar",
+		Name:     "healthy-bar",
 		Priority: Default,
-		Status:   func() State { return State{Installed: true} },
+		Status:   func() State { return State{Healthy: true} },
 	}
 	if err := Register(bar); err != nil {
 		t.Errorf("register returned error: %v", err)
 	}
 
+	foo := DriverDef{
+		Name:     "unhealthy-foo",
+		Priority: Default,
+		Status:   func() State { return State{Healthy: false} },
+	}
+	if err := Register(foo); err != nil {
+		t.Errorf("register returned error: %v", err)
+	}
+
 	expected := []DriverState{
 		{
-			Name:     "bar",
+			Name:     "healthy-bar",
 			Priority: Default,
-			State: State{
-				Installed: true,
-			},
+			State:    State{Healthy: true},
+		},
+		{
+			Name:     "unhealthy-foo",
+			Priority: Unhealthy,
+			State:    State{Healthy: false},
 		},
 	}
 
-	if diff := cmp.Diff(Installed(), expected); diff != "" {
-		t.Errorf("installed mismatch (-want +got):\n%s", diff)
+	if diff := cmp.Diff(Available(), expected); diff != "" {
+		t.Errorf("available mismatch (-want +got):\n%s", diff)
 	}
 }