Don't redownload kicbase when already loaded #15528
Conversation
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
the
approved
|
/ok-to-test |
k8s-ci-robot
added
the
ok-to-test
| @@ -197,118 +195,36 @@ func parseImage(img string) (*name.Tag, name.Reference, error) { | |||
| } | |||
|
|
|||
| // CacheToDaemon loads image from tarball in the local cache directory to the local docker daemon | |||
There was a problem hiding this comment.
add in comment what does it return, (what is the string that it returns)
|
kvm2 driver with docker runtime Times for minikube ingress: 28.7s 24.1s 27.6s 25.6s 27.6s Times for minikube start: 53.2s 52.2s 54.4s 55.2s 52.3s docker driver with docker runtime Times for minikube start: 24.9s 25.5s 24.1s 25.5s 26.0s Times for minikube ingress: 22.4s 20.9s 22.5s 21.5s 21.4s docker driver with containerd runtime Times for minikube start: 21.6s 21.5s 21.7s 22.4s 22.2s Times for minikube ingress: 26.5s 26.4s 27.4s 26.4s 25.9s |
|
These are the flake rates of all failed tests.
Too many tests failed - See test logs for more details. To see the flake rates of all tests by environment, click here. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: medyagh, spowelljr 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 |
|
lgtm, ready to be merged when you ready |
Problem:
Starting minikube when the kicbase image isn't loaded into Docker (mainly first time users or first start after an update), after loading the kicbase image from cache into Docker, the kicbase image is completely redownloaded from the internet directly to Docker. This unnecessary download causes the start time to make much longer than necessary.
Cause:
The cause is that loading the image from the minikube dir into Docker the SHA of the image is lost (expected behavior). Following the load from cache is a call to download the image from the internet, before the download starts it checks if the image already exists, but it includes the SHA in that comparison, but the image loaded from the cache doesn't have the SHA so the image exists check fails, so it runs
daemon.Writeto download the kicbase image.daemon.Writedoes not respect Docker's cache, so it completely redownloads the kicbase image (385 MB) which takes a considerable amount of time.Solution:
I learned that
daemon.Writedoesn't even include the hash, we do adocker pull <image>after it's downloaded to get the SHA. So I added the same to the end of the func that loads the image from the cache into Docker, only log a warning if it fails incase the user is offline. This fix resulted inImageToDaemonnot being needed anymore as the only value the function was adding was setting the hash viadocker pull <image>which cache load now does, so I fully removedImageToDaemon.Before:
After:
Result:
43.6s or 58% reduction in start time
Additional Fix 1:
When starting minikube while offline the following confusing message is printed:
❗ minikube was unable to download gcr.io/k8s-minikube/kicbase:v0.0.36, but successfully downloaded gcr.io/k8s-minikube/kicbase:v0.0.36 as a fallback imageThis was caused because the comparison behind the scenes was comparing the expected image which had a SHA and the image that was actually loaded that didn't have a SHA due to being unable to pull the SHA from the internet resulting in the comparison failing and outputting the message. Stripped the SHAs before doing the comparison to get the expected output of not printing the message.
Before (no internet connection and message is printed):
After (no internet connection and no message is printed)
Additional Fix 2:
When using
--download-onlythe kicbase image download would return after the first kicbase was attempted to be downloaded, regardless if the download was successful or not. Fixed the logic so if the first image fails it will retry with fallback images as expected.Before:
After: