hyperkit driver should be happy with current minimum verison #9365
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…version of the hyperkit driver
k8s-ci-robot
added
do-not-merge/work-in-progress
|
Hi @ilya-zuyev. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
k8s-ci-robot
added
needs-ok-to-test
|
Can one of the admins verify this patch? |
ilya-zuyev
changed the title
Travis tests have failedHey @ilya-zuyev, 1st Buildmake testTravisBuddy Request Identifier: fe54dbd0-0360-11eb-8abf-036a378110f7 |
Travis tests have failedHey @ilya-zuyev, 1st Buildmake testTravisBuddy Request Identifier: 7ce81e30-0361-11eb-8abf-036a378110f7 |
medyagh
reviewed
Sep 30, 2020
medyagh
reviewed
Sep 30, 2020
medyagh
reviewed
Sep 30, 2020
k8s-ci-robot
added
size/L
…p/permissions. This change lets run more integration tests without sudo
ilya-zuyev
changed the title
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
|
/ok-to-test |
k8s-ci-robot
added
the
ok-to-test
Travis tests have failedHey @ilya-zuyev, 1st Buildmake test2nd Buildmake testTravisBuddy Request Identifier: 81f2d920-0f18-11eb-9bff-abeacd856cb9 |
|
kvm2 Driver Times for Minikube (PR 9365): 62.2s 61.7s 57.1s Averages Time Per Log docker Driver Times for Minikube (PR 9365): 29.2s 28.3s 28.3s Averages Time Per Log |
Travis tests have failedHey @ilya-zuyev, 1st Buildmake test2nd Buildmake testTravisBuddy Request Identifier: 0b1533b0-0f19-11eb-9bff-abeacd856cb9 |
Travis tests have failedHey @ilya-zuyev, 1st Buildmake test2nd Buildmake testTravisBuddy Request Identifier: d78b1ef0-0f19-11eb-9bff-abeacd856cb9 |
Travis tests have failedHey @ilya-zuyev, 1st Buildmake test2nd Buildmake testTravisBuddy Request Identifier: 50996330-0f1d-11eb-9bff-abeacd856cb9 |
medyagh
requested changes
Oct 15, 2020
| }() | ||
|
|
||
| // start "minikube start --download-only --interactive=false --driver=hyperkit --preload=false" | ||
| cmd := exec.Command(Target(), "start", "--download-only", "--interactive=false", "--driver=hyperkit", "--preload=false") |
There was a problem hiding this comment.
when setting preload=false it will still download those things except they are not preloaded (slightly bigger tar file) but TestDownloadOnly runs before all the tests and downloads the preload tar file so this test will not redownload it
but if you set preload=false the extra images will be downloaded here:
ls ~/.minikube/cache/images/k8s.gcr.io/
| @@ -0,0 +1,221 @@ | |||
| /* | |||
| Copyright 2020 The Kubernetes Authors All rights reserved. | |||
There was a problem hiding this comment.
I still think we should use shorter filename and also youse T.Skip
this will give us visibiliity how many tests were skipped in the gopogh report
for example when running on windows, we will see a list of skipped tests so the tester would be more aware of what they are not testing on this platform.
how about driver_download_test.go ?
There was a problem hiding this comment.
Returned T.Skip checks and moved my tests to driver_install_or_update_tst.go. It's an existent has and has other tests related to driver upgrades, so probably makes sense to keep all such tests in one place
| return "", "", fmt.Errorf("failed to setup current hyperkit driver: %v", err) | ||
| } | ||
| // change permission to allow driver to be executable | ||
| if err = os.Chmod(testDriverPath, 0777); err != nil { |
|
kvm2 Driver Times for Minikube (PR 9365): 62.0s 62.5s 61.8s Averages Time Per Log docker Driver Times for Minikube (PR 9365): 28.2s 28.1s 28.4s Averages Time Per Log |
Travis tests have failedHey @ilya-zuyev, 1st Buildmake testTravisBuddy Request Identifier: fd7438e0-0f1e-11eb-9bff-abeacd856cb9 |
|
kvm2 Driver Times for Minikube (PR 9365): 60.7s 60.3s 62.2s Averages Time Per Log docker Driver Times for Minikube (PR 9365): 29.3s 28.2s 29.8s Averages Time Per Log |
medyagh
requested changes
Oct 15, 2020
| } | ||
| }() | ||
|
|
||
| // start "minikube start --download-only --interactive=false --driver=hyperkit --preload=false" |
medyagh
approved these changes
Oct 15, 2020
Cleanup comments
|
kvm2 Driver Times for Minikube (PR 9365): 61.6s 58.4s 61.4s Averages Time Per Log docker Driver Times for Minikube (PR 9365): 27.8s 27.8s 29.1s Averages Time Per Log |
|
kvm2 Driver Times for Minikube (PR 9365): 60.6s 59.8s 61.7s Averages Time Per Log docker Driver Times for Minikube (PR 9365): 29.6s 28.3s 30.9s Averages Time Per Log |
tstromberg
approved these changes
Oct 21, 2020
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: ilya-zuyev, medyagh, tstromberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
tstromberg
reviewed
Oct 21, 2020
| @@ -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") | |||
| } | |||
|
|
|||
There was a problem hiding this comment.
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!
|
kvm2 Driver Times for Minikube (PR 9365): 62.6s 62.5s 55.8s Averages Time Per Log docker Driver Times for Minikube (PR 9365): 28.9s 28.6s 28.7s Averages Time Per Log |
|
thank you |
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds an ability to keep using the installed hyperkit driver after minikube upgrade, if the drives is capable with the new version of minikube
Fixes #7422
The PR adds a new function
minDriverVersion(dirver, mkVersion)to get the lowest version of driverdrivercompatible with the current code.The minimum supported hyperkit driver version is set to
1.11.0Before:
After:
1.11.0 -> 1.13.1 The driver is NOT upgraded
1.2.0 -> 1.13.1 The driver IS upgraded
closes
#8775