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

Added hyperkit options for enterprise VPN support #2850

Merged
merged 1 commit into from
Jun 4, 2018

Conversation

seborama
Copy link
Contributor

The purpose of these changes is to enhance Hyperkit support from the
minikube command line for better integration with enterprise networks
behind a VPN.

uuid: Provide VM UUID to restore MAC address (only supported with
Hyperkit driver).
vpnkitSock: Location of the VPNKit socket used for networking. If empty,
disables Hyperkit VPNKitSock, if 'auto' uses Docker for Mac
VPNKit connection, otherwise uses the specified VSock."
vsockPorts: List of guest VSock ports that should be exposed as sockets
on the host (Only supported on with hyperkit now).

Note:
tests pass but file:
vendor/github.com/google/certificate-transparency/go/x509/root_darwin.go
has to be edited to correct an issue - not committed since this is in
the vendor directory.

@k8s-ci-robot k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 27, 2018
@k8s-ci-robot
Copy link
Contributor

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.


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. I understand the commands that are listed here.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label May 27, 2018
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@seborama
Copy link
Contributor Author

/assign @luxas

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels May 27, 2018
@praveenkumar
Copy link
Contributor

@seborama We are extracting the driver part and putting it to https://github.com/machine-drivers/docker-machine-driver-hyperkit location, can you please send PR to that also, start flags should be part of the minikube side.

@luxas luxas assigned dlorenc and unassigned luxas May 28, 2018
The purpose of these changes is to enhance Hyperkit support from the
minikube command line for better integration with enterprise networks
behind a VPN.

uuid: Provide VM UUID to restore MAC address (only supported with
      Hyperkit driver).
vpnkitSock: Location of the VPNKit socket used for networking. If empty,
            disables Hyperkit VPNKitSock, if 'auto' uses Docker for Mac
            VPNKit connection, otherwise uses the specified VSock."
vsockPorts: List of guest VSock ports that should be exposed as sockets
            on the host (Only supported on with hyperkit now).

Note:
tests pass but file:
`vendor/github.com/google/certificate-transparency/go/x509/root_darwin.go`
has to be edited to correct an issue - not committed since this is in
the vendor directory.
@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: seborama
To fully approve this pull request, please assign additional approvers.
We suggest the following additional approver: dlorenc

Assign the PR to them by writing /assign @dlorenc in a comment when ready.

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@seborama
Copy link
Contributor Author

seborama commented May 28, 2018

Hi @praveenkumar ,

I have submitted a PR over at docker-machine-driver-hyperkit.

It would be nice if Minikube was updated to remove the embedded Hyperkit code and reference the new docker-machine-driver-hyperkit repo. It would reduce the risk of code divergence.

Best regards,
Seb

@dlorenc
Copy link
Contributor

dlorenc commented May 28, 2018

Thanks for sending this! I'll take a closer look at the PR in the other repo soon.

@praveenkumar
Copy link
Contributor

@seborama Thanks for creating this in the respective repo. @dlorenc Can we start consuming the hyperkit driver from the machine-driver repo instead having it part of minikube otherwise in future folks will send PR again back here :(

Copy link
Contributor

@dlorenc dlorenc left a comment

Choose a reason for hiding this comment

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

Thanks for this! We should probably merge it in both locations until we get this copy removed.

@seborama
Copy link
Contributor Author

seborama commented Jun 1, 2018

Hi,

With regards to testing, below are examples of ~/.minikube/machines/minikube/hyperkit.json with different minikube start options.

EXAMPLE 1

./out/minikube-darwin-amd64 start --vm-driver=hyperkit --cache-images --memory=6144 --logtostderr --loglevel=0 -v=5 --uuid=8de16914-60d3-11e8-b5f4-784f438fc458 --hyperkit-vpnkit-sock=auto --hyperkit-vsock-ports="2376,8443,30000,8080" && ./out/minikube-darwin-amd64 ip

~/.minikube/machines/minikube/hyperkit.json:
Notice the values of the uuid, vsock, vsock_ports, vpnkit_sock. Also, the VM was given the same IP address as previously given for this uuid.
{"hyperkit":"/usr/local/bin/hyperkit","argv0":"","state_dir":"/Users/someone/.minikube/machines/minikube","vpnkit_sock":"/Users/someone/Library/Containers/com.docker.docker/Data/s50","vpnkit_uuid":"","vpnkit_preferred_ipv4":"","uuid":"8de16914-60d3-11e8-b5f4-784f438fc458","disks":[{"path":"/Users/someone/.minikube/machines/minikube/minikube.rawdisk","size":20000,"format":"","driver":"virtio-blk"}],"iso":["/Users/someone/.minikube/machines/minikube/boot2docker.iso"],"vsock":true,"vsock_ports":[2376,8443,30000,8080],"vsock_guest_cid":3,"vmnet":true,"9p_sockets":null,"kernel":"/Users/someone/.minikube/machines/minikube/bzImage","initrd":"/Users/someone/.minikube/machines/minikube/initrd","bootrom":"","cpus":2,"memory":6144,"console":1,"extra_files":null,"pid":6305,"arguments":["-A","-u","-F","/Users/someone/.minikube/machines/minikube/hyperkit.pid","-c","2","-m","6144M","-s","0:0,hostbridge","-s","31,lpc","-s","1:0,virtio-vpnkit,path=/Users/someone/Library/Containers/com.docker.docker/Data/s50","-s","2:0,virtio-net","-U","8de16914-60d3-11e8-b5f4-784f438fc458","-s","3:0,virtio-blk,/Users/someone/.minikube/machines/minikube/minikube.rawdisk","-s","4,virtio-sock,guest_cid=3,path=/Users/someone/.minikube/machines/minikube,guest_forwards=2376;8443;30000;8080","-s","5,ahci-cd,/Users/someone/.minikube/machines/minikube/boot2docker.iso","-s","6,virtio-rnd","-l","com1,autopty=/Users/someone/.minikube/machines/minikube/tty,log=/Users/someone/.minikube/machines/minikube/console-ring","-f","kexec,/Users/someone/.minikube/machines/minikube/bzImage,/Users/someone/.minikube/machines/minikube/initrd,earlyprintk=serial loglevel=3 user=docker console=ttyS0 console=tty0 noembed nomodeset norestore waitusb=10 systemd.legacy_systemd_cgroup_controller=yes base host=minikube"],"cmdline":"/usr/local/bin/hyperkit -A -u -F /Users/someone/.minikube/machines/minikube/hyperkit.pid -c 2 -m 6144M -s 0:0,hostbridge -s 31,lpc -s 1:0,virtio-vpnkit,path=/Users/someone/Library/Containers/com.docker.docker/Data/s50 -s 2:0,virtio-net -U 8de16914-60d3-11e8-b5f4-784f438fc458 -s 3:0,virtio-blk,/Users/someone/.minikube/machines/minikube/minikube.rawdisk -s 4,virtio-sock,guest_cid=3,path=/Users/someone/.minikube/machines/minikube,guest_forwards=2376;8443;30000;8080 -s 5,ahci-cd,/Users/someone/.minikube/machines/minikube/boot2docker.iso -s 6,virtio-rnd -l com1,autopty=/Users/someone/.minikube/machines/minikube/tty,log=/Users/someone/.minikube/machines/minikube/console-ring -f kexec,/Users/someone/.minikube/machines/minikube/bzImage,/Users/someone/.minikube/machines/minikube/initrd,earlyprintk=serial loglevel=3 user=docker console=ttyS0 console=tty0 noembed nomodeset norestore waitusb=10 systemd.legacy_systemd_cgroup_controller=yes base host=minikube"}

EXAMPLE 2

./out/minikube-darwin-amd64 start --vm-driver=hyperkit --cache-images --memory=6144 --logtostderr --loglevel=0 -v=5 && ./out/minikube-darwin-amd64 ip

~/.minikube/machines/minikube/hyperkit.json:
A new IP was provided, no more vsock activation
{"hyperkit":"/usr/local/bin/hyperkit","argv0":"","state_dir":"/Users/someone/.minikube/machines/minikube","vpnkit_sock":"","vpnkit_uuid":"","vpnkit_preferred_ipv4":"","uuid":"4b8107f3-656a-11e8-9069-784f438fc457","disks":[{"path":"/Users/someone/.minikube/machines/minikube/minikube.rawdisk","size":20000,"format":"","driver":"virtio-blk"}],"iso":["/Users/someone/.minikube/machines/minikube/boot2docker.iso"],"vsock":false,"vsock_ports":null,"vsock_guest_cid":3,"vmnet":true,"9p_sockets":null,"kernel":"/Users/someone/.minikube/machines/minikube/bzImage","initrd":"/Users/someone/.minikube/machines/minikube/initrd","bootrom":"","cpus":2,"memory":6144,"console":1,"extra_files":null,"pid":8607,"arguments":["-A","-u","-F","/Users/someone/.minikube/machines/minikube/hyperkit.pid","-c","2","-m","6144M","-s","0:0,hostbridge","-s","31,lpc","-s","1:0,virtio-net","-U","4b8107f3-656a-11e8-9069-784f438fc457","-s","2:0,virtio-blk,/Users/someone/.minikube/machines/minikube/minikube.rawdisk","-s","3,ahci-cd,/Users/someone/.minikube/machines/minikube/boot2docker.iso","-s","4,virtio-rnd","-l","com1,autopty=/Users/someone/.minikube/machines/minikube/tty,log=/Users/someone/.minikube/machines/minikube/console-ring","-f","kexec,/Users/someone/.minikube/machines/minikube/bzImage,/Users/someone/.minikube/machines/minikube/initrd,earlyprintk=serial loglevel=3 user=docker console=ttyS0 console=tty0 noembed nomodeset norestore waitusb=10 systemd.legacy_systemd_cgroup_controller=yes base host=minikube"],"cmdline":"/usr/local/bin/hyperkit -A -u -F /Users/someone/.minikube/machines/minikube/hyperkit.pid -c 2 -m 6144M -s 0:0,hostbridge -s 31,lpc -s 1:0,virtio-net -U 4b8107f3-656a-11e8-9069-784f438fc457 -s 2:0,virtio-blk,/Users/someone/.minikube/machines/minikube/minikube.rawdisk -s 3,ahci-cd,/Users/someone/.minikube/machines/minikube/boot2docker.iso -s 4,virtio-rnd -l com1,autopty=/Users/someone/.minikube/machines/minikube/tty,log=/Users/someone/.minikube/machines/minikube/console-ring -f kexec,/Users/someone/.minikube/machines/minikube/bzImage,/Users/someone/.minikube/machines/minikube/initrd,earlyprintk=serial loglevel=3 user=docker console=ttyS0 console=tty0 noembed nomodeset norestore waitusb=10 systemd.legacy_systemd_cgroup_controller=yes base host=minikube"}

@dlorenc
Copy link
Contributor

dlorenc commented Jun 1, 2018

@seborama We are extracting the driver part and putting it to https://github.com/machine-drivers/docker-machine-driver-hyperkit location, can you please send PR to that also, start flags should be part of the minikube side.

@praveenkumar That sounds good. I think it's mostly a documentation change initially. Do you have the cycles to get started on it? I can help out on the CI changes.

@dlorenc
Copy link
Contributor

dlorenc commented Jun 1, 2018

@praveenkumar any concerns about merging this change in both places to keep things in sync until we consolidate?

@dlorenc
Copy link
Contributor

dlorenc commented Jun 1, 2018

@minikube-bot OK to test

@praveenkumar
Copy link
Contributor

That sounds good. I think it's mostly a documentation change initially. Do you have the cycles to get started on it? I can help out on the CI changes.

I will try to replace that hyperkit part and use the machine-driver/hyperkit in the coming week (let's see how it goes). If we have CI in place on machine-driver/hyperkit side that would be great so the driver changes can be tested.

@praveenkumar
Copy link
Contributor

any concerns about merging this change in both places to keep things in sync until we consolidate?

Please go ahead to merge it as of now for both places.

@seborama
Copy link
Contributor Author

seborama commented Jun 2, 2018

Sorry, I have found one problem with the parsing of ports. They're defined as int rather than uint16 by Hyperkit and the conversion sometimes fails.
Investigating options...

@seborama
Copy link
Contributor Author

seborama commented Jun 2, 2018

OK, it seems the problem may have been caused by me. I have 3 versions of the driver and 2 of minikube owing to the forked repos and one from homebrew.
I've re-sync'ed both minikube and the hyperkit driver from a fresh compilation and the problem has gone away. I can only assume that different versions caused a clash.

@dlorenc dlorenc merged commit ee73cbd into kubernetes:master Jun 4, 2018
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/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants