docker/podman: add alternative image repo for base image to fallback #7943
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: medyagh 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
medyagh
changed the title
medyagh
changed the title
|
/ok-to-test |
k8s-ci-robot
added
the
ok-to-test
medyagh
changed the title
|
kvm2 Driver Times for Minikube (PR 7943): [66.960058238 66.826738866 66.04560380000001] Averages Time Per Log docker Driver Times for Minikube (PR 7943): [27.856463181000002 27.245262438 27.033811661999994] Averages Time Per Log |
What was the problem with using the regular tag (instead of the digest) ? |
that didnt work either, acording to their docs we must not use the sha. its kind of disappointing that github wouldnt allow you to pull by SHA but thats why we first pick gcr.io then fall back to github
|
pkg/minikube/node/cache.go
Outdated
| @@ -107,7 +108,15 @@ func beginDownloadKicArtifacts(g *errgroup.Group, driver string, cRuntime string | |||
| out.T(out.Pulling, "Pulling base image ...") | |||
| g.Go(func() error { | |||
| glog.Infof("Downloading %s to local daemon", baseImage) | |||
There was a problem hiding this comment.
Please add a TODO to respect --image-repository: #7915
My question was why we are not using the tag ( That way it (kicbase) would work with all runtimes and all registries. Do we plan to have multiple images per tag, or why do we need digest ? |
using SHA is very valuable for security and integrity, I prefer we use it whenever we can. at least for our official repo. |
Codecov Report
@@ Coverage Diff @@
## master #7943 +/- ##
=======================================
Coverage 36.01% 36.01%
=======================================
Files 144 144
Lines 9207 9207
=======================================
Hits 3316 3316
Misses 5490 5490
Partials 401 401 |
medyagh
changed the title
k8s-ci-robot
added
the
do-not-merge/work-in-progress
medyagh
force-pushed
the
kicbase_gh_package
branch
from
c3a9fea
to
1ee8cab
Compare
medyagh
changed the title
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
medyagh
changed the title
medyagh
changed the title
|
kvm2 Driver Times for Minikube (PR 7943): [62.59491426299999 63.378134301 64.38491705] Averages Time Per Log docker Driver Times for Minikube (PR 7943): [26.161750855 28.219286312000005 27.155855386] Averages Time Per Log |
Problem is that the SHA is on something that we cannot verify (without the Docker registry) The image itself doesn't know which tag/digest it will end up in, and the ID is not consistent... So we end up having to patch every tool and library, for something that Docker doesn't support ? |
|
kvm2 Driver Times for Minikube (PR 7943): [62.520841604999994 63.495557346999995 64.83121233399999] Averages Time Per Log docker Driver Times for Minikube (PR 7943): [26.93760412 26.045017758 26.278521251] Averages Time Per Log |
|
kvm2 Driver Times for Minikube (PR 7943): [66.848807541 66.155819009 66.07025268699999] Averages Time Per Log docker Driver Times for Minikube (PR 7943): [26.745438058999998 26.204547349000002 26.970223288999996] Averages Time Per Log |
This PR adds an alternative image-repo in https://github.com/kubernetes/minikube/packages
for kic base image. instead of gcr.io
fixes: #7472
First: Emulating not having access to GCR:
Before this PR: time out with ugly error when image not avialble
After this PR
verifying image with docker ps
footnotes and not in the scope of this PR
1- github docker packages does NOT support pulling with SHA, which we consider it Acceptable but not ideal for our fallback image. ( see the note in this doc https://help.github.com/en/packages/using-github-packages-with-your-projects-ecosystem/configuring-docker-for-use-with-github-packages)
2- our go-containerregstiry lib is producing some errors about github needs login, that seems to be harmless error. (a spam error) but wont affect the functionality of using fallback. however that will be another PR to refactor our image.WriteToDaemon to not produce that error or handle it carefuly. so I will not consider to do that in this PR.
3- created another issue that we should detect early which image repo is best instead of failing one after time out and trying another one. that is also NOT in the scope of this PR.
#7942