-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Add vmware driver which supports Fusion and Workstation #2118
Conversation
Can one of the admins verify this patch? |
This is originally contributed to docker/machine#4280 but they don't accept new features anymore. |
Codecov Report
@@ Coverage Diff @@
## master #2118 +/- ##
==========================================
- Coverage 27.75% 26.43% -1.33%
==========================================
Files 82 85 +3
Lines 5538 6148 +610
==========================================
+ Hits 1537 1625 +88
- Misses 3806 4324 +518
- Partials 195 199 +4
Continue to review full report at Codecov.
|
cc @r2d4 do you have concerns? if everything is fine, I will fix the coverage. |
Hey, Sorry for the delay. Unforunately I don't think we're interested in supporting any additional drivers in minikube at this time either. See #2058 for some more context. |
@dlorenc thanks for the reply. this is more like an improvement to |
We have the same issue at minishift, and we will be dropping support of VMware. Adding the driver is not enough to have a reliable setup for users. CI and integration tests would need to run and debug would need to be possible on the engineering side. At the moment, we have none of this. Unless this situation can be alleviated, we might also consider deprecating the existing support. We rather reduce the number of drivers to lighten the support effort. |
pkg/drivers/vmware/driver.go
Outdated
vmrun("-gu", B2DUser, "-gp", B2DPass, "enableSharedFolders", d.vmxPath()) | ||
|
||
shareName := "Users" | ||
shareDir := "/Users" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is still macOS-specific
pkg/drivers/vmware/driver.go
Outdated
case "darwin": | ||
shareName = "Users" | ||
shareDir = "/Users" | ||
// TODO "linux" and "windows" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does not seem to provide a cross-platform solution. This will be one of the first questions we will get. How to get the shared folders working.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can follow the same pattern as virtualbox, use C:\Users
for windows, /home
for Linux.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is fixed.
pkg/drivers/vmware/vmrun.go
Outdated
|
||
func vmrun(args ...string) (string, string, error) { | ||
cmd := exec.Command(vmrunbin, args...) | ||
if os.Getenv("MACHINE_DEBUG") != "" { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is repeated over and over. Can the check be abstracted into a simple func isMachineDebugEnabled() bool { }
?
pkg/drivers/vmware/vmware_windows.go
Outdated
return path | ||
} | ||
for _, fp := range []string{ | ||
`C:\Program Files (x86)\VMware\VMware Workstation`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not guaranteed to be the case. might be possible to include the Uninstall stored data for this. eg. people might have the application installed on the D:
drive
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm. Not a Windows expert. But you are right. People might install it some other places. Maybe get it in registry, or some other places. Will address it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
@gbraad I might need your help to setup a testbed for this. I've noticed you have Jenkins slaves for different platforms. @dougm and I are trying to get some fusion/workstation licenses. then we can enable the e2e/integration test for this driver. It would make sense to merge this after test is fully setup. |
1cec989
to
7b31b60
Compare
Linux e2e result:
|
f0829ee
to
446d134
Compare
@anfernee & @dougm - I needed some licenses for fusion, workstation and vsphere, and ended up getting a 3 year subscription to https://www.vmug.com/VMUG-Join/VMUG-Advantage - which gives you access to everything... see https://www.vmug.com/Join//EVALExperience |
Mac e2e test passed:
|
windows e2e:
|
I removed |
Would it be possible to have the driver be externally provided, as in a separate binary? For instance in the same way the KVM2 driver is provided? In that way the driver could also in the future be consumed by Minishift, and it would not directly affect the codebase. |
@gbraad I think it makes sense. Let me take a look at how to do it, and update this pr. |
VMware Fusion is available on MacOS. VMware Workstation is available on both Linux and Windows. vmwarefusion is still supported, but it only works for fusion. It should be deprecated in future.
Did the Fusion license problem get solved? Or can I help unblock that? |
@alanjcastonguay I already gave licenses to @r2d4 ;) |
@alanjcastonguay @anfernee We have discussed the drivers situation between Minikube and Minishift. We will go ahead with review and setting up the necessary infrastructure... |
We might consider to move driver plugins outside the repo. At the moment we might be handed the ownership of the KVM driver (dhiltgen/docker-machine-kvm#67) Although this is not confirmed, we did discuss other drivers we have or have been proposed. In this case, the VMware driver could be one of the candidates to live in the suggested organization, which will make consumption and maintenance a shared effort. WDYT? |
That would IMO be awesome! If we could separate out the lifecycle of all machine drivers in an org, then everyone can use them and keep the ones they use up to date. I, and possibly @nathanleclaire would be likely to help out too.... |
@gbraad I think it makes sense. We used to discuss if it's possible to keep the driver inside vmware org, so we can own the testing environment, and relief the community from that hassle. @SvenDowideit Is this |
@anfernee yes! I recon TBH, i think its the last best hope we have to get not waste the great code that's out there. Hopefully, we can find ways to combine that org, with in-side vmware driver testing - I don't really know what the problems with that would be. |
@anfernee The |
Close this because of #2606 |
VMware Fusion is available on MacOS. VMware Workstation is available on both Linux and Windows. This driver will work for both Fusion and Workstation in Linux/MacOS/Windows.
vmwarefusion is still supported, but it only works for fusion. It should be deprecated in future.
Fixes #806