Merge pull request #9365 from ilya-zuyev/gh_7422-reuse_hyperkit_driver

hyperkit driver should be happy with current minimum verison

Author: medyagh

go.mod

@@ -4,6 +4,7 @@ go 1.15
 
 require (
 	cloud.google.com/go/storage v1.8.0
+	github.com/Azure/azure-sdk-for-go v42.3.0+incompatible
 	github.com/Microsoft/go-winio v0.4.15-0.20190919025122-fc70bd9a86b5 // indirect
 	github.com/Parallels/docker-machine-parallels v1.3.0
 	github.com/StackExchange/wmi v0.0.0-20190523213315-cbe66965904d // indirect

go.sum

@@ -51,6 +51,7 @@ github.com/Azure/azure-sdk-for-go v29.0.0+incompatible/go.mod h1:9XXNKU+eRnpl9mo
 github.com/Azure/azure-sdk-for-go v30.1.0+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc=
 github.com/Azure/azure-sdk-for-go v35.0.0+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc=
 github.com/Azure/azure-sdk-for-go v38.0.0+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc=
+github.com/Azure/azure-sdk-for-go v42.3.0+incompatible h1:PAHkmPqd/vQV4LJcqzEUM1elCyTMWjbrO8oFMl0dvBE=
 github.com/Azure/azure-sdk-for-go v42.3.0+incompatible/go.mod h1:9XXNKU+eRnpl9moKnB4QOLf1HestfXbmab5FXxiDBjc=
 github.com/Azure/azure-service-bus-go v0.9.1/go.mod h1:yzBx6/BUGfjfeqbRZny9AQIbIe3AcV9WZbAdpkoXOa0=
 github.com/Azure/azure-storage-blob-go v0.8.0/go.mod h1:lPI3aLPpuLTeUwh1sViKXFxwl2B6teiRqI0deQUvsw0=

pkg/minikube/driver/install.go

@@ -30,6 +30,7 @@ import (
 	"github.com/pkg/errors"
 
 	"k8s.io/klog/v2"
+
 	"k8s.io/minikube/pkg/minikube/download"
 	"k8s.io/minikube/pkg/minikube/out"
 	"k8s.io/minikube/pkg/minikube/style"
@@ -55,13 +56,12 @@ func InstallOrUpdate(name string, directory string, v semver.Version, interactiv
 	defer releaser.Release()
 
 	exists := driverExists(executable)
-	path, err := validateDriver(executable, v)
+	path, err := validateDriver(executable, minAcceptableDriverVersion(name, v))
 	if !exists || (err != nil && autoUpdate) {
 		klog.Warningf("%s: %v", executable, err)
 		path = filepath.Join(directory, executable)
-		derr := download.Driver(executable, path, v)
-		if derr != nil {
-			return derr
+		if err := download.Driver(executable, path, v); err != nil {
+			return err
 		}
 	}
 	return fixDriverPermissions(name, path, interactive)
@@ -133,6 +133,8 @@ func validateDriver(executable string, v semver.Version) (string, error) {
 	if err != nil {
 		return path, errors.Wrap(err, "can't parse driver version")
 	}
+	klog.Infof("%s version is %s", path, driverVersion)
+
 	if driverVersion.LT(v) {
 		return path, fmt.Errorf("%s is version %s, want %s", executable, driverVersion, v)
 	}

pkg/minikube/driver/version.go

@@ -0,0 +1,52 @@
+/*
+Copyright 2020 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package driver
+
+import (
+	"github.com/blang/semver"
+	"k8s.io/klog/v2"
+)
+
+// minHyperkitVersion is the minimum version of the minikube hyperkit driver compatible with the current minikube code
+var minHyperkitVersion *semver.Version
+
+const minHyperkitVersionStr = "1.11.0"
+
+func init() {
+	v, err := semver.New(minHyperkitVersionStr)
+	if err != nil {
+		klog.Errorf("Failed to parse the hyperkit driver version: %v", err)
+	} else {
+		minHyperkitVersion = v
+	}
+}
+
+// minAcceptableDriverVersion is the minimum version of driver supported by current version of minikube
+func minAcceptableDriverVersion(driver string, mkVer semver.Version) semver.Version {
+	switch driver {
+	case HyperKit:
+		if minHyperkitVersion != nil {
+			return *minHyperkitVersion
+		}
+		return mkVer
+	case KVM2:
+		return mkVer
+	default:
+		klog.Warningf("Unexpected driver: %v", driver)
+		return mkVer
+	}
+}

pkg/minikube/driver/version_test.go

@@ -0,0 +1,52 @@
+/*
+Copyright 2020 The Kubernetes Authors All rights reserved.
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package driver
+
+import (
+	"testing"
+
+	"github.com/blang/semver"
+)
+
+func Test_minDriverVersion(t *testing.T) {
+
+	tests := []struct {
+		desc   string
+		driver string
+		mkV    string
+		want   semver.Version
+	}{
+		{"Hyperkit", HyperKit, "1.1.1", *minHyperkitVersion},
+		{"Invalid", "_invalid_", "1.1.1", v("1.1.1")},
+		{"KVM2", KVM2, "1.1.1", v("1.1.1")},
+	}
+	for _, tt := range tests {
+		t.Run(tt.desc, func(t *testing.T) {
+			if got := minAcceptableDriverVersion(tt.driver, v(tt.mkV)); !got.EQ(tt.want) {
+				t.Errorf("Invalid min supported version, got: %v, want: %v", got, tt.want)
+			}
+		})
+	}
+}
+
+func v(s string) semver.Version {
+	r, err := semver.New(s)
+	if err != nil {
+		panic(err)
+	}
+	return *r
+}

test/integration/driver_install_or_update_test.go

@@ -25,9 +25,12 @@ import (
 	"runtime"
 	"testing"
 
+	"github.com/Azure/azure-sdk-for-go/tools/apidiff/ioext"
 	"github.com/blang/semver"
 
 	"k8s.io/minikube/pkg/minikube/driver"
+	"k8s.io/minikube/pkg/minikube/localpath"
+	"k8s.io/minikube/pkg/version"
 )
 
 func TestKVMDriverInstallOrUpdate(t *testing.T) {
@@ -158,8 +161,8 @@ func TestHyperKitDriverInstallOrUpdate(t *testing.T) {
 			t.Fatalf("Expected new semver. test: %v, got: %v", tc.name, err)
 		}
 
-		if err := exec.Command("sudo", "-n", "ls").Run(); err != nil {
-			t.Skipf("password required to execute 'ls', skipping remaining test: %v", err)
+		if sudoNeedsPassword() {
+			t.Skipf("password required to execute 'sudo', skipping remaining test")
 		}
 
 		err = driver.InstallOrUpdate("hyperkit", dir, newerVersion, false, true)
@@ -173,3 +176,123 @@ func TestHyperKitDriverInstallOrUpdate(t *testing.T) {
 		}
 	}
 }
+
+func TestHyperkitDriverSkipUpgrade(t *testing.T) {
+	if runtime.GOOS != "darwin" {
+		t.Skip("Skip if not darwin.")
+	}
+
+	MaybeParallel(t)
+	tests := []struct {
+		name            string
+		path            string
+		expectedVersion string
+	}{
+		{
+			name:            "upgrade-v1.11.0-to-current",
+			path:            filepath.Join(*testdataDir, "hyperkit-driver-version-1.11.0"),
+			expectedVersion: "v1.11.0",
+		},
+		{
+			name:            "upgrade-v1.2.0-to-current",
+			path:            filepath.Join(*testdataDir, "hyperkit-driver-older-version"),
+			expectedVersion: version.GetVersion(),
+		},
+	}
+
+	sudoPath, err := exec.LookPath("sudo")
+	if err != nil {
+		t.Fatalf("No sudo in path: %v", err)
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			mkDir, drvPath, err := prepareTempMinikubeDirWithHyperkitDriver(tc.name, tc.path)
+			if err != nil {
+				t.Fatalf("Failed to prepare tempdir. test: %s, got: %v", tc.name, err)
+			}
+			defer func() {
+				if err := os.RemoveAll(mkDir); err != nil {
+					t.Errorf("Failed to remove mkDir %q: %v", mkDir, err)
+				}
+			}()
+
+			cmd := exec.Command(Target(), "start", "--download-only", "--interactive=false", "--driver=hyperkit")
+			cmd.Stdout = os.Stdout
+			cmd.Stderr = os.Stdout
+			cmd.Env = append(os.Environ(),
+				fmt.Sprintf("PATH=%v%c%v", filepath.Dir(drvPath), filepath.ListSeparator, filepath.Dir(sudoPath)),
+				"MINIKUBE_HOME="+mkDir)
+			if err = cmd.Run(); err != nil {
+				t.Fatalf("failed to run minikube. got: %v", err)
+			}
+
+			upgradedVersion, err := driverVersion(drvPath)
+			if err != nil {
+				t.Fatalf("failed to check driver version. got: %v", err)
+			}
+
+			if upgradedVersion != tc.expectedVersion {
+				t.Fatalf("invalid driver version. expected: %v, got: %v", tc.expectedVersion, upgradedVersion)
+			}
+		})
+	}
+}
+
+func sudoNeedsPassword() bool {
+	err := exec.Command("sudo", "-n", "ls").Run()
+	return err != nil
+}
+
+func driverVersion(path string) (string, error) {
+	output, err := exec.Command(path, "version").Output()
+	if err != nil {
+		return "", err
+	}
+
+	var resultVersion string
+	_, err = fmt.Sscanf(string(output), "version: %s\n", &resultVersion)
+	if err != nil {
+		return "", err
+	}
+	return resultVersion, nil
+}
+
+// prepareTempMinikubeDirWithHyperkitDriver creates a temp .minikube directory
+// with structure essential to testing of hyperkit driver updates
+func prepareTempMinikubeDirWithHyperkitDriver(name, driver string) (string, string, error) {
+	temp, err := ioutil.TempDir("", name)
+	if err != nil {
+		return "", "", fmt.Errorf("failed to create tempdir: %v", err)
+	}
+	mkDir := filepath.Join(temp, ".minikube")
+	mkBinDir := filepath.Join(mkDir, "bin")
+	err = os.MkdirAll(mkBinDir, 0777)
+	if err != nil {
+		return "", "", fmt.Errorf("failed to prepare tempdir: %v", err)
+	}
+
+	pwd, err := os.Getwd()
+	if err != nil {
+		return "", "", fmt.Errorf("failed to get working directory: %v", err)
+	}
+
+	testDataDriverPath := filepath.Join(pwd, driver, "docker-machine-driver-hyperkit")
+	if _, err = os.Stat(testDataDriverPath); err != nil {
+		return "", "", fmt.Errorf("expected driver to exist: %v", err)
+	}
+	// copy driver to temp bin
+	testDriverPath := filepath.Join(mkBinDir, "docker-machine-driver-hyperkit")
+	if err = ioext.CopyFile(testDataDriverPath, testDriverPath, false); err != nil {
+		return "", "", fmt.Errorf("failed to setup current hyperkit driver: %v", err)
+	}
+
+	// try to copy cached files to the temp minikube folder to avoid downloading of iso and preloads
+	_ = ioext.CopyDir(filepath.Join(localpath.MakeMiniPath("cache")), filepath.Join(mkDir, "cache"))
+
+	// change permission to allow driver to be executable
+	if err = os.Chmod(testDriverPath, 0755); err != nil {
+		return "", "", fmt.Errorf("failed to set driver permission: %v", err)
+	}
+	return temp, testDriverPath, nil
+}

test/integration/testdata/hyperkit-driver-version-1.11.0/README

@@ -0,0 +1,2 @@
+Starting minikube version 1.14 we do not update the installed hyperkit driver if its version is 1.11.0 or higher.
+Have the hyperkit driver v1.11.0 here to test this behaviour.