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

Conversation

tstromberg
Copy link
Contributor

@tstromberg tstromberg commented Apr 2, 2020

Our current error message is not as helpful as it could be:

💣 Unable to determine a default driver to use. Try specifying --driver, or see https://minikube.sigs.k8s.io/docs/start/

Here is the new error message this PR proposes:

👎  Unable to pick a default driver. Here is what was considered, in preference order:
    ▪ parallels: Not installed: exec: "docker-machine-driver-parallels": executable file not found in $PATH
    ▪ podman: Not installed: exec: "podman": executable file not found in $PATH
    ▪ vmware: Not installed: exec: "docker-machine-driver-vmware": executable file not found in $PATH
👉  Try specifying a --driver, or see https://minikube.sigs.k8s.io/docs/start/

Solves mysteries such as #7371

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 2, 2020
@tstromberg tstromberg requested a review from medyagh April 2, 2020 13:23
@k8s-ci-robot k8s-ci-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Apr 2, 2020
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tstromberg

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@k8s-ci-robot k8s-ci-robot requested a review from RA489 April 2, 2020 13:23
@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Apr 2, 2020
@codecov-io
Copy link

Codecov Report

Merging #7379 into master will increase coverage by 0.00%.
The diff coverage is 50.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #7379   +/-   ##
=======================================
  Coverage   37.18%   37.19%           
=======================================
  Files         145      145           
  Lines        8836     8851   +15     
=======================================
+ Hits         3286     3292    +6     
- Misses       5154     5162    +8     
- Partials      396      397    +1     
Impacted Files Coverage Δ
cmd/minikube/cmd/start.go 34.34% <0.00%> (-0.27%) ⬇️
pkg/minikube/out/style.go 91.66% <ø> (ø)
pkg/minikube/registry/global.go 58.69% <ø> (ø)
pkg/minikube/driver/driver.go 64.00% <76.92%> (-0.45%) ⬇️

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.

@tstromberg
Copy link
Contributor Author

/ok-to-test

@k8s-ci-robot k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Apr 2, 2020
@minikube-pr-bot
Copy link

Error: running mkcmp: exit status 1

@tstromberg tstromberg merged commit e937a60 into kubernetes:master Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants