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

Incorporate changes from KVM2 driver plugin and do a release #3

Closed
anjannath opened this issue May 11, 2018 · 8 comments
Closed

Incorporate changes from KVM2 driver plugin and do a release #3

anjannath opened this issue May 11, 2018 · 8 comments

Comments

@anjannath
Copy link

No description provided.

@afbjorklund
Copy link
Contributor

Is anybody working on this ? There are significant differences between the two machine drivers...

If moving code over to this project, there is some separate consideration about the dependencies:

import (

        "k8s.io/minikube/pkg/minikube/config"
        "k8s.io/minikube/pkg/minikube/constants"
        "k8s.io/minikube/pkg/util"

        pkgdrivers "k8s.io/minikube/pkg/drivers"
)

@mheese
Copy link

mheese commented Sep 26, 2018

and I have actually some fixes for the KVM2 driver lined up: kubernetes/minikube#3148

If I understand the further development of this correctly, they should probably also be done against this repo?

@efrecon
Copy link

efrecon commented Oct 22, 2018

@mheese any news on the PR that you are mentioning at minikube above? I also have a slight fix on the original KVM driver so that it works as it should when docker-machine uses a relative storage path. I can't provide them right now though, since github is having a hiccup today...

@efrecon
Copy link

efrecon commented Oct 22, 2018

github is slowly getting back to normal operations, you should now see my changes in this PR: #5. BTW, I agree to the above that various constants and minikube specifics needs to be removed from KVM2 when merging into KVM to be libvirt.

@mheese
Copy link

mheese commented Oct 22, 2018

@efrecon I'm planning to work on it this week, however, there is something else that I need to finish before I can start this.

Absolutely, the driver should not hold any minikube specifics! However, I'm not so sure yet what that entails. I guess I find out this week. Depending on the outcome I might first actually draft a proposal in https://github.com/machine-drivers/discussion before I do anything else.

You should get your PR merged first imho. It's a real problem that can already be addressed.

@efrecon
Copy link

efrecon commented Oct 22, 2018

Ping here in case you start in the discussion thread, I might be able to chime in as I looked into the code today. What is the process for merging PRs?

@afbjorklund
Copy link
Contributor

If not planning on doing any changes, should probably change the name back to KVM again ?

Suggest using docker-machine-driver-kvm rather than docker-machine-kvm, though...

For now, everyone is still using the binaries from the old repo:

https://github.com/dhiltgen/docker-machine-kvm/releases

@afbjorklund
Copy link
Contributor

I renamed the repo, to reflect that is just a fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants