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 rejection reason to 'unable to find driver' error #7379

Merged
merged 2 commits into from
Apr 2, 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
10 changes: 8 additions & 2 deletions cmd/minikube/cmd/start.go
Original file line number Diff line number Diff line change
Expand Up @@ -561,9 +561,15 @@ func selectDriver(existing *config.ClusterConfig) registry.DriverState {
return ds
}

pick, alts := driver.Suggest(driver.Choices(viper.GetBool("vm")))
choices := driver.Choices(viper.GetBool("vm"))
pick, alts, rejects := driver.Suggest(choices)
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/")
out.T(out.ThumbsDown, "Unable to pick a default driver. Here is what was considered, in preference order:")
for _, r := range rejects {
out.T(out.Option, "{{ .name }}: {{ .rejection }}", out.V{"name": r.Name, "rejection": r.Rejection})
}
out.T(out.Workaround, "Try specifying a --driver, or see https://minikube.sigs.k8s.io/docs/start/")
os.Exit(exit.Unavailable)
}

if len(alts) > 1 {
Expand Down
21 changes: 17 additions & 4 deletions pkg/minikube/driver/driver.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ func Choices(vm bool) []registry.DriverState {
return options
}

// Suggest returns a suggested driver from a set of options
func Suggest(options []registry.DriverState) (registry.DriverState, []registry.DriverState) {
// Suggest returns a suggested driver, alternate drivers, and rejected drivers
func Suggest(options []registry.DriverState) (registry.DriverState, []registry.DriverState, []registry.DriverState) {
pick := registry.DriverState{}
for _, ds := range options {
if !ds.State.Installed {
Expand All @@ -198,17 +198,30 @@ func Suggest(options []registry.DriverState) (registry.DriverState, []registry.D
}

alternates := []registry.DriverState{}
rejects := []registry.DriverState{}
for _, ds := range options {
if ds != pick {
if !ds.State.Healthy || !ds.State.Installed {
glog.Errorf("%s: %s", ds.Name, ds.Rejection)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like all this logic could be done in one loop instead of two, though I'm not sure if there's a real advantage of either strategy.

Otherwise this PR is great.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree, but I wasn't smart enough to figure out how without making it overly complicated.

We aren't sure who to pick until the end of the first loop - so no matter what, we need to iterate twice to part out the alternates. I'l leave this for someone better at reasoning about algorithms.

if !ds.State.Installed {
ds.Rejection = fmt.Sprintf("Not installed: %v", ds.State.Error)
rejects = append(rejects, ds)
continue
}

if !ds.State.Healthy {
ds.Rejection = fmt.Sprintf("Not healthy: %v", ds.State.Error)
rejects = append(rejects, ds)
continue
}

ds.Rejection = fmt.Sprintf("%s is preferred", pick.Name)
alternates = append(alternates, ds)
}
}
glog.Infof("Picked: %+v", pick)
glog.Infof("Alternatives: %+v", alternates)
return pick, alternates
glog.Infof("Rejects: %+v", rejects)
return pick, alternates, rejects
}

// Status returns the status of a driver
Expand Down
16 changes: 15 additions & 1 deletion pkg/minikube/driver/driver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ func TestSuggest(t *testing.T) {
choices []string
pick string
alts []string
rejects []string
}{
{
def: registry.DriverDef{
Expand All @@ -122,6 +123,7 @@ func TestSuggest(t *testing.T) {
choices: []string{"unhealthy"},
pick: "",
alts: []string{},
rejects: []string{"unhealthy"},
},
{
def: registry.DriverDef{
Expand All @@ -132,6 +134,7 @@ func TestSuggest(t *testing.T) {
choices: []string{"discouraged", "unhealthy"},
pick: "",
alts: []string{"discouraged"},
rejects: []string{"unhealthy"},
},
{
def: registry.DriverDef{
Expand All @@ -142,6 +145,7 @@ func TestSuggest(t *testing.T) {
choices: []string{"default", "discouraged", "unhealthy"},
pick: "default",
alts: []string{"discouraged"},
rejects: []string{"unhealthy"},
},
{
def: registry.DriverDef{
Expand All @@ -152,6 +156,7 @@ func TestSuggest(t *testing.T) {
choices: []string{"preferred", "default", "discouraged", "unhealthy"},
pick: "preferred",
alts: []string{"default", "discouraged"},
rejects: []string{"unhealthy"},
},
}
for _, tc := range tests {
Expand All @@ -172,7 +177,7 @@ func TestSuggest(t *testing.T) {
t.Errorf("choices mismatch (-want +got):\n%s", diff)
}

pick, alts := Suggest(got)
pick, alts, rejects := Suggest(got)
if pick.Name != tc.pick {
t.Errorf("pick = %q, expected %q", pick.Name, tc.pick)
}
Expand All @@ -184,6 +189,15 @@ func TestSuggest(t *testing.T) {
if diff := cmp.Diff(gotAlts, tc.alts); diff != "" {
t.Errorf("alts mismatch (-want +got):\n%s", diff)
}

gotRejects := []string{}
for _, r := range rejects {
gotRejects = append(gotRejects, r.Name)
}
if diff := cmp.Diff(gotRejects, tc.rejects); diff != "" {
t.Errorf("rejects mismatch (-want +got):\n%s", diff)
}

})
}
}
1 change: 1 addition & 0 deletions pkg/minikube/out/style.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ var styles = map[StyleEnum]style{
Launch: {Prefix: "πŸš€ "},
Sad: {Prefix: "😿 "},
ThumbsUp: {Prefix: "πŸ‘ "},
ThumbsDown: {Prefix: "πŸ‘Ž "},
Option: {Prefix: " β–ͺ ", LowPrefix: lowIndent}, // Indented bullet
Command: {Prefix: " β–ͺ ", LowPrefix: lowIndent}, // Indented bullet
LogEntry: {Prefix: " "}, // Indent
Expand Down
1 change: 1 addition & 0 deletions pkg/minikube/out/style_enum.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ const (
Launch
Sad
ThumbsUp
ThumbsDown
Option
Command
LogEntry
Expand Down
2 changes: 2 additions & 0 deletions pkg/minikube/registry/global.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ type DriverState struct {
Name string
Priority Priority
State State
// Rejection is why we chose not to use this driver
Rejection string
}

func (d DriverState) String() string {
Expand Down