Code review feedback.

kubernetes · Aug 24, 2017 · f6115d7 · f6115d7
1 parent de31909
commit f6115d7
Show file tree

Hide file tree

Showing 6 changed files with 22 additions and 7 deletions.
diff --git a/Makefile b/Makefile
@@ -51,6 +51,7 @@ LOCALKUBE_LDFLAGS := "$(K8S_VERSION_LDFLAGS) $(MINIKUBE_LDFLAGS) -s -w -extldfla
 
 LOCALKUBEFILES := GOPATH=$(GOPATH) go list  -f '{{join .Deps "\n"}}' ./cmd/localkube/ | grep k8s.io | GOPATH=$(GOPATH) xargs go list -f '{{ range $$file := .GoFiles }} {{$$.Dir}}/{{$$file}}{{"\n"}}{{end}}'
 MINIKUBEFILES := GOPATH=$(GOPATH) go list  -f '{{join .Deps "\n"}}' ./cmd/minikube/ | grep k8s.io | GOPATH=$(GOPATH) xargs go list -f '{{ range $$file := .GoFiles }} {{$$.Dir}}/{{$$file}}{{"\n"}}{{end}}'
+HYPERKIT_FILES :=  GOPATH=$(GOPATH) 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}}'
 
 MINIKUBE_ENV_linux  		:= CGO_ENABLED=1 GOARCH=amd64 GOOS=linux
 MINIKUBE_ENV_darwin  		:= CGO_ENABLED=1 GOARCH=amd64 GOOS=darwin
@@ -213,7 +214,7 @@ 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:
+out/docker-machine-driver-hyperkit: $(shell $(HYPERKIT_FILES))
 ifeq ($(MINIKUBE_BUILD_IN_DOCKER),y)
 	$(MINIKUBE_DOCKER_CMD) '$(MINIKUBE_ENV_darwin_DOCKER) $(MINIKUBE_ENV_darwin) go build -o $(BUILD_DIR)/docker-machine-driver-hyperkit k8s.io/minikube/cmd/drivers/hyperkit'
 else

diff --git a/hack/jenkins/common.sh b/hack/jenkins/common.sh
@@ -34,7 +34,7 @@ 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/"
+export PATH="$(pwd)/out/":$PATH
 
 # Linux cleanup
 virsh -c qemu:///system list --all \

diff --git a/pkg/minikube/drivers/hyperkit/disk.go b/pkg/minikube/drivers/hyperkit/disk.go
@@ -26,7 +26,7 @@ import (
 	"github.com/docker/machine/libmachine/mcnutils"
 )
 
-func createDiskImage(sshKeyPath, diskPath string, diskSize int) error {
+func createDiskImage(sshKeyPath, diskPath string, diskSizeMb int) error {
 	tarBuf, err := mcnutils.MakeDiskImage(sshKeyPath)
 	if err != nil {
 		return err
@@ -44,7 +44,7 @@ func createDiskImage(sshKeyPath, diskPath string, diskSize int) error {
 	}
 	file.Close()
 
-	if err := os.Truncate(diskPath, int64(diskSize*1048576)); err != nil {
+	if err := os.Truncate(diskPath, int64(diskSizeMb*1000000)); err != nil {
 		return err
 	}
 	return nil

diff --git a/pkg/minikube/drivers/hyperkit/disk_test.go b/pkg/minikube/drivers/hyperkit/disk_test.go
@@ -34,7 +34,7 @@ func Test_createDiskImage(t *testing.T) {
 	diskPath := filepath.Join(tmpdir, "disk")
 
 	sizeInMb := 100
-	sizeInBytes := int64(sizeInMb) * 1048576
+	sizeInBytes := int64(sizeInMb) * 1000000
 	if err := createDiskImage(sshPath, diskPath, sizeInMb); err != nil {
 		t.Errorf("createDiskImage() error = %v", err)
 	}

diff --git a/pkg/minikube/drivers/hyperkit/iso.go b/pkg/minikube/drivers/hyperkit/iso.go
@@ -76,11 +76,12 @@ func ReadFile(isoPath, srcPath string) (string, error) {
 }
 
 func findFile(r *iso9660.Reader, path string) (os.FileInfo, error) {
+	// Look through the ISO for a file with a matching path.
 	for f, err := r.Next(); err != io.EOF; f, err = r.Next() {
-		// Some files get an extra ',' at the end.
+		// For some reason file paths in the ISO sometimes contain a '.' character at the end, so strip that off.
 		if strings.TrimSuffix(f.Name(), ".") == path {
 			return f, nil
 		}
 	}
-	return nil, fmt.Errorf("Unable to find file %s.", path)
+	return nil, fmt.Errorf("unable to find file %s", path)
 }
diff --git a/pkg/minikube/drivers/hyperkit/network_test.go b/pkg/minikube/drivers/hyperkit/network_test.go
@@ -38,6 +38,13 @@ var validLeases = []byte(`{
 	hw_address=1,a4:b5:c6:d7:e8:f9
 	identifier=1,a0:b0:c0:d0:e0:f0
 	lease=0x597e1267
+}
+{
+	name=bar
+	ip_address=192.168.64.4
+	hw_address=1,a5:b6:c7:d8:e9:f1
+	identifier=1,a5:b6:c7:d8:e9:f1
+	lease=0x597e1268
 }`)
 
 func Test_getIpAddressFromFile(t *testing.T) {
@@ -66,6 +73,12 @@ func Test_getIpAddressFromFile(t *testing.T) {
 			"1.2.3.4",
 			false,
 		},
+		{
+			"duplicate",
+			args{"a4:b5:c6:d7:e8:f9", dhcpFile},
+			"192.168.64.3",
+			false,
+		},
 		{
 			"invalid",
 			args{"a1:b2:c3:d4:e5:f6", invalidFile},