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

Provide solution message for unnecessary sudo #4420

Closed
medyagh opened this issue Jun 4, 2019 · 6 comments · Fixed by #4455
Closed

Provide solution message for unnecessary sudo #4420

medyagh opened this issue Jun 4, 2019 · 6 comments · Fixed by #4455
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. priority/backlog Higher priority than priority/awaiting-more-evidence.

Comments

@medyagh
Copy link
Member

medyagh commented Jun 4, 2019

Running minikube with "sudo" privileges is not necessary in some cases causes issues. (such as host env vars)

We need to provide a solution message (warning) not to run minikube in sudo:

"Please don't run minikube as root or with "sudo" privileges. It isn't necessary."

@medyagh medyagh added help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. needs-solution-message Issues where where offering a solution for an error would be helpful labels Jun 4, 2019
@djmgit
Copy link
Contributor

djmgit commented Jun 4, 2019

@medyagh I would like to work on this.
I am new to kube contribution, any hints and pointers will be of great help :)

@medyagh
Copy link
Member Author

medyagh commented Jun 4, 2019

Awesome @djmgit ! Thank you for offering to help, yes please.

I look forward to review your PR :) please make sure to sign the CLA

@kubernetes kubernetes deleted a comment from k8s-ci-robot Jun 4, 2019
@medyagh
Copy link
Member Author

medyagh commented Jun 5, 2019

@djmgit I suggest using golang "os/user" std lib package:
here are things u can do :

import "os/user"

u, err := user.Current()
// check for errors
// check if u.Username is root , then add a console warning with a right emoji (check the start cmd for the example console warnings) 

important thing is make sure it works on both windows and mac and linux ( the go package seems to be working on all but make sure )

@medyagh
Copy link
Member Author

medyagh commented Jun 6, 2019

@djmgit one more detail is, the sudo is required for --vm-driver hyperv so the message should not warn users who are using hyperv

@sharifelgamal sharifelgamal added the priority/backlog Higher priority than priority/awaiting-more-evidence. label Jun 6, 2019
@djmgit
Copy link
Contributor

djmgit commented Jun 9, 2019

@medyagh thanks a lot for the pointers, I will send a PR ASAP

djmgit pushed a commit to djmgit/minikube that referenced this issue Jun 10, 2019
Fixes kubernetes#4420. If minikue is started with root privilege and vm-driver is
not hyperv, then a warning message is displayed.
djmgit added a commit to djmgit/minikube that referenced this issue Jun 10, 2019
Fixes kubernetes#4420. If minikue is started with root privilege and vm-driver is
not hyperv, then a warning message is displayed.
@djmgit
Copy link
Contributor

djmgit commented Jun 10, 2019

@medyagh I have submitted a PR for this. Let me know what changes need to be made.

@tstromberg tstromberg removed the needs-solution-message Issues where where offering a solution for an error would be helpful label Apr 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants