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

Add vmware driver which supports Fusion and Workstation #2118

Closed
wants to merge 2 commits into from

Conversation

anfernee
Copy link
Member

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

@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Oct 25, 2017
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@anfernee
Copy link
Member Author

This is originally contributed to docker/machine#4280 but they don't accept new features anymore.

@codecov-io
Copy link

codecov-io commented Oct 25, 2017

Codecov Report

Merging #2118 into master will decrease coverage by 1.32%.
The diff coverage is 15.01%.

Impacted file tree graph

@@            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
Impacted Files Coverage Δ
pkg/minikube/machine/client.go 43.22% <0%> (-2.05%) ⬇️
pkg/minikube/machine/client_linux.go 23.52% <0%> (-3.14%) ⬇️
pkg/drivers/vmware/vmrun.go 10.52% <10.52%> (ø)
pkg/drivers/vmware/driver.go 14.93% <14.93%> (ø)
pkg/minikube/cluster/cluster.go 29.71% <23.52%> (-1.35%) ⬇️
pkg/drivers/vmware/vmware_linux.go 30.76% <30.76%> (ø)
pkg/util/kubeconfig/config.go 47.61% <0%> (-0.69%) ⬇️
pkg/minikube/service/service.go 28.95% <0%> (-0.14%) ⬇️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bc20281...289ba72. Read the comment docs.

@anfernee
Copy link
Member Author

anfernee commented Nov 1, 2017

cc @r2d4 do you have concerns? if everything is fine, I will fix the coverage.

@dlorenc
Copy link
Contributor

dlorenc commented Nov 2, 2017

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.

@anfernee
Copy link
Member Author

anfernee commented Nov 3, 2017

@dlorenc thanks for the reply. this is more like an improvement to vmwarefusion so that it's not just supporting fusion but workstation on linux/windows as well. I see plenty of people still rely on fusion, and we should give them an option to keep using it. If you don't have resource maintaining that driver, I can definitely help on integration test to make sure it's in good shape. I am also going to comment on the other issue.

@gbraad
Copy link
Contributor

gbraad commented Nov 5, 2017

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.

vmrun("-gu", B2DUser, "-gp", B2DPass, "enableSharedFolders", d.vmxPath())

shareName := "Users"
shareDir := "/Users"
Copy link
Contributor

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

case "darwin":
shareName = "Users"
shareDir = "/Users"
// TODO "linux" and "windows"
Copy link
Contributor

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.

Copy link
Member Author

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is fixed.


func vmrun(args ...string) (string, string, error) {
cmd := exec.Command(vmrunbin, args...)
if os.Getenv("MACHINE_DEBUG") != "" {
Copy link
Contributor

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 { } ?

return path
}
for _, fp := range []string{
`C:\Program Files (x86)\VMware\VMware Workstation`,
Copy link
Contributor

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

Copy link
Member Author

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.

@anfernee
Copy link
Member Author

anfernee commented Nov 9, 2017

@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.

@anfernee anfernee changed the title Add vmware driver which supports Fusion and Workstation WIP: Add vmware driver which supports Fusion and Workstation Nov 9, 2017
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 9, 2017
@anfernee anfernee force-pushed the vmware branch 2 times, most recently from 1cec989 to 7b31b60 Compare November 14, 2017 00:35
@anfernee
Copy link
Member Author

Linux e2e result:

out/e2e-linux-amd64 '-minikube-start-args=--vm-driver=vmware --kubernetes-version=v1.8.0' '-minikube-args=--v=10 --logtostderr --bootstrapper=localkube' -test.v -test.timeout=30m -binary=out/minikube-linux-amd64
=== RUN   TestDocker
Environment=DOCKER_RAMDISK=yes FOO=BAR BAZ=BAT

ExecStart={ path=/usr/bin/dockerd ; argv[]=/usr/bin/dockerd -H tcp://0.0.0.0:2376 -H unix:///var/run/docker.sock --tlsverify --tlscacert /etc/docker/ca.pem --tlscert /etc/docker/server.pem --tlskey /etc/docker/server-key.pem --label provider=vmware --insecure-registry 10.0.0.0/24 --debug --icc=true ; ignore_errors=no ; start_time=[n/a] ; stop_time=[n/a] ; pid=0 ; code=(null) ; status=0/0 }

--- PASS: TestDocker (51.43s)
=== RUN   TestFunctional
=== RUN   TestFunctional/Status
=== RUN   TestFunctional/DNS
=== RUN   TestFunctional/Logs
=== RUN   TestFunctional/Addons
=== RUN   TestFunctional/Dashboard
=== RUN   TestFunctional/ServicesList
=== RUN   TestFunctional/Provisioning
=== RUN   TestFunctional/EnvVars
=== RUN   TestFunctional/SSH
--- PASS: TestFunctional (2.17s)
    --- PASS: TestFunctional/Status (0.15s)
    	cluster_status_test.go:35: Checking if cluster is healthy.
    	cluster_status_test.go:45: Component: , Healthy: True.
    	cluster_status_test.go:45: Component: , Healthy: True.
    	cluster_status_test.go:45: Component: , Healthy: True.
    --- PASS: TestFunctional/ServicesList (0.50s)
    --- PASS: TestFunctional/EnvVars (0.55s)
    --- PASS: TestFunctional/SSH (0.86s)
    --- PASS: TestFunctional/Logs (2.50s)
    --- PASS: TestFunctional/Addons (12.51s)
    --- PASS: TestFunctional/Provisioning (15.91s)
    	util.go:272: Error: No default StorageClass yet., Retrying in 5s. 20 Retries remaining.
    	util.go:272: Error: No default StorageClass yet., Retrying in 5s. 19 Retries remaining.
    	util.go:272: Error: No default StorageClass yet., Retrying in 5s. 18 Retries remaining.
    --- PASS: TestFunctional/Dashboard (28.98s)
    --- PASS: TestFunctional/DNS (34.08s)
=== RUN   TestPersistence
--- PASS: TestPersistence (346.79s)
=== RUN   TestStartStop
--- PASS: TestStartStop (377.69s)
PASS

@anfernee anfernee force-pushed the vmware branch 3 times, most recently from f0829ee to 446d134 Compare November 18, 2017 01:03
@SvenDowideit
Copy link

SvenDowideit commented Nov 18, 2017

@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

@anfernee
Copy link
Member Author

Mac e2e test passed:

$ time out/e2e-darwin-amd64 '-minikube-start-args=--vm-driver=vmware --kubernetes-version=file:///Users/anfernee/Code/gopath1.6/src/k8s.io/minikube/out/localkube' '-minikube-args=--v=10 --logtostderr --bootstrapper=localkube' -test.v -test.timeout=15m -binary=out/minikube-darwin-amd64
=== RUN   TestDocker
Environment=DOCKER_RAMDISK=yes FOO=BAR BAZ=BAT

ExecStart={ path=/usr/bin/dockerd ; argv[]=/usr/bin/dockerd -H tcp://0.0.0.0:2376 -H unix:///var/run/docker.sock --tlsverify --tlscacert /etc/docker/ca.pem --tlscert /etc/docker/server.pem --tlskey /etc/docker/server-key.pem --label provider=vmware --insecure-registry 10.96.0.0/12 --debug --icc=true ; ignore_errors=no ; start_time=[n/a] ; stop_time=[n/a] ; pid=0 ; code=(null) ; status=0/0 }

--- PASS: TestDocker (50.23s)
=== RUN   TestFunctional
=== RUN   TestFunctional/Status
=== RUN   TestFunctional/DNS
=== RUN   TestFunctional/Logs
=== RUN   TestFunctional/Addons
=== RUN   TestFunctional/Dashboard
=== RUN   TestFunctional/ServicesList
=== RUN   TestFunctional/Provisioning
=== RUN   TestFunctional/EnvVars
=== RUN   TestFunctional/SSH
--- PASS: TestFunctional (2.00s)
    --- PASS: TestFunctional/Status (0.40s)
    	cluster_status_test.go:35: Checking if cluster is healthy.
    	cluster_status_test.go:45: Component: , Healthy: True.
    	cluster_status_test.go:45: Component: , Healthy: True.
    	cluster_status_test.go:45: Component: , Healthy: True.
    --- PASS: TestFunctional/ServicesList (0.50s)
    --- PASS: TestFunctional/EnvVars (0.65s)
    --- PASS: TestFunctional/SSH (0.70s)
    --- PASS: TestFunctional/Logs (2.17s)
    --- PASS: TestFunctional/Addons (5.04s)
    --- PASS: TestFunctional/Provisioning (11.29s)
    	util.go:272: Error: No default StorageClass yet., Retrying in 5s. 20 Retries remaining.
    	util.go:272: Error: No default StorageClass yet., Retrying in 5s. 19 Retries remaining.
    --- PASS: TestFunctional/Dashboard (22.14s)
    --- PASS: TestFunctional/DNS (23.63s)
=== RUN   TestPersistence
--- PASS: TestPersistence (70.37s)
=== RUN   TestStartStop
--- PASS: TestStartStop (108.49s)
PASS

real	4m14.775s
user	0m44.241s
sys	0m7.754s

@anfernee
Copy link
Member Author

windows e2e:

C:\Users\agui\go\src\k8s.io\minikube>out\e2e-windows-amd64.exe -minikube-start-args="--vm-driver=vmware --kubernetes-version=file:///Users/agui/go/src/k8s.io/minikube/out/localkube" -minikube-args="--v=10 --logtostderr" -binary=out/minikube-windows-amd64.exe -test.v -test.timeout=15m
=== RUN   TestDocker
Environment=DOCKER_RAMDISK=yes FOO=BAR BAZ=BAT

ExecStart={ path=/usr/bin/dockerd ; argv[]=/usr/bin/dockerd -H tcp://0.0.0.0:2376 -H unix:///var/run/docker.sock --tlsverify --tlscacert /etc/docker/ca.pem --tlscert /etc/docker/server.pem --tlskey /etc/docker/server-key.pem --label provider=vmware --insecure-registry 10.96.0.0/12 --debug --icc=true ; ignore_errors=no ; start_time=[n/a] ; stop_time=[n/a] ; pid=0 ; code=(null) ; status=0/0 }

--- PASS: TestDocker (70.22s)
=== RUN   TestFunctional
=== RUN   TestFunctional/Status
=== RUN   TestFunctional/DNS
=== RUN   TestFunctional/Logs
=== RUN   TestFunctional/Addons
=== RUN   TestFunctional/Dashboard
=== RUN   TestFunctional/ServicesList
=== RUN   TestFunctional/Provisioning
=== RUN   TestFunctional/EnvVars
=== RUN   TestFunctional/SSH
--- PASS: TestFunctional (4.45s)
    --- PASS: TestFunctional/Status (0.69s)
        cluster_status_test.go:35: Checking if cluster is healthy.
        cluster_status_test.go:45: Component: , Healthy: True.
        cluster_status_test.go:45: Component: , Healthy: True.
        cluster_status_test.go:45: Component: , Healthy: True.
    --- PASS: TestFunctional/ServicesList (1.41s)
    --- PASS: TestFunctional/EnvVars (1.49s)
    --- PASS: TestFunctional/SSH (1.61s)
    --- PASS: TestFunctional/Addons (4.63s)
    --- PASS: TestFunctional/Logs (5.54s)
    --- PASS: TestFunctional/Provisioning (12.50s)
        util.go:272: Error: No default StorageClass yet., Retrying in 5s. 20 Retries remaining.
        util.go:272: Error: No default StorageClass yet., Retrying in 5s. 19 Retries remaining.
    --- PASS: TestFunctional/Dashboard (20.11s)
    --- PASS: TestFunctional/DNS (30.67s)
=== RUN   TestPersistence
--- PASS: TestPersistence (80.93s)
=== RUN   TestStartStop
--- PASS: TestStartStop (126.79s)
PASS

@gbraad
Copy link
Contributor

gbraad commented Nov 22, 2017

@anfernee jenkins slaves have to be discussed with @r2d4. At the moment I am not able to test this, so reviewing is impeded.

@anfernee anfernee changed the title WIP: Add vmware driver which supports Fusion and Workstation Add vmware driver which supports Fusion and Workstation Nov 27, 2017
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 27, 2017
@anfernee
Copy link
Member Author

anfernee commented Nov 27, 2017

Thanks @gbraad . Actually @dougm and I was trying to get necessary license for jenkins slave. @r2d4 do you have any other concerns, please don't hesitate letting us know.

@anfernee
Copy link
Member Author

I removed WIP because the integration test has passed and the scripts are ready too.

@gbraad
Copy link
Contributor

gbraad commented Nov 30, 2017

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.

@anfernee
Copy link
Member Author

anfernee commented Dec 4, 2017

@gbraad I think it makes sense. Let me take a look at how to do it, and update this pr.

Yongkun Anfernee Gui added 2 commits December 14, 2017 16:49
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.
@ellieayla
Copy link

Did the Fusion license problem get solved? Or can I help unblock that?

@anfernee
Copy link
Member Author

@alanjcastonguay I already gave licenses to @r2d4 ;)

@gbraad
Copy link
Contributor

gbraad commented Dec 20, 2017

@alanjcastonguay @anfernee We have discussed the drivers situation between Minikube and Minishift. We will go ahead with review and setting up the necessary infrastructure...

@anfernee
Copy link
Member Author

@gbraad @r2d4 hey, do you guys have updates?

@gbraad
Copy link
Contributor

gbraad commented Jan 18, 2018

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?

@SvenDowideit
Copy link

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....

@anfernee
Copy link
Member Author

@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 machine-drivers the "suggested organization" for community drivers?

@SvenDowideit
Copy link

@anfernee yes! I recon machine-drivers is the "suggested organization" for community drivers :)

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.

@gbraad
Copy link
Contributor

gbraad commented Jan 30, 2018

@anfernee The machine-drivers is the way to go... and for sure, you are open to setup CI for this according to your needs. But this is something you and @alanjcastonguay need to decide on. I do hope to see you there... Note: I'd rather refer to a community driver than a corporate driver

@anfernee
Copy link
Member Author

Close this because of #2606

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants