-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Changes from all commits
1fc11f5
680b542
443ae74
8631587
4b63c0d
5d4bf08
118783c
90ac694
c4fc450
dcdac36
bdd937d
bdd0f38
c925f65
8920a88
b004527
d94fc65
82cb5a1
f9b0c77
586f300
c2fd3c6
fda1aba
33ef4e1
c153919
c50a849
04acb59
d3618fb
416f132
e46502c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe put the version of Virsh or Hyperkit.... would help a lot in debugging ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Displaying the hypervisor version would be extremely useful, but also a fair bit of work to do across hypervisors, so I would rather not include it in this already large PR. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sounds good. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This can also be done as part of hte drivers themselves, as libmachine allows to log these values... this way using the verbose/debug logs for this would collect them. This is also what we do for CRC. |
||
} | ||
|
||
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 { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,8 @@ JOB_NAME="HyperKit_macOS" | |
EXTRA_ARGS="--bootstrapper=kubeadm" | ||
EXTRA_START_ARGS="" | ||
PARALLEL_COUNT=3 | ||
EXPECTED_DEFAULT_DRIVER="hyperkit" | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. virtualbox There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good point! |
||
|
||
|
||
# Download files and set permissions | ||
source common.sh |
There was a problem hiding this comment.
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 ?
There was a problem hiding this comment.
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 !