Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Default to best-available local hypervisor rather than VirtualBox #5700

Merged
merged 28 commits into from
Oct 25, 2019
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
1fc11f5
Add logic for picking the default driver on a host
tstromberg Oct 23, 2019
680b542
Rename package files appropriately
tstromberg Oct 23, 2019
443ae74
Make unit tests pass again
tstromberg Oct 23, 2019
8631587
Merge branch 'master' into drv-select2
tstromberg Oct 23, 2019
4b63c0d
Update hyperv driver for recent changes
tstromberg Oct 23, 2019
5d4bf08
Merge branch 'master' into drv-select2
tstromberg Oct 23, 2019
118783c
Remove dependency between minikube and kvm driver
tstromberg Oct 23, 2019
90ac694
Add tests for new registry methods, split package into two files
tstromberg Oct 24, 2019
c4fc450
Add unit tests for driver
tstromberg Oct 24, 2019
dcdac36
Add integration tests
tstromberg Oct 24, 2019
bdd937d
Adjust -expected-default-driver invocation
tstromberg Oct 24, 2019
bdd0f38
Merge branch 'drv-select2' of github.com:tstromberg/minikube into drv…
tstromberg Oct 24, 2019
c925f65
Warn about driver status, add more logging
tstromberg Oct 24, 2019
8920a88
Minor improvements to the clarity of hypervisor errors
tstromberg Oct 24, 2019
b004527
Merge branch 'drv-select2' of github.com:tstromberg/minikube into drv…
tstromberg Oct 24, 2019
d94fc65
Merge branch 'master' into drv-select2
tstromberg Oct 24, 2019
82cb5a1
Fix extra comma
tstromberg Oct 24, 2019
f9b0c77
Include output in the error message
tstromberg Oct 24, 2019
586f300
Merge
tstromberg Oct 24, 2019
c2fd3c6
Move expected driver check to its own subtest
tstromberg Oct 24, 2019
fda1aba
Merge branch 'drv-select2' of github.com:tstromberg/minikube into drv…
tstromberg Oct 24, 2019
33ef4e1
Add comment explaining why EXPECTED_DEFAULT_DRIVER=hyperkit
tstromberg Oct 24, 2019
c153919
Mark none driver as Linux only
tstromberg Oct 25, 2019
c50a849
Add more debug logs
tstromberg Oct 25, 2019
04acb59
Use virsh domcapabilites instead of virt-host-validate as it behaves …
tstromberg Oct 25, 2019
d3618fb
Merge branch 'drv-select2' of github.com:tstromberg/minikube into drv…
tstromberg Oct 25, 2019
416f132
Pass --expected-default-driver to the test rather than minikube
tstromberg Oct 25, 2019
e46502c
Add automatic selection message even when there is only one choice
tstromberg Oct 25, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 35 additions & 8 deletions cmd/minikube/cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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()))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a random thought:
if auto-detect is not expensive, maybe add it to the help message. so the user doesnt have to guess what the auto-detect comes up with ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but thinking again, maybe not !

startCmd.Flags().Bool(disableDriverMounts, false, "Disables the filesystem mounts provided by the hypervisors")

// kvm2
Expand Down Expand Up @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -534,17 +539,39 @@ 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})
}

if pick.Name == "" {
exit.WithCodeT(exit.Config, "Unable to determine a default driver to use. Try specifying --vm-driver, or visiting 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 {
Expand Down
3 changes: 3 additions & 0 deletions hack/jenkins/common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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 ""

Expand Down
1 change: 1 addition & 0 deletions hack/jenkins/linux_integration_tests_kvm.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
1 change: 1 addition & 0 deletions hack/jenkins/linux_integration_tests_none.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
1 change: 1 addition & 0 deletions hack/jenkins/linux_integration_tests_virtualbox.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 2 additions & 0 deletions hack/jenkins/osx_integration_tests_hyperkit.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ JOB_NAME="HyperKit_macOS"
EXTRA_ARGS="--bootstrapper=kubeadm"
EXTRA_START_ARGS=""
PARALLEL_COUNT=3
EXPECTED_DEFAULT_DRIVER="hyperkit"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no need for extra line


# Download files and set permissions
source common.sh
2 changes: 2 additions & 0 deletions hack/jenkins/osx_integration_tests_virtualbox.sh
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ VM_DRIVER="virtualbox"
JOB_NAME="VirtualBox_macOS"
EXTRA_ARGS="--bootstrapper=kubeadm"
PARALLEL_COUNT=3
EXPECTED_DEFAULT_DRIVER="hyperkit"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

virtualbox

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If hyperkit is installed on the machine, this will be hyperkit as it takes precedence. I assume our VirtualBox mac also has hyperkit installed.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point!



# Download files and set permissions
source common.sh
2 changes: 1 addition & 1 deletion hack/jenkins/windows_integration_test_hyperv.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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 -minikube-start-args="--vm-driver=hyperv --expected-default-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"}
Expand Down
2 changes: 1 addition & 1 deletion hack/jenkins/windows_integration_test_virtualbox.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -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"}
Expand Down
12 changes: 4 additions & 8 deletions pkg/minikube/cluster/cluster.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
23 changes: 9 additions & 14 deletions pkg/minikube/cluster/cluster_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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)
Expand Down Expand Up @@ -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())
}
}

Expand Down
10 changes: 4 additions & 6 deletions pkg/minikube/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ import (
"os"
"reflect"
"testing"

"k8s.io/minikube/pkg/minikube/driver"
)

type configTestCase struct {
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
65 changes: 57 additions & 8 deletions pkg/minikube/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ package driver
import (
"fmt"
"os"
"sort"

"github.com/golang/glog"
"k8s.io/minikube/pkg/minikube/registry"
)

const (
Expand All @@ -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
Expand Down Expand Up @@ -62,22 +71,62 @@ 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 {
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)
}
Loading