WIP: Initial hyperkit driver implementation. #1776

dlorenc · 2017-08-01T15:51:57Z

This is an implementation of a hyperkit driver, in-tree. It still runs as a separate executable/process that needs to be installed on your path somewhere as 'docker-machine-driver-hyperkit' because it needs setuid permissions.

It uses the native hyperkit.go wrapper and vmnet for networking.

zchee · 2017-08-02T00:45:15Z

@dlorenc SGTM. Thanks for port of my code :)

zchee · 2017-08-02T00:47:01Z

@dlorenc But FYI, zchee/go-vmnet is unstable. If you don't mind, it might be better to copy all file to minikube.

dlorenc · 2017-08-02T00:48:12Z

@dlorenc But FYI, zchee/go-vmnet is unstable. If you don't mind, it might be better to copy all file to minikube.

Thanks, I'll definitely vendor it in.

dlorenc · 2017-08-15T19:39:21Z

@minikube-bot test this please

codecov-io · 2017-08-16T00:11:45Z

Codecov Report

Merging #1776 into master will increase coverage by 0.2%.
The diff coverage is 39.13%.

@@            Coverage Diff            @@
##           master    #1776     +/-   ##
=========================================
+ Coverage   32.73%   32.94%   +0.2%     
=========================================
  Files          66       69      +3     
  Lines        4112     4250    +138     
=========================================
+ Hits         1346     1400     +54     
- Misses       2571     2650     +79     
- Partials      195      200      +5

Impacted Files	Coverage Δ
pkg/minikube/cluster/cluster.go	`43.04% <0%> (-0.2%)`	⬇️
pkg/minikube/cluster/cluster_non_darwin_panic.go	`0% <0%> (ø)`	⬆️
pkg/minikube/drivers/hyperkit/iso.go	`0% <0%> (ø)`
pkg/minikube/drivers/hyperkit/disk.go	`29.03% <29.03%> (ø)`
pkg/minikube/drivers/hyperkit/network.go	`77.58% <77.58%> (ø)`

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 88fef61...a05a4fe. Read the comment docs.

dlorenc · 2017-08-21T22:10:57Z

@minikube-bot test this please

zhaytee · 2017-08-22T06:38:04Z

Eagerly anticipating this! Thanks for your work, @dlorenc and @zchee.
I'm probably jumping the gun by trying to get it running already, but: I built minikube from your hyperkit fork/branch, @dlorenc, and built (and installed) "docker-machine-driver-hyperkit". Then I tried creating a fresh minikube cluster. It seemed to be working, at first: I saw a "hyperkit" process in Activity Monitor taking up some CPU time, but then minikube timed out with an ssh error. Looks like it wasn't able to ssh into the new cluster hosted by hyperkit:

$ minikube start --vm-driver hyperkit
Starting local Kubernetes v1.7.4 cluster...
Starting VM...
E0821 23:34:31.145147   10338 start.go:132] Error starting host: Temporary Error: Error configuring auth on host: Too many retries waiting for SSH to be available.  Last error: Maximum number of retries (60) exceeded.

Should I just be patient and wait for this to make it in for real, or is there a small tweak I can make to make it work now? 😁

dlorenc · 2017-08-22T16:21:57Z

I'm probably jumping the gun by trying to get it running already, but: I built minikube from your hyperkit fork/branch, @dlorenc, and built (and installed) "docker-machine-driver-hyperkit". Then I tried creating a fresh minikube cluster. It seemed to be working, at first: I saw a "hyperkit" process in Activity Monitor taking up some CPU time, but then minikube timed out with an ssh error. Looks like it wasn't able to ssh into the new cluster hosted by hyperkit:

Could you open an issue and attach the logs of minikube start --v=10 --vm-driver=hyperkit ?

dlorenc · 2017-08-22T19:55:18Z

@r2d4 this should be ready for a review.

r2d4

Looks awesome. Just a few comments.

r2d4 · 2017-08-22T20:05:54Z

Makefile

@@ -207,6 +207,19 @@ out/minikube-installer.exe: out/minikube-windows-amd64.exe
 	mv out/windows_tmp/minikube-installer.exe out/minikube-installer.exe
 	rm -rf out/windows_tmp

+out/docker-machine-driver-hyperkit:


Maybe add a

HYPERKIT_FILES := $(shell go list -f '{{join .Deps "\n"}}' k8s.io/minikube/cmd/drivers/hyperkit | grep k8s.io | GOPATH=$(GOPATH) xargs go list -f '{{ range $$file := .GoFiles }} {{$$.Dir}}/{{$$file}}{{"\n"}}{{end}}')

and make this rule depend on that.

r2d4 · 2017-08-22T20:07:03Z

hack/jenkins/common.sh

 gsutil cp gs://minikube-builds/${MINIKUBE_LOCATION}/e2e-${OS_ARCH} out/
 gsutil cp gs://minikube-builds/${MINIKUBE_LOCATION}/testdata/busybox.yaml testdata/
 gsutil cp gs://minikube-builds/${MINIKUBE_LOCATION}/testdata/pvc.yaml testdata/
 gsutil cp gs://minikube-builds/${MINIKUBE_LOCATION}/testdata/busybox-mount-test.yaml testdata/

+# Add the out/ directory to the PATH, for using new drivers.
+export PATH=$PATH:"$(pwd)/out/"


I would reverse the order of these two, so $(pwd)/out/ always takes precedence.

export PATH="$(pwd)/out/":$PATH

r2d4 · 2017-08-22T20:18:46Z

pkg/minikube/drivers/hyperkit/disk.go

+	}
+	file.Close()
+
+	if err := os.Truncate(diskPath, int64(diskSize*1048576)); err != nil {


diskSize * 10^6 instead of diskSize * 2^20. This code will give you diskSize * MiB instead of disksize * MB

I tried this out with

package main import "os" func main() { os.Create("./test") os.Truncate("./test", 1*1048576) }

$ ls -l --block-size=KB total 5kB -rw-r----- 1 mrick eng 1kB Aug 22 13:16 main.go -rw-r----- 1 mrick eng 1049kB Aug 22 13:16 test

Good catch!

r2d4 · 2017-08-22T20:19:18Z

pkg/minikube/drivers/hyperkit/disk.go

+	"github.com/docker/machine/libmachine/mcnutils"
+)
+
+func createDiskImage(sshKeyPath, diskPath string, diskSize int) error {


add a comment that says diskSize is in MB.

r2d4 · 2017-08-22T20:23:24Z

pkg/minikube/drivers/hyperkit/disk_test.go

+	diskPath := filepath.Join(tmpdir, "disk")
+
+	sizeInMb := 100
+	sizeInBytes := int64(sizeInMb) * 1048576


r2d4 · 2017-08-22T20:26:24Z

pkg/minikube/drivers/hyperkit/iso.go

+
+func findFile(r *iso9660.Reader, path string) (os.FileInfo, error) {
+	for f, err := r.Next(); err != io.EOF; f, err = r.Next() {
+		// Some files get an extra ',' at the end.


Can you explain this function a little more?

r2d4 · 2017-08-22T20:32:14Z

pkg/minikube/drivers/hyperkit/network_test.go

+	lease=0x597e1267
+}`)
+
+func Test_getIpAddressFromFile(t *testing.T) {


You might want to add a test with multiple entries with the same name but different MACs.

Added a third entry. The code looks by MAC, so I can't really add a test for that case since name isn't part of the API at all.

r2d4 · 2017-08-23T23:35:51Z

@minikube-bot retest this please

r2d4

LGTM. Excited to use this.

r2d4 · 2017-08-23T23:41:24Z

This looks fine - since its working properly, but worth it to note in the build logs:

# k8s.io/minikube/vendor/github.com/zchee/go-vmnet
ld: warning: object file (/tmp/go-build684517637/k8s.io/minikube/vendor/github.com/zchee/go-vmnet/_obj/_cgo_export.o) was built for newer OSX version (10.10) than being linked (10.6)
ld: warning: object file (/tmp/go-build684517637/k8s.io/minikube/vendor/github.com/zchee/go-vmnet/_obj/vmnet.cgo2.o) was built for newer OSX version (10.10) than being linked (10.6)
ld: warning: object file (/tmp/go-build684517637/k8s.io/minikube/vendor/github.com/zchee/go-vmnet/_obj/_cgo_main.o) was built for newer OSX version (10.10) than being linked (10.6)
# k8s.io/minikube/vendor/github.com/zchee/go-vmnet
ld: warning: object file (/tmp/go-build684517637/k8s.io/minikube/vendor/github.com/zchee/go-vmnet/_obj/_cgo_export.o) was built for newer OSX version (10.10) than being linked (10.6)
ld: warning: object file (/tmp/go-build684517637/k8s.io/minikube/vendor/github.com/zchee/go-vmnet/_obj/vmnet.cgo2.o) was built for newer OSX version (10.10) than being linked (10.6)

r2d4 · 2017-08-23T23:53:19Z

hack/jenkins/common.sh

 gsutil cp gs://minikube-builds/${MINIKUBE_LOCATION}/e2e-${OS_ARCH} out/
 gsutil cp gs://minikube-builds/${MINIKUBE_LOCATION}/testdata/busybox.yaml testdata/
 gsutil cp gs://minikube-builds/${MINIKUBE_LOCATION}/testdata/pvc.yaml testdata/
 gsutil cp gs://minikube-builds/${MINIKUBE_LOCATION}/testdata/busybox-mount-test.yaml testdata/

+# Add the out/ directory to the PATH, for using new drivers.
+export "$(pwd)/out/":PATH=$PATH


export PATH="$(pwd)/out/":$PATH

++ export /home/jenkins/workspace/Linux_Integration_Tests_KVM/out/:PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/usr/local/go/bin:/home/jenkins/go/bin:/usr/local/bin:/usr/local/go/bin:/home/jenkins/google-cloud-sdk/bin:/usr/local/bin:/usr/local/go/bin:/home/jenkins/google-cloud-sdk/bin common.sh: line 37: export: `/home/jenkins/workspace/Linux_Integration_Tests_KVM/out/:PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/usr/local/go/bin:/home/jenkins/go/bin:/usr/local/bin:/usr/local/go/bin:/home/jenkins/google-cloud-sdk/bin:/usr/local/bin:/usr/local/go/bin:/home/jenkins/google-cloud-sdk/bin': not a valid identifier Build step 'Execute shell' marked build as failure

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Aug 1, 2017

dlorenc force-pushed the hyperkit branch from fe7f8f5 to b77b58a Compare August 1, 2017 15:59

dlorenc force-pushed the hyperkit branch 12 times, most recently from 2ac1dd6 to f197143 Compare August 15, 2017 17:07

dlorenc force-pushed the hyperkit branch 2 times, most recently from e1d5207 to e60bf66 Compare August 15, 2017 23:46

dlorenc force-pushed the hyperkit branch from be79795 to 384bb3c Compare August 21, 2017 15:33

dlorenc force-pushed the hyperkit branch from 5cd8f4d to 0c9f050 Compare August 22, 2017 16:21

r2d4 reviewed Aug 22, 2017

View reviewed changes

dlorenc force-pushed the hyperkit branch from 0c9f050 to aac6eb7 Compare August 23, 2017 16:43

r2d4 approved these changes Aug 23, 2017

View reviewed changes

r2d4 suggested changes Aug 23, 2017

View reviewed changes

dlorenc force-pushed the hyperkit branch from aac6eb7 to f6115d7 Compare August 24, 2017 01:07

dlorenc and others added 5 commits August 24, 2017 11:54

Initial hyperkit driver implementation.

28f9d5d

Jenkins/Makefile targets.

e3d2399

Unit tests.

b4202d6

Add darwin build tag.

1ac4166

Code review feedback.

a05a4fe

dlorenc force-pushed the hyperkit branch from f6115d7 to a05a4fe Compare August 24, 2017 18:55

r2d4 approved these changes Aug 24, 2017

View reviewed changes

r2d4 merged commit caa8a54 into kubernetes:master Aug 24, 2017

tstromberg mentioned this pull request May 22, 2019

hyperv dashboard: [SSH_TCP_FAILURE] Error dialing tcp via ssh client #4320

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Initial hyperkit driver implementation. #1776

WIP: Initial hyperkit driver implementation. #1776

dlorenc commented Aug 1, 2017

zchee commented Aug 2, 2017

zchee commented Aug 2, 2017

dlorenc commented Aug 2, 2017

dlorenc commented Aug 15, 2017

codecov-io commented Aug 16, 2017 •

edited

Loading

dlorenc commented Aug 21, 2017

zhaytee commented Aug 22, 2017

dlorenc commented Aug 22, 2017

dlorenc commented Aug 22, 2017

r2d4 left a comment

r2d4 Aug 22, 2017

dlorenc Aug 23, 2017

r2d4 Aug 22, 2017

dlorenc Aug 23, 2017

r2d4 Aug 22, 2017

dlorenc Aug 23, 2017

r2d4 Aug 22, 2017

dlorenc Aug 23, 2017

r2d4 Aug 22, 2017

r2d4 Aug 22, 2017

dlorenc Aug 23, 2017

r2d4 Aug 22, 2017

dlorenc Aug 23, 2017

r2d4 commented Aug 23, 2017

r2d4 left a comment

r2d4 commented Aug 23, 2017

r2d4 Aug 23, 2017

r2d4 Aug 23, 2017

dlorenc Aug 24, 2017

WIP: Initial hyperkit driver implementation. #1776

WIP: Initial hyperkit driver implementation. #1776

Conversation

dlorenc commented Aug 1, 2017

zchee commented Aug 2, 2017

zchee commented Aug 2, 2017

dlorenc commented Aug 2, 2017

dlorenc commented Aug 15, 2017

codecov-io commented Aug 16, 2017 • edited Loading

Codecov Report

dlorenc commented Aug 21, 2017

zhaytee commented Aug 22, 2017

dlorenc commented Aug 22, 2017

dlorenc commented Aug 22, 2017

r2d4 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

r2d4 commented Aug 23, 2017

r2d4 left a comment

Choose a reason for hiding this comment

r2d4 commented Aug 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Aug 16, 2017 •

edited

Loading