-
Notifications
You must be signed in to change notification settings - Fork 29
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 #9
Conversation
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.
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.
Overall looks good to me, we do need to test this piece and merge it.
@@ -103,7 +107,7 @@ func (d *Driver) Create() error { | |||
|
|||
isoPath := d.ResolveStorePath(isoFilename) | |||
if err := d.extractKernel(isoPath); err != nil { | |||
return err | |||
return errors.Wrap(err, "extracting kernel") |
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.
👍
@@ -175,25 +179,32 @@ func (d *Driver) Restart() error { | |||
|
|||
// Start a host | |||
func (d *Driver) Start() error { | |||
h, err := hyperkit.New("", "", filepath.Join(d.StorePath, "machines", d.MachineName)) | |||
h, err := hyperkit.New("", d.VpnKitSock, filepath.Join(d.StorePath, "machines", d.MachineName)) |
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.
👍
Hi, With regards to testing, below are examples of ~/.minikube/machines/minikube/hyperkit.json with different EXAMPLE 1
~/.minikube/machines/minikube/hyperkit.json: EXAMPLE 2
~/.minikube/machines/minikube/hyperkit.json: |
Sorry, I have found one problem with the parsing of ports. They're defined as |
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. |
@seborama Thanks for the PR, it will be helpful for other projects (including Minishift) using hyperkit. |
Hi @LalatenduMohanty / @dlorenc / @praveenkumar , |
Yes that's correct https://github.com/machine-drivers/docker-machine-driver-hyperkit/blob/master/pkg/hyperkit/network.go#L48-L65 |
I can also see this PR is merged from minikube side so @seborama do you think we can merge it here also and then gradually built it by resolving the issues. |
Hi Praveen, Thanks for your reply. I'd say so yes. The code itself works fine as far as I can tell. The intricacies (or my understanding thereof 😛) come from how the Minikube VM's IP address is allocated by Hyperkit. Starting a new machine with vsock for the first time won't allocate an IP. The machine needs creating once then deleted and finally re-created with vsock and the uuid of the defunct machine. Awkward. I need to look into the Hyperkit code to gain a better understanding. |
Is this PR ready to be merged over on this side? I currently make heavy use of this and and building the driver from source, but it would be nice to see these changes merged. |
It is for this iteration. |
Hi @praveenkumar, Is there anything outstanding on this PR? Cheers |
Hey @nanzhong , Currently the code from this PR exists both in the minikube repo (already merged by @dlorenc ) and this repo (this PR, pending). This means that you can compile the drivers from the Minikube repo (use HOWEVER... As documented at https://github.com/kubernetes/minikube/blob/master/docs/drivers.md#hyperkit-driver, I've had a look at the binary distribution at https://storage.googleapis.com/minikube/releases/latest/docker-machine-driver-hyperkit. The binary has been updated from the minikube repo. So you don't need to compile it yourself if you don't want to, just follow the steps in the doc from the minikube project (above link). Best of luck. PS: details of the driver update as reported at https://storage.googleapis.com/minikube/: <Contents>
<Key>releases/latest/docker-machine-driver-hyperkit</Key>
<Generation>1532362270225121</Generation>
<MetaGeneration>1</MetaGeneration>
<LastModified>2018-07-23T16:11:10.224Z</LastModified>
<ETag>"6c90be83e9882c5bd838e2bb8461dc59"</ETag>
<Size>26811748</Size>
</Contents> |
A perfect, thanks! |
Hi, |
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).