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

Download our own kvm2 driver if installed version is missing or out of date #4391

Closed
tstromberg opened this issue May 30, 2019 · 14 comments
Closed
Assignees
Labels
co/kvm2-driver KVM2 driver related issues 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. roadmap/2019 Items on the 2019 roadmap
Milestone

Comments

@tstromberg
Copy link
Contributor

Related: #3975 #4387

Also, mentioned in our roadmap: https://github.com/kubernetes/minikube/blob/master/docs/contributors/roadmap.md

@tstromberg tstromberg added kind/feature Categorizes issue or PR as related to a new feature. roadmap/2019 Items on the 2019 roadmap co/kvm2-driver KVM2 driver related issues labels May 30, 2019
@josedonizetti
Copy link
Member

@tstromberg can I work on this? It shouldn't be hard after we have #4387

@medyagh
Copy link
Member

medyagh commented Jul 2, 2019

Indeed @josedonizetti

@josedonizetti
Copy link
Member

Looking at the libmachine code, and it expects the driver to be in the PATH. We can't pass a custom path
of the driver location.

So the download would have to happen to a folder that is already on the path (/usr/local/bin?) but for this, we need sudo access, and I would prefer users not doing sudo minikube start --vm-driver=kvm2 only to have the driver downloaded.

My idea here would be to make the download optional, not force it, and if the user accepts we ask for sudo only at this moment. Something like:

minikube start --vm-driver=kvm2
Starting....
There's a new version for the kvm2 driver. Would you like to install it? [Y/n]
(link to doc to install yourself)
[sudo] password for XXX:

or

minikube start --vm-driver=kvm2
Starting....
You don't have the kvm2 driver installed. Would you like to install it? [Y/n]
(link to doc to install yourself)
[sudo] password for XXX:

@medyagh @afbjorklund @tstromberg wdyt?

@afbjorklund
Copy link
Collaborator

Personally I think that a "solution" message is fine, especially if you installed minikube from package.

It is possible that long-term we want to build kvm2 into minikube, like we do with virtualbox today.

The downside is that it would require everyone to install libvirt, just for the library dependency to work...

But maybe we can dlopen it ? As in only load when needed. Check github.com/libvirt/libvirt-go

@afbjorklund
Copy link
Collaborator

We could cut down on the dependencies, by "only" loading libvirt.so.0 and libvirt-qemu.so.0 ?

By default the binding will support APIs in libvirt.so, libvirt-qemu.so and libvirt-lxc.so. Coverage for the latter two libraries can be dropped from the build using build tags 'without_qemu' or 'without_lxc' respectively.

https://github.com/coreos/pkg/tree/master/dlopen

@tstromberg
Copy link
Contributor Author

Looking at the libmachine code, and it expects the driver to be in the PATH. We can't pass a custom path
of the driver location.

So the download would have to happen to a folder that is already on the path (/usr/local/bin?) but for this, we need sudo access, and I would prefer users not doing sudo minikube start --vm-driver=kvm2 only to have the driver downloaded.

My idea here would be to make the download optional, not force it, and if the user accepts we ask for sudo only at this moment. Something like:

minikube start --vm-driver=kvm2
Starting....
There's a new version for the kvm2 driver. Would you like to install it? [Y/n]
(link to doc to install yourself)
[sudo] password for XXX:

or

minikube start --vm-driver=kvm2
Starting....
You don't have the kvm2 driver installed. Would you like to install it? [Y/n]
(link to doc to install yourself)
[sudo] password for XXX:

@medyagh @afbjorklund @tstromberg wdyt?

Great question - and great work here.

What do you think about setting the PATH to the version of the driver we have downloaded, just prior to invoking it with machine-driver?

IMHO, there is no need to ask the user about downloading the driver if the installed one is incompatible or unavailable, similarly to how we don't ask the user about the built-in VM drivers or kubeadm. We should be able to treat the VM driver as an extension of our code, albeit one which has some library awkwardness.

For hyperkit, we'll need to ask about calling sudo to bless the hyperkit download appropriately.

@afbjorklund
Copy link
Collaborator

Loading libvirt "on demand" looks like it will be horrible to implement. So it will probably stay external.

@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 Jul 12, 2019
@afbjorklund
Copy link
Collaborator

I included the drivers in the tarballs and in the packages. I still think they "belong" to libmachine.
Since we are not installing any system packages with minikube, that is probably as good as it gets ?

Shouldn't be much worse for the end user, they can still just add the drivers to the install command.
Instead of "just" installing minikube, they can also add docker-machine-driver-kvm2 (or hyperkit)

@josedonizetti
Copy link
Member

@tstromberg @medyagh @afbjorklund To confirm, download docker-machine-driver-kvm2/hyperkit to .minikube is something we want to implement? Or given this on the package/tarballs, warning the user of the old version should be enough?

@afbjorklund
Copy link
Collaborator

I'm getting mixed messages :-) I think the important thing is making it easier for the user, either to install the required components (like kubectl and docker-machine-driver-kvm2) or to download those for them...

@medyagh
Copy link
Member

medyagh commented Jul 16, 2019

@josedonizetti Yes we still want to do this ! currently we have one place for bins in ~/.minikube folder for kubelet and kubeadm

~/.minikube/cache/v1.15.0/ but that one is per version of kuberentes for this case I recommend using a new one ~/.minikube/bin and it is okay in the cases that we have to sudo, to ask for sudo. (hyperkit)

@tstromberg
Copy link
Contributor Author

Any update here?

The intent here is to preserve the ability users to download a single minikube binary, but still allow them to use the latest kvm2 driver if the pre-requisites have been fulfilled otherwise. This would for instance, make it easier for tools that want to embed minikube but not require additional privileges.

In a more perfect world, these users would all be installing minikube and the kvm2 driver from their native package manager. We should aim for that as a parallel effort.

@afbjorklund
Copy link
Collaborator

Only on the parallel effort, and we had some feedback about replacing other general system programs such as kubectl (in this case it would be docker-machine-driver-kvm2 binary, for docker-machine)

This would for instance, make it easier for tools that want to embed minikube but not require additional privileges.

Using the libvirt driver will always require additional privileges (being part of the root-equivalent "libvirtd" group), but it could possibly avoid the sudo requirement by placing it somewhere else along the PATH

@afbjorklund
Copy link
Collaborator

In a more perfect world, these users would all be installing minikube and the kvm2 driver from their native package manager. We should aim for that as a parallel effort.

deb

rpm

Maybe we should use GCS for both, and not have GitHub as our main location for the packages ?
But the important part is adding support for apt and for yum, including repo and package signing...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
co/kvm2-driver KVM2 driver related issues 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. roadmap/2019 Items on the 2019 roadmap
Projects
None yet
Development

No branches or pull requests

4 participants