Skip to content

hyperkit driver should be happy with current minimum verison #9365

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

Merged
merged 27 commits into from
Oct 21, 2020
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
2423f81
Introduce minHyperkitVersion constant to keep the minimum compatible …
ilya-zuyev Sep 30, 2020
d124f29
Update comments
ilya-zuyev Sep 30, 2020
8ba7478
Add copyrights
ilya-zuyev Sep 30, 2020
334779c
Change required min hyperkit version to 1.11.0
ilya-zuyev Sep 30, 2020
11b8534
Fix unit tests error messages
ilya-zuyev Sep 30, 2020
c82e327
Do not panic if min version constant is invalid
ilya-zuyev Oct 1, 2020
6ca9402
Extract a function to download driver file, but don't change ownershi…
ilya-zuyev Oct 6, 2020
2641e3d
Add hyperkit driver v1.11.0 to testdata
ilya-zuyev Oct 6, 2020
7322fdd
Add an integration test for hyperkit driver upgrades
ilya-zuyev Oct 6, 2020
08da827
Rename minDriverVersion => minAcceptableDriverVersion. Add comments
ilya-zuyev Oct 9, 2020
9b70ec8
Update integration test:
ilya-zuyev Oct 13, 2020
786037e
Fix error message in int test
ilya-zuyev Oct 13, 2020
1381394
Fix comments
ilya-zuyev Oct 13, 2020
0a9cbbb
Use "--interactive=false" to avoid asking for sudo password
ilya-zuyev Oct 14, 2020
bfca3a6
Reorganize code - move tests related to hyperkit to driver_install_or…
ilya-zuyev Oct 15, 2020
489b59b
Integration tests: fix the default test driver perms
ilya-zuyev Oct 15, 2020
eef3fd8
Integration tests: add comments to internal functions
ilya-zuyev Oct 15, 2020
4eb6f9a
Integration tests: fix naming
ilya-zuyev Oct 15, 2020
3941018
Integration tests: update README in testsdata
ilya-zuyev Oct 15, 2020
2b32f6f
Integration tests: add copyright header
ilya-zuyev Oct 15, 2020
63ca225
Integration tests: fix driver permission
ilya-zuyev Oct 15, 2020
4c5ac5a
Integration tests: remove driver_install_or_update_darwin_test.go
ilya-zuyev Oct 15, 2020
a072e89
Merge branch 'master' into gh_7422-reuse_hyperkit_driver
ilya-zuyev Oct 15, 2020
8db8f55
replace glog -> klog
ilya-zuyev Oct 15, 2020
57a0e40
Reuse $MINIKUBE_HOME/cache files;
ilya-zuyev Oct 15, 2020
5fc73c7
Add a comment
ilya-zuyev Oct 15, 2020
6c65c5d
Log driver version when validating driver
ilya-zuyev Oct 21, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ go 1.13

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
Expand Down
1 change: 1 addition & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -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=
Expand Down
8 changes: 4 additions & 4 deletions pkg/minikube/driver/install.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,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)
Expand Down Expand Up @@ -133,6 +132,7 @@ func validateDriver(executable string, v semver.Version) (string, error) {
if err != nil {
return path, errors.Wrap(err, "can't parse driver version")
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Now that we allow version mismatches, could you please add a log sttatement declaring what driver version we are using? This will help for future debugging;

klog.Infof("%s version is %s", path, driverVersion)

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Done.

if driverVersion.LT(v) {
return path, fmt.Errorf("%s is version %s, want %s", executable, driverVersion, v)
}
Expand Down
52 changes: 52 additions & 0 deletions pkg/minikube/driver/version.go
Original file line number Diff line number Diff line change
@@ -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
}
}
52 changes: 52 additions & 0 deletions pkg/minikube/driver/version_test.go
Original file line number Diff line number Diff line change
@@ -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
}
127 changes: 125 additions & 2 deletions test/integration/driver_install_or_update_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand All @@ -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
}
Original file line number Diff line number Diff line change
@@ -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.
Binary file not shown.