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: minikube shouldn't write binaries to /usr/bin #3718

Closed
tstromberg opened this issue Feb 19, 2019 · 14 comments
Closed

none driver: minikube shouldn't write binaries to /usr/bin #3718

tstromberg opened this issue Feb 19, 2019 · 14 comments
Assignees
Labels
co/none-driver 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. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. r/2019q2 Issue was last reviewed 2019q2
Milestone

Comments

@tstromberg
Copy link
Contributor

With the none driver, minikube will overwrite the following system paths:

  • /usr/bin/kubeadm - Updated to match the exact version of Kubernetes selected
  • /usr/bin/kubectl - Updated to match the exact version of Kubernetes selected

Basically, anywhere else is better, but to write directly to /usr/bin feels yucky. It'd be nicer to write the binaries somewhere else, preferably but not necessarily versioned. For instance:

$XDG_CACHE_HOME/minikube/binaries/kubeadm-v1.13.0

Or:

/usr/local/minikube/kubeadm-v1.13.0

@tstromberg tstromberg changed the title minikube shouldn't write binaries to /usr/bin none driver: minikube shouldn't write binaries to /usr/bin Feb 19, 2019
@tstromberg tstromberg added the kind/bug Categorizes issue or PR as related to a bug. label Feb 19, 2019
@afbjorklund
Copy link
Collaborator

I think it is perfectly reasonable to install kubeadm to the default /usr/bin location.
The same thing happens, if you install it from an .rpm and a .deb for instance ?

https://kubernetes.io/docs/setup/independent/install-kubeadm/#installing-kubeadm-kubelet-and-kubectl

After all, the VM you give to minikube is dedicated to running kubernetes on.
You are not giving it something important - like directly on your laptop, right ? ;-)

https://github.com/kubernetes/minikube/blob/master/docs/vmdriver-none.md

@afbjorklund
Copy link
Collaborator

Could install to /usr/local/bin, but I'm not sure that would help anything.

@afbjorklund
Copy link
Collaborator

With the none driver, minikube will overwrite the following system paths:

  • /usr/bin/kubeadm - Updated to match the exact version of Kubernetes selected
  • /usr/bin/kubectl - Updated to match the exact version of Kubernetes selected

You probably mean "/usr/bin/kubelet" here ? We don't install kubectl - yet.

@tstromberg
Copy link
Contributor Author

tstromberg commented Feb 19, 2019

Using /usr/bin violates both user expectations and the LSB/FHS standards: https://refspecs.linuxfoundation.org/FHS_3.0/fhs/ch04s09.html

The main user expectation violated is that we may be overwriting system binaries.

Storing files in /usr/bin also conflicts with the goal of reducing/removing usage of 'minikube' by the root user: #3138

@tstromberg tstromberg added the priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. label Feb 20, 2019
@afbjorklund
Copy link
Collaborator

afbjorklund commented Feb 20, 2019

@tstromberg : I guess I am still thinking about the old/existing implementation, when you run sudo -E minikube start --vm-driver=none locally on your dedicated VM - rather than the new version of actually running it on the same machine as the 'kubectl` like your laptop or so. When running locally, it makes sense to not overwrite system binaries and to run minikube without root (by it using sudo where it is needed).

Maybe we could use the binaries straight off the cache, rather than copying them to a temp location ?
~/.minikube/cache/v1.13.0/kubeadm vs ~/.cache/minikube/binaries/kubeadm-v1.13.0

@MatejLach
Copy link

Is there a workaround for now?

@afbjorklund
Copy link
Collaborator

Suppose you could store the old files, and then restore/reinstall them after minikube ?

@tstromberg
Copy link
Contributor Author

I did some prototyping where the binaries are stored in /data/minikube, and it worked great. The plus side is that it's persistent across reboots, which speeds up minikube.

@medyagh medyagh added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Jul 2, 2019
@medyagh
Copy link
Member

medyagh commented Jul 2, 2019

I also think having it in /usr/bin is a bad idea, since many corporates deny access to that folder but they allow /usr/loca/bin

@afbjorklund
Copy link
Collaborator

afbjorklund commented Jul 6, 2019

Normally this is because /usr is handled by the package manager, while /usr/local is not.

Note that for the current release, we could run them right off the cache (now made executable)
This was originally intended for kubectl, but ended up the same for kubeadm and kubelet too:

$ ~/.minikube/cache/v1.15.0/kubeadm version
kubeadm version: &version.Info{Major:"1", Minor:"15", GitVersion:"v1.15.0", GitCommit:"e8462b5b5dc2584fdcd18e6bcfe9f1e4d970a529", GitTreeState:"clean", BuildDate:"2019-06-19T16:37:41Z", GoVersion:"go1.12.5", Compiler:"gc", Platform:"linux/amd64"}
$ ~/.minikube/cache/v1.15.0/kubelet --version
Kubernetes v1.15.0

It's not in the PATH though, and there are some changes needed to the created startup scripts:

https://github.com/kubernetes/minikube/blob/master/pkg/minikube/bootstrapper/kubeadm/kubeadm.go

https://github.com/kubernetes/minikube/blob/master/pkg/minikube/bootstrapper/kubeadm/templates.go

@tstromberg tstromberg added priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Jul 12, 2019
@rzr
Copy link

rzr commented Aug 3, 2019

well do you agree that the minikube.deb package could write to /usr/bin
as long as there is a conflict with kubeadm.deb

grep -i Conflicts  ./installers/linux/deb/*deb*/DEBIAN/control

Is that correct ? I can open a PR to fix this

The nicer way would be to use alternatives links.

BTW, way isnt minikube.deb in same repo as other k8 debs ?

@afbjorklund
Copy link
Collaborator

The minikube.deb will install /usr/bin/minikube, there should be no conflict ?

For the none driver, we will either use ~/.minikube/cache or the other deb.
Currently we install /usr/bin/kubeadm, but that's a bug. Specifically, this one.

So far, minikube has just pulled the static binaries - and installed them to /usr/bin.
That should probably have been /usr/local/bin, but that's not ideal either...

https://storage.googleapis.com/kubernetes-release/release/v1.15.1/bin/linux/amd64/kubeadm

The "other" kubeadm.deb would be this one: kubeadm_1.15.1-00_amd64_*.deb
But we haven't really had a policy for installing minikube with apt and yum just yet.

https://kubernetes.io/docs/setup/production-environment/tools/kubeadm/install-kubeadm/


Theoretically it could use the deb and rpm packages - on the supported platforms ?
This is similar to how the docker installation works, when using https://get.docker.com

And then continue to use the static binaries, on non-ubuntu and non-centos distros.
For instance when using our standard minikube distribution, i.e. the minikube.iso

It's all for show anyway, since it is the same binary packaged in several multiple ways.

3d42441ae177826f1181e559cd2a729464ca8efadef196cfa0e8053a615333b5  kubeadm
3d42441ae177826f1181e559cd2a729464ca8efadef196cfa0e8053a615333b5  usr/bin/kubeadm

One benefit with our approach is that we can have multiple versions - at the same time.

@afbjorklund
Copy link
Collaborator

afbjorklund commented Aug 3, 2019

BTW, way isnt minikube.deb in same repo as other k8 debs ?

Sadly, not yet. There's some work under way, to improve this.

#3110 #4716

@tstromberg
Copy link
Contributor Author

tstromberg commented Aug 22, 2019

Fix merged to head.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
co/none-driver 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. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. r/2019q2 Issue was last reviewed 2019q2
Projects
None yet
Development

No branches or pull requests

5 participants