Merge pull request #5700 from tstromberg/drv-select2

Default to best-available local hypervisor rather than VirtualBox

cmd/minikube/cmd/start.go

@@ -63,6 +63,7 @@ import (
 	"k8s.io/minikube/pkg/minikube/notify"
 	"k8s.io/minikube/pkg/minikube/out"
 	"k8s.io/minikube/pkg/minikube/proxy"
+	"k8s.io/minikube/pkg/minikube/translate"
 	pkgutil "k8s.io/minikube/pkg/util"
 	"k8s.io/minikube/pkg/util/lock"
 	"k8s.io/minikube/pkg/util/retry"
@@ -194,7 +195,7 @@ func initKubernetesFlags() {
 
 // initDriverFlags inits the commandline flags for vm drivers
 func initDriverFlags() {
-	startCmd.Flags().String("vm-driver", "", fmt.Sprintf("Driver is one of: %v (defaults to %s)", driver.SupportedDrivers(), driver.Default()))
+	startCmd.Flags().String("vm-driver", "", fmt.Sprintf("Driver is one of: %v (defaults to auto-detect)", driver.SupportedDrivers()))
 	startCmd.Flags().Bool(disableDriverMounts, false, "Disables the filesystem mounts provided by the hypervisors")
 
 	// kvm2
@@ -289,6 +290,7 @@ func runStart(cmd *cobra.Command, args []string) {
 	}
 
 	driverName := selectDriver(oldConfig)
+	glog.Infof("selected: %v", driverName)
 	err = autoSetDriverOptions(cmd, driverName)
 	if err != nil {
 		glog.Errorf("Error autoSetOptions : %v", err)
@@ -297,11 +299,14 @@ func runStart(cmd *cobra.Command, args []string) {
 	validateFlags(driverName)
 	validateUser(driverName)
 
-	v, err := version.GetSemverVersion()
-	if err != nil {
-		out.WarningT("Error parsing minikube version: {{.error}}", out.V{"error": err})
-	} else if err := driver.InstallOrUpdate(driverName, localpath.MakeMiniPath("bin"), v, viper.GetBool(interactive), viper.GetBool(autoUpdate)); err != nil {
-		out.WarningT("Unable to update {{.driver}} driver: {{.error}}", out.V{"driver": driverName, "error": err})
+	// No need to install a driver in download-only mode
+	if !viper.GetBool(downloadOnly) {
+		v, err := version.GetSemverVersion()
+		if err != nil {
+			out.WarningT("Error parsing minikube version: {{.error}}", out.V{"error": err})
+		} else if err := driver.InstallOrUpdate(driverName, localpath.MakeMiniPath("bin"), v, viper.GetBool(interactive), viper.GetBool(autoUpdate)); err != nil {
+			out.WarningT("Unable to update {{.driver}} driver: {{.error}}", out.V{"driver": driverName, "error": err})
+		}
 	}
 
 	k8sVersion, isUpgrade := getKubernetesVersion(oldConfig)
@@ -534,17 +539,41 @@ func showKubectlInfo(kcs *kubeconfig.Settings, k8sVersion string) error {
 
 func selectDriver(oldConfig *cfg.Config) string {
 	name := viper.GetString("vm-driver")
-	// By default, the driver is whatever we used last time
+	glog.Infof("selectDriver: flag=%q, old=%v", name, oldConfig)
 	if name == "" {
-		name = driver.Default()
+		// 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})
+		}
+
+		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/")
+		}
+
+		name = pick.Name
 	}
 	if !driver.Supported(name) {
 		exit.WithCodeT(exit.Failure, "The driver '{{.driver}}' is not supported on {{.os}}", out.V{"driver": name, "os": runtime.GOOS})
 	}
 
+	st := driver.Status(name)
+	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.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("")
+	}
+
 	// 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 {

hack/jenkins/common.sh

@@ -256,11 +256,14 @@ fi
 
 echo ""
 echo ">> Starting ${E2E_BIN} at $(date)"
+set -x
 ${SUDO_PREFIX}${E2E_BIN} \
   -minikube-start-args="--vm-driver=${VM_DRIVER} ${EXTRA_START_ARGS}" \
+  -expected-default-driver="${EXPECTED_DEFAULT_DRIVER}" \
   -test.timeout=60m \
   -test.parallel=${PARALLEL_COUNT} \
   -binary="${MINIKUBE_BIN}" && result=$? || result=$?
+set +x
 echo ">> ${E2E_BIN} exited with ${result} at $(date)"
 echo ""
 

hack/jenkins/linux_integration_tests_kvm.sh

@@ -29,6 +29,7 @@ OS_ARCH="linux-amd64"
 VM_DRIVER="kvm2"
 JOB_NAME="KVM_Linux"
 PARALLEL_COUNT=4
+EXPECTED_DEFAULT_DRIVER="kvm2"
 
 # Download files and set permissions
 source ./common.sh

hack/jenkins/linux_integration_tests_none.sh

@@ -31,6 +31,7 @@ VM_DRIVER="none"
 JOB_NAME="none_Linux"
 EXTRA_ARGS="--bootstrapper=kubeadm"
 PARALLEL_COUNT=1
+EXPECTED_DEFAULT_DRIVER="kvm2"
 
 SUDO_PREFIX="sudo -E "
 export KUBECONFIG="/root/.kube/config"

hack/jenkins/linux_integration_tests_virtualbox.sh

@@ -29,6 +29,7 @@ OS_ARCH="linux-amd64"
 VM_DRIVER="virtualbox"
 JOB_NAME="VirtualBox_Linux"
 PARALLEL_COUNT=4
+EXPECTED_DEFAULT_DRIVER="kvm2"
 
 # Download files and set permissions
 source ./common.sh

hack/jenkins/osx_integration_tests_hyperkit.sh

@@ -32,6 +32,8 @@ JOB_NAME="HyperKit_macOS"
 EXTRA_ARGS="--bootstrapper=kubeadm"
 EXTRA_START_ARGS=""
 PARALLEL_COUNT=3
+EXPECTED_DEFAULT_DRIVER="hyperkit"
+
 
 # Download files and set permissions
 source common.sh

hack/jenkins/osx_integration_tests_virtualbox.sh

@@ -30,6 +30,10 @@ VM_DRIVER="virtualbox"
 JOB_NAME="VirtualBox_macOS"
 EXTRA_ARGS="--bootstrapper=kubeadm"
 PARALLEL_COUNT=3
+# hyperkit behaves better, so it has higher precedence.
+# Assumes that hyperkit is also installed on the VirtualBox CI host.
+EXPECTED_DEFAULT_DRIVER="hyperkit"
+
 
 # Download files and set permissions
 source common.sh

hack/jenkins/windows_integration_test_hyperv.ps1

@@ -19,7 +19,7 @@ gsutil.cmd -m cp -r gs://minikube-builds/$env:MINIKUBE_LOCATION/testdata .
 
 ./out/minikube-windows-amd64.exe delete
 
-out/e2e-windows-amd64.exe -minikube-start-args="--vm-driver=hyperv --hyperv-virtual-switch=primary-virtual-switch" -binary=out/minikube-windows-amd64.exe -test.v -test.timeout=65m
+out/e2e-windows-amd64.exe --expected-default-driver=hyperv -minikube-start-args="--vm-driver=hyperv --hyperv-virtual-switch=primary-virtual-switch" -binary=out/minikube-windows-amd64.exe -test.v -test.timeout=65m
 $env:result=$lastexitcode
 # If the last exit code was 0->success, x>0->error
 If($env:result -eq 0){$env:status="success"}

hack/jenkins/windows_integration_test_virtualbox.ps1

@@ -19,7 +19,7 @@ gsutil.cmd -m cp -r gs://minikube-builds/$env:MINIKUBE_LOCATION/testdata .
 
 ./out/minikube-windows-amd64.exe delete
 
-out/e2e-windows-amd64.exe -minikube-start-args="--vm-driver=virtualbox" -binary=out/minikube-windows-amd64.exe -test.v -test.timeout=30m
+out/e2e-windows-amd64.exe -minikube-start-args="--vm-driver=virtualbox" -expected-default-driver=hyperv -binary=out/minikube-windows-amd64.exe -test.v -test.timeout=30m
 $env:result=$lastexitcode
 # If the last exit code was 0->success, x>0->error
 If($env:result -eq 0){$env:status="success"}

pkg/minikube/cluster/cluster.go

@@ -433,15 +433,11 @@ func createHost(api libmachine.API, config cfg.MachineConfig) (*host.Host, error
 		}
 	}
 
-	def, err := registry.Driver(config.VMDriver)
-	if err != nil {
-		if err == registry.ErrDriverNotFound {
-			return nil, fmt.Errorf("unsupported/missing driver: %s", config.VMDriver)
-		}
-		return nil, errors.Wrap(err, "error getting driver")
+	def := registry.Driver(config.VMDriver)
+	if def.Empty() {
+		return nil, fmt.Errorf("unsupported/missing driver: %s", config.VMDriver)
 	}
-
-	dd := def.ConfigCreator(config)
+	dd := def.Config(config)
 	data, err := json.Marshal(dd)
 	if err != nil {
 		return nil, errors.Wrap(err, "marshal")

pkg/minikube/cluster/cluster_test.go

@@ -22,8 +22,8 @@ import (
 	"testing"
 	"time"
 
-	// Register drivers
-	_ "k8s.io/minikube/pkg/minikube/registry/drvs"
+	// Driver used by testdata
+	_ "k8s.io/minikube/pkg/minikube/registry/drvs/virtualbox"
 
 	"github.com/docker/machine/libmachine/drivers"
 	"github.com/docker/machine/libmachine/host"
@@ -47,18 +47,13 @@ func createMockDriverHost(c config.MachineConfig) interface{} {
 
 func RegisterMockDriver(t *testing.T) {
 	t.Helper()
-	_, err := registry.Driver(driver.Mock)
-	// Already registered
-	if err == nil {
+	if !registry.Driver(driver.Mock).Empty() {
 		return
 	}
-	err = registry.Register(registry.DriverDef{
-		Name:          driver.Mock,
-		Builtin:       true,
-		ConfigCreator: createMockDriverHost,
-		DriverCreator: func() drivers.Driver {
-			return &tests.MockDriver{T: t}
-		},
+	err := registry.Register(registry.DriverDef{
+		Name:   driver.Mock,
+		Config: createMockDriverHost,
+		Init:   func() drivers.Driver { return &tests.MockDriver{T: t} },
 	})
 	if err != nil {
 		t.Fatalf("register failed: %v", err)
@@ -103,15 +98,15 @@ func TestCreateHost(t *testing.T) {
 	}
 
 	found := false
-	for _, def := range registry.ListDrivers() {
+	for _, def := range registry.List() {
 		if h.DriverName == def.Name {
 			found = true
 			break
 		}
 	}
 
 	if !found {
-		t.Fatalf("Wrong driver name: %v. It should be among drivers %v", h.DriverName, registry.ListDrivers())
+		t.Fatalf("Wrong driver name: %v. It should be among drivers %v", h.DriverName, registry.List())
 	}
 }
 

pkg/minikube/config/config_test.go

@@ -22,8 +22,6 @@ import (
 	"os"
 	"reflect"
 	"testing"
-
-	"k8s.io/minikube/pkg/minikube/driver"
 )
 
 type configTestCase struct {
@@ -48,10 +46,10 @@ var configTestCases = []configTestCase{
     "log_dir": "/etc/hosts",
     "show-libmachine-logs": true,
     "v": 5,
-    "vm-driver": "kvm2"
+    "vm-driver": "test-driver"
 }`,
 		config: map[string]interface{}{
-			"vm-driver":                 driver.KVM2,
+			"vm-driver":                 "test-driver",
 			"cpus":                      4,
 			"disk-size":                 "20g",
 			"v":                         5,
@@ -132,7 +130,7 @@ func TestReadConfig(t *testing.T) {
 	}
 
 	expectedConfig := map[string]interface{}{
-		"vm-driver":            driver.KVM2,
+		"vm-driver":            "test-driver",
 		"cpus":                 4,
 		"disk-size":            "20g",
 		"show-libmachine-logs": true,
@@ -151,7 +149,7 @@ func TestWriteConfig(t *testing.T) {
 	}
 
 	cfg := map[string]interface{}{
-		"vm-driver":            driver.KVM2,
+		"vm-driver":            "test-driver",
 		"cpus":                 4,
 		"disk-size":            "20g",
 		"show-libmachine-logs": true,

pkg/minikube/driver/driver.go

@@ -19,6 +19,10 @@ package driver
 import (
 	"fmt"
 	"os"
+	"sort"
+
+	"github.com/golang/glog"
+	"k8s.io/minikube/pkg/minikube/registry"
 )
 
 const (
@@ -33,6 +37,11 @@ const (
 	Parallels    = "parallels"
 )
 
+var (
+	// systemdResolvConf is path to systemd's DNS configuration. https://github.com/kubernetes/minikube/issues/3511
+	systemdResolvConf = "/run/systemd/resolve/resolv.conf"
+)
+
 // SupportedDrivers returns a list of supported drivers
 func SupportedDrivers() []string {
 	return supportedDrivers
@@ -62,22 +71,63 @@ type FlagHints struct {
 // FlagDefaults returns suggested defaults based on a driver
 func FlagDefaults(name string) FlagHints {
 	if name != None {
-		return FlagHints{}
+		return FlagHints{CacheImages: true}
 	}
 
-	// for more info see: https://github.com/kubernetes/minikube/issues/3511
-	f := "/run/systemd/resolve/resolv.conf"
 	extraOpts := ""
-	if _, err := os.Stat(f); err == nil {
-		extraOpts = fmt.Sprintf("kubelet.resolv-conf=%s", f)
+	if _, err := os.Stat(systemdResolvConf); err == nil {
+		extraOpts = fmt.Sprintf("kubelet.resolv-conf=%s", systemdResolvConf)
 	}
 	return FlagHints{
 		ExtraOptions: extraOpts,
 		CacheImages:  false,
 	}
 }
 
-// Default returns the default driver on this hos
-func Default() string {
-	return VirtualBox
+// 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)
+	}
+
+	// Descending priority for predictability and appearance
+	sort.Slice(options, func(i, j int) bool {
+		return options[i].Priority > options[j].Priority
+	})
+	return options
+}
+
+// Choose returns a suggested driver from a set of options
+func Choose(options []registry.DriverState) (registry.DriverState, []registry.DriverState) {
+	pick := registry.DriverState{}
+	for _, ds := range options {
+		if ds.Priority <= registry.Discouraged {
+			glog.Infof("not recommending %q due to priority: %d", ds.Name, ds.Priority)
+			continue
+		}
+		if ds.Priority > pick.Priority {
+			glog.V(1).Infof("%q has a higher priority (%d) than %q (%d)", ds.Name, ds.Priority, pick.Name, pick.Priority)
+			pick = ds
+		}
+	}
+
+	alternates := []registry.DriverState{}
+	for _, ds := range options {
+		if ds != pick {
+			alternates = append(alternates, ds)
+		}
+	}
+	glog.Infof("Picked: %+v", pick)
+	glog.Infof("Alternatives: %+v", alternates)
+	return pick, alternates
+}
+
+// Status returns the status of a driver
+func Status(name string) registry.State {
+	return registry.Status(name)
 }