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

none driver doesn't start if /bin/bash doesn't exist. #4122

Closed
alunduil opened this issue Apr 20, 2019 · 18 comments
Closed

none driver doesn't start if /bin/bash doesn't exist. #4122

alunduil opened this issue Apr 20, 2019 · 18 comments
Labels
co/none-driver 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. kind/bug Categorizes issue or PR as related to a bug. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/backlog Higher priority than priority/awaiting-more-evidence.

Comments

@alunduil
Copy link

Command:

minikube --logtostderr start -p sentinel --vm-driver none

Output: https://gist.github.com/alunduil/1e78933095fc69402ec0dc2e0927724f
Minikube logs: https://gist.github.com/alunduil/a05cc1071bb0cf709f875817198d20f8
My os-release: https://gist.github.com/alunduil/f49e9aa43b826de82011eada8dc4cf75

In NixOS, there is no /bin/bash so starting the none driver fails to function. A simple fix for this error would be to use /usr/bin/env to invoke bash by following the path rather than assuming that it's hard coded, but I'm not sure what other assumptions that might break. This solution should work because NixOS ensures a symlink exists at /usr/bin/env that points at the executable and most distributions that I'm aware of ship with /usr/bin/env.

If that sounds like a reasonable solution, I can work on figuring out a pull request (unless someone can get to it faster).

@alunduil
Copy link
Author

I found also, that I cannot delete the machine using minikube delete -p sentinel in this situation but deleting the entries from .minikube/{profiles,machines} seems to have worked.

I also forgot to mention that the following line (for quick reference) has the /bin/bash invocation:

c := exec.Command("/bin/bash", "-c", cmd)

@tstromberg
Copy link
Contributor

Thanks for the bug report! I don't love that minikube requires a shell at all, but this seems like reasonable workaround. My preference however is to call exec.LookPath() instead of double-forking. I'd be happy to review a PR which does so.

Thanks again!

@tstromberg tstromberg added co/none-driver kind/bug Categorizes issue or PR as related to a bug. labels Apr 22, 2019
@tstromberg
Copy link
Contributor

Alternatively - perhaps this should just use /bin/sh. We shouldn't be doing anything bash specific.

@tstromberg tstromberg added 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. labels Apr 22, 2019
@alunduil
Copy link
Author

alunduil commented Apr 23, 2019

@tstromberg I thought about that but didn't know if minikube used any bash specific code in its commands. If that works then it will work on systems like NixOS as well. I'll look into what it will take for me to submit the pull request making that change, but if someone else is also interested in this issue they should feel free to not wait for me.

@afbjorklund
Copy link
Collaborator

Or maybe just continue to use /bin/bash where available, and do the LookPath thing otherwise ?

I haven't investigated how bashist the scripts are, but do recall having to install bash just for minikube...

Supporting NixOS for running the "VM" is probably strectching things.

Hopefully nothing else breaks, otherwise welcome to use Minikube OS.

@alunduil
Copy link
Author

@afbjorklund is minikube no longer the recommended way to setup a development kubernetes cluster without any configuration? In this case, the issue is just the none driver's shell invocations which seem more portable by using sh anyway. I haven't seen minikube OS but I can look into that if it's the recommended replacement for minikube.

@tstromberg is the bash -c there just to lookup the command in the path? If so, then LookPath seems like the better solution in general.

Why would LookPath not be preferred over invoking bash?

@afbjorklund
Copy link
Collaborator

Currently minikube assumes and expects that you start in a VM dedicated for the purpose (of running Kubernetes). The "none" driver bypasses VM allocation and let's the user provide their own VM.

Using minikube to run directly on the laptop (or other developer machine) is not well defined, and assumes things - such as /bin/bash. We can make it better, but VirtualBox is still the supported mainstream.

@alunduil
Copy link
Author

@afbjorklund that makes more sense. Yeah, I'm not looking to do this for the general case but I'm making a test harness that interacts with minikube and running through the motions with the none driver real quick makes the verification that my test harness is working correctly much faster.

I'll see about submitting a pull request for changing the shell used or using the path for the none driver.

Thanks for the caveats to the none driver!

@tstromberg tstromberg added the r/2019q2 Issue was last reviewed 2019q2 label May 24, 2019
@palnabarun
Copy link
Member

@tstromberg I would like to work on the transition to using exec.LookPath. I am a new contributor to kubernetes community.

From the above conversation, I can infer that I have to find the full path of the command using exec.LookPath and then execute the command using the very same workflow.

@alunduil
Copy link
Author

alunduil commented Jun 4, 2019

@palnabarun that's my understanding as well. You can do a quick test that this is working by creating a chroot or other isolated environment and ensure that bash can be found by looking the in the PATH when stored in a non-standard location.

Alternatively, if you want to setup a NixOS VM, testing should be direct in that environment since there isn't any /bin/bash.

@palnabarun
Copy link
Member

Thanks @alunduil for the insights. :) I will test it out inside a NixOS VM.

@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Sep 2, 2019
@tstromberg tstromberg removed the r/2019q2 Issue was last reviewed 2019q2 label Sep 20, 2019
@fejta-bot
Copy link

Stale issues rot after 30d of inactivity.
Mark the issue as fresh with /remove-lifecycle rotten.
Rotten issues close after an additional 30d of inactivity.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle rotten

@k8s-ci-robot k8s-ci-robot added lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Oct 20, 2019
@medyagh
Copy link
Member

medyagh commented Oct 30, 2019

@alunduil I would love to see removing the depdendecy on bash, however some commands we have in bootstrapper, does use bash functions,

as part of this PR #5530
a lot of the commands were converted to exec.Cmd format in this PR but others need more work to do so.

I would review any PR that carefully converts the commands to exec.Cmd format ( with args as oppsed to /bin/bash/ -c

@alunduil
Copy link
Author

Remember, that I'm fine with bash as long as the path is not assumed since different systems may place it elsewhere. Using /usr/bin/env seems to be the canonical way to invoke it without worrying about where it might live, but I'm sure this has problems on some systems as well (I don't know of any but didn't want to assume it was perfect).

@afbjorklund
Copy link
Collaborator

A quick LookPath should fix this.

@fejta-bot
Copy link

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

@k8s-ci-robot
Copy link
Contributor

@fejta-bot: Closing this issue.

In response to this:

Rotten issues close after 30d of inactivity.
Reopen the issue with /reopen.
Mark the issue as fresh with /remove-lifecycle rotten.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
co/none-driver 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. kind/bug Categorizes issue or PR as related to a bug. lifecycle/rotten Denotes an issue or PR that has aged beyond stale and will be auto-closed. priority/backlog Higher priority than priority/awaiting-more-evidence.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants