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

Move kvm2 driver upstream (was: machine-drivers/docker-machine-driver-libvirt) #3169

Closed
tstromberg opened this issue Sep 26, 2018 · 19 comments
Closed
Labels
area/code-deps Code dependencies (guest-vm deps belong in guest-vm) co/kvm2-driver KVM2 driver related issues help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. 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. r/2019q2 Issue was last reviewed 2019q2

Comments

@tstromberg
Copy link
Contributor

Evidently it has some fixes we might be interested in.

We'll need to ensure that our local improvements to dhiltgen/docker-machine-kvm are also represented in machine-drivers/docker-machine-driver-libvirt

@mheese
Copy link
Contributor

mheese commented Sep 26, 2018

ok, let me reference the issue on "the other side" here: machine-drivers/docker-machine-kvm#3

@tstromberg tstromberg added area/code-deps Code dependencies (guest-vm deps belong in guest-vm) co/kvm2-driver KVM2 driver related issues labels Sep 27, 2018
@afbjorklund
Copy link
Collaborator

Currently that URL contains the KVM driver, not the KVM2 driver:

https://github.com/dhiltgen/docker-machine-kvm/releases/tag/v0.10.0 ==
https://github.com/machine-drivers/docker-machine-driver-libvirt/releases/tag/v0.10.0

And the prebuilt binaries are only available at the previous location...


Once the two drivers have been merged, it's probably a good idea.

But at the moment, the driver location is still kubernetes/minikube

@mheese
Copy link
Contributor

mheese commented Sep 29, 2018

@afbjorklund, this is why I referenced the issue from the machine-drivers repo. The KVM2 driver changes need to get merged there first. @tstromberg and I had a quick discussion about this before, I guess we should have been more explicit in the ticket here. Sorry for the confusion. I'm planning on working on a PR to the machine-drivers repo next week.

@afbjorklund
Copy link
Collaborator

The renaming was also slightly premature, just didn't want it to be named "kvm2" and thought the name of "docker-machine-kvm" was confusing and it didn't really talk to kvm (like our qemu did) but used libvirt...

See dhiltgen/docker-machine-kvm#67

@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 Dec 29, 2018
@tstromberg tstromberg added priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. and removed lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. labels Jan 23, 2019
@tstromberg
Copy link
Contributor Author

tstromberg commented Jan 23, 2019

@mheese - any interest in taking this on?

@tstromberg tstromberg added the help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. label Jan 24, 2019
@mheese
Copy link
Contributor

mheese commented Jan 24, 2019

@tstromberg yes, you can assign me

@gbraad
Copy link
Contributor

gbraad commented Jan 24, 2019 via email

@afbjorklund
Copy link
Collaborator

Probably least confusing would be to rename the current KVM repo back to "kvm" again (since that is the name of the driver binary), and then use the "libvirt" name for a new designation for the KVM2 driver ?
Not that it matters much, compared to providing it... But considering that nothing in the driver has changed in the machine-drivers repo and that the upstream repo still remains (although unmaintained), I mean.

@mheese
Copy link
Contributor

mheese commented Jan 25, 2019

@afbjorklund Yeah, I like that idea ... However, I would maybe even like to go a step further and remove the kvm driver completely. They are both doing the same thing and they are both using libvirt, it's just that kvm2 covers a bit more - and at the end of the day, it's based on the kvm driver anyway. The current state is confusing people (even me) too much.

With regards to naming in general: I'd like to use the name libvirt instead of kvm because it is a more accurate description of what it uses. At KubeCon in December in Seattle, I learned from a minikube user that this driver did not work for him because he needed to install libvirt - which apparently was not obvious to him. He was though however using kvm, but not together with libvirt. I've personally never see anybody do this, but obviously it seems to happen. If we keep the kvm folder and/or driver around, we should at least make a note that the libvirt driver is recommended and should be used. After all people are more familiar with kvm than with libvirt.

@gbraad there might be something that is needed from machine-drivers that has to do with passing on configuration (or something like that) while initializing the driver. There is currently a reference to other minikube code, however, if I remember correctly from the top of my head that part can be removed (it's been a couple of months since I looked at this the last time). I'll let you know when I run into an impasse, and will just go through the official github issue way and will reference your github user account.

@afbjorklund
Copy link
Collaborator

@mheese : I also prefer the name "libvirt", which was why we renamed the repository when it was forked... But since the code never changed and nothing ever merged, right now it is not really adding anything ? We couldn't use libvirt because it requires root (libvirt group, BZ1284447), so that was why the qemu driver was used - still using kvm, where available. Unfortunately that meant user networking, a pain with k8s.

So when support for localkube was dropped, it was also time to drop support for qemu-kvm (#2428). Might as well focus on kubeadm and the libvirt driver (same as e.g. Boxes), even if the "requirements" are higher. I'm thinking now that the best would be to leave the old "machine-driver-kvm" and "machine" as they were (since there are no commits anyway), and focus on making a better minikube experience (like #2982)

@gbraad
Copy link
Contributor

gbraad commented Jan 26, 2019 via email

@tstromberg tstromberg added r/2019q2 Issue was last reviewed 2019q2 priority/backlog Higher priority than priority/awaiting-more-evidence. and removed priority/important-longterm Important over the long term, but may not be staffed and/or may need multiple releases to complete. labels Apr 4, 2019
@josedonizetti
Copy link
Member

Is this something we still want to do? If so, I can definitely start looking into it. Checking what are the differences, and work on migrating it to machine-drivers/docker-machine-driver-libvirt

@afbjorklund
Copy link
Collaborator

afbjorklund commented May 31, 2019

@josedonizetti
The "machine-drivers/docker-machine-driver-libvirt" development never went anywhere, so you can (for the discussion above) just consider it a fork of "dhiltgen/docker-machine-kvm" (which hosts the binaries):

https://github.com/dhiltgen/docker-machine-kvm Latest commit f328c4b on 17 Apr 2017
https://github.com/machine-drivers/docker-machine-driver-libvirt Latest commit f328c4b on 17 Apr 2017

So I guess an alternative phrasing of this issue would be "Move KVM driver upstream", similar to #3939 for hyperkit. As in: removing minikube-specific issues from the driver, and have it replace kvm(1) driver ?

This probably also means that the driver binary should work for both minikube (libmachine) and docker-machine, we are currently using API version v0.16.1: https://github.com/docker/machine/releases

@afbjorklund afbjorklund changed the title Switch kvm2 driver to machine-drivers/docker-machine-driver-libvirt Move kvm2 driver upstream (was: machine-drivers/docker-machine-driver-libvirt) Jul 6, 2019
@afbjorklund
Copy link
Collaborator

I renamed https://github.com/machine-drivers/docker-machine-driver-libvirt, since it was misleading.
The repository never became more than a fork of https://github.com/dhiltgen/docker-machine-kvm ...

We could still move the "kvm2" driver to machine-drivers organization, if it works with docker-machine ?
But I'm not sure anyone has tried it with boot2docker.iso/anything other than minikube.iso, for a long time.

@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 Oct 4, 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 Nov 3, 2019
@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
area/code-deps Code dependencies (guest-vm deps belong in guest-vm) co/kvm2-driver KVM2 driver related issues help wanted Denotes an issue that needs help from a contributor. Must meet "help wanted" guidelines. kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. 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. r/2019q2 Issue was last reviewed 2019q2
Projects
None yet
Development

No branches or pull requests

7 participants