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.

Sign up for GitHub

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

Jump to bottom

do not crash, when missplelled container runtime #7426

Closed
medyagh opened this issue Apr 5, 2020 · 5 comments · Fixed by #8593
Closed

do not crash, when missplelled container runtime #7426

medyagh opened this issue Apr 5, 2020 · 5 comments · Fixed by #8593
Assignees
@zerefwayne
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.

Comments

@medyagh
Copy link
Member

medyagh commented Apr 5, 2020 •
edited
Loading 

./out/minikube start --driver=docker --container-runtime=conatinerd
😄  minikube v1.9.2 on Darwin 10.13.6
✨  Using the docker driver based on user configuration

💣  Failed to generate config: unknown runtime type: "conatinerd"

😿  minikube is exiting due to an error. If the above message is not useful, open an issue:
👉  https://github.com/kubernetes/minikube/issues/new/choose

in this case i misspelled containerd
and instead of crying and crashing and asking user to create an issue, we could say "conatinerd" is not a valid container-runtime,
the valid options are ...
@medyagh medyagh added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Apr 5, 2020
@zerefwayne
Copy link

@medyagh I would like to work on this one.

@afbjorklund
Copy link
Collaborator

afbjorklund commented Apr 5, 2020 •
edited
Loading

This should be doable, since we already have a hardcoded list of runtimes in there.

runtimes := []string{"containerd", "crio", "docker"}

Arguably this could use a "registry", but then again that is also broken elsewhere.

So we could start by moving the list to a function ?

@afbjorklund afbjorklund added the kind/feature Categorizes issue or PR as related to a new feature. label Apr 5, 2020
@medyagh
Copy link
Member Author

medyagh commented Apr 5, 2020

@zerefwayne you can work on this.

please make sure to parse the conatainer-runtime flag from cobra
and then if it is not in the list that @afbjorklund mentioned, you should exit.WtihUsage and tell the user that they valid options are ...

@zerefwayne
Copy link

/assign

This was referenced Apr 15, 2020
Closed
Closed
@sunny-b
Copy link
Contributor

sunny-b commented Jun 28, 2020

@zerefwayne Are you still working on this? I see the PRs for this issue have been closed. I can take over if you don't have the time to finish them.

@sunny-b sunny-b mentioned this issue Jun 28, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@zerefwayne zerefwayne

Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/feature Categorizes issue or PR as related to a new feature. priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete.
Projects
None yet
Milestone
No milestone
4 participants
@medyagh @afbjorklund @sunny-b @zerefwayne