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

Add --vm flag for users who want to autoselect only VM's #7068

Merged
merged 4 commits into from
Mar 25, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 2 additions & 1 deletion cmd/minikube/cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ func initDriverFlags() {
startCmd.Flags().String("driver", "", fmt.Sprintf("Driver is one of: %v (defaults to auto-detect)", driver.DisplaySupportedDrivers()))
startCmd.Flags().String("vm-driver", "", "DEPRECATED, use `driver` instead.")
startCmd.Flags().Bool(disableDriverMounts, false, "Disables the filesystem mounts provided by the hypervisors")
startCmd.Flags().Bool("vm", false, "Filter to use only VM Drivers")

// kvm2
startCmd.Flags().String(kvmNetwork, "default", "The KVM network name. (kvm2 driver only)")
Expand Down Expand Up @@ -465,7 +466,7 @@ func selectDriver(existing *config.ClusterConfig) registry.DriverState {
return ds
}

pick, alts := driver.Suggest(driver.Choices())
pick, alts := driver.Suggest(driver.Choices(viper.GetBool("vm")))
if pick.Name == "" {
exit.WithCodeT(exit.Config, "Unable to determine a default driver to use. Try specifying --driver, or see https://minikube.sigs.k8s.io/docs/start/")
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/minikube/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -164,8 +164,8 @@ func FlagDefaults(name string) FlagHints {
}

// Choices returns a list of drivers which are possible on this system
func Choices() []registry.DriverState {
options := registry.Available()
func Choices(vm bool) []registry.DriverState {
options := registry.Available(vm)

// Descending priority for predictability and appearance
sort.Slice(options, func(i, j int) bool {
Expand Down
2 changes: 1 addition & 1 deletion pkg/minikube/driver/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ func TestSuggest(t *testing.T) {
}
}

got := Choices()
got := Choices(false)
gotNames := []string{}
for _, c := range got {
gotNames = append(gotNames, c.Name)
Expand Down
44 changes: 42 additions & 2 deletions pkg/minikube/registry/global.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,40 @@ import (
"github.com/golang/glog"
)

const (
// Podman is Kubernetes in container using podman driver
Podman = "podman"
// Docker is Kubernetes in container using docker driver
Docker = "docker"
// Mock driver
Mock = "mock"
// None driver
None = "none"
)

// IsKIC checks if the driver is a kubernetes in container
func IsKIC(name string) bool {
return name == Docker || name == Podman
}

// IsMock checks if the driver is a mock
func IsMock(name string) bool {
return name == Mock
}

// IsVM checks if the driver is a VM
func IsVM(name string) bool {
Copy link
Contributor

Choose a reason for hiding this comment

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

rename this to isVM -- it does not need to be made public.

if IsKIC(name) || IsMock(name) || BareMetal(name) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I know you just copy and pasted the previous function, but because the other functions aren't used, just inline it:

if name == Docker || name == Mock || name == None || name == Podman {
  return true
}

Then you can remove IsKic and IsMock.

return false
}
return true
}

// BareMetal returns if this driver is unisolated
func BareMetal(name string) bool {
return name == None || name == Mock
}

var (
// globalRegistry is a globally accessible driver registry
globalRegistry = newRegistry()
Expand Down Expand Up @@ -59,7 +93,7 @@ func Driver(name string) DriverDef {
}

// Available returns a list of available drivers in the global registry
func Available() []DriverState {
func Available(vm bool) []DriverState {
Copy link
Contributor

Choose a reason for hiding this comment

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

this might be clearer if it was named vmOnly

sts := []DriverState{}
glog.Infof("Querying for installed drivers using PATH=%s", os.Getenv("PATH"))

Expand All @@ -76,7 +110,13 @@ func Available() []DriverState {
priority = Unhealthy
}

sts = append(sts, DriverState{Name: d.Name, Priority: priority, State: s})
if vm {
Copy link
Contributor

Choose a reason for hiding this comment

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

With go, we try to avoid if/else statements when possible, particularly in a loop. Kind of along the lines of https://github.com/golang/go/wiki/CodeReviewComments#indent-error-flow

This would be clearer:

if vm && !IsVM(d.Name) {
  continue
}

if IsVM(d.Name) {
sts = append(sts, DriverState{Name: d.Name, Priority: priority, State: s})
}
} else {
sts = append(sts, DriverState{Name: d.Name, Priority: priority, State: s})
}
}

// Descending priority for predictability
Expand Down
2 changes: 1 addition & 1 deletion pkg/minikube/registry/global_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ func TestGlobalAvailable(t *testing.T) {
},
}

if diff := cmp.Diff(Available(), expected); diff != "" {
if diff := cmp.Diff(Available(false), expected); diff != "" {
t.Errorf("available mismatch (-want +got):\n%s", diff)
}
}
Expand Down