docker-agnostic kicBase cache interactions #15696
Conversation
k8s-ci-robot
added
cncf-cla: yes
|
Hi @x7upLime. 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
the
size/L
|
Can one of the admins verify this patch? |
k8s-ci-robot
added
size/XXL
x7upLime
force-pushed
the
docker-agnostic-KicBase-cache
branch
from
4b57449 to
754f676
Compare
k8s-ci-robot
added
size/L
Being pkg/drivers/kic/types.go the source of truth for the version of the container we're using to instantiate our kübernetes cluster in, the pr should start here.. Initially I thought about hardcoding the contentDigest(a.k.a. imageId) here as well, to then use it to check against the images inside the kicDriver.. It later took another turn(we're retrieving it from tar). Plus a collaborator showed me that it was a bad idea.. maintaining it here would bean bumping it as part of the image build process. The idea is based on the following concepts: .contentDigest is the most reliable way to address image content: if the image is tampered with after push to a registry, the contentDigest we'd see after pull, would be different than the one hardcoded here. It is also part of the image itself, i.e. part of the tar archive; thus giving us a way to always know if the cache is up to date, even offline. .distributionDigest is the most reliable way to determine which image we're looking to pull from a registry; a tag can be detached from an image and recycled, referencing another one, with different content. It is not part of the image itself; it is computed on the image in compressed state.. and since different engines/mechanisms could use different types of compression, this digest is totally unreliable as a way to address content. [*] refs: https://windsock.io/explaining-docker-image-ids/ google/go-containerregistry#895 (comment) https://stackoverflow.com/questions/45533005/why-digests-are-different-depend-on-registry https://blog.aquasec.com/docker-image-tags -- follow links
Ther are two versions of imagePathInCache: one in pkg/minikube/download one in pkg/minikube/image They refer to two different caches.. I thought that we could use the one referring to the kicBase cache, instead of rewriting the same thing inside pkg/minikube/node
The idea is that the distributionDigest was not meant to be used as something content-addressable. So instead of invalidating the kicBase cache based on the distributionDigest present in the kicDriver's storage, we could move to something like this: .User expresses preferences in regard of kicBase image, by specifying the image by name:tag@digest, name:tag,... whatever .The image's specified name is sanitized and used as filename for the tarbal in minikube's cache[*] .Image stored inside kicDriver(docker, podman,...) is validated against the image's contentDigest(imageID), which we're retrieving directly from the .tar archive.. which we're selecting from the cache, by sanitized name I initially tried implementing a mechanism based on a json file that would contain entries in regard of cached images, plus other infos that we would need to address content, like distributionDigest(registry digest)... Something like a repositories.json file.. in a docker fashion (/var/lib/docker/image/overlay2/repositories.json) That would lead to complications like "what if user removes a .tar to save space?", (basically.. how we keep the file up-to-date with cache content) that would add extra complexity in solving.. like some logic that would be called on startup, that would read all cached files and reflect in the file. As far as I can understand our usecase, which doesn't seem very complex(I might be wrong..), some kind of simple/raw mechanism like the one in this commit could suffice.. Like we don't have to maintain a lot of images.. The source of truth is hardcoded in the sources.. the image/file relation for the kicBase cache is based on the sanitize(img) mechanism.. ... [+] refs.. https://windsock.io/explaining-docker-image-ids/ https://blog.aquasec.com/docker-image-tags https://stackoverflow.com/questions/45533005/why-digests-are-different-depend-on-registry [*] TODO: this could be one possible shortcoming.. - what if user wants the image:tag to be always up-to-date with registry? perhaps some flag?
Abilitates the low-level pkg/drivers/kic/oci to load container images from tarballs, and adds the glue to that to the pkg/minikube/download so that it could be used inside pkg/minikube/node
Since we're now able to address content inside the kicDriver based on contentDigest(a.k.a. imageID, a.k.a. ID), being it a more containerEngine agnostic mechanism.. we can effectively load cached images to a more generic kicDriver entity, rather than worrying about how each container engine treats the distributionDigest/Tag/Name triple
x7upLime
force-pushed
the
docker-agnostic-KicBase-cache
branch
from
754f676 to
eee5e2c
Compare
k8s-ci-robot
removed
the
needs-rebase
|
PR needs rebase. 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
the
needs-rebase
|
The Kubernetes project currently lacks enough contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle stale |
k8s-ci-robot
added
the
lifecycle/stale
|
The Kubernetes project currently lacks enough active contributors to adequately respond to all PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /lifecycle rotten |
k8s-ci-robot
added
lifecycle/rotten
|
The Kubernetes project currently lacks enough active contributors to adequately respond to all issues and PRs. This bot triages PRs according to the following rules:
You can:
Please send feedback to sig-contributor-experience at kubernetes/community. /close |
|
@k8s-triage-robot: Closed this PR. In response to this:
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. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: x7upLime The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Continues the work started with #15678
(reproposes #15491 )
Fixes the "not yet implemented, see issue #8426" error message when
starting minikube with podman driver.
Undockerizes the code for the kicBase cache.
The output for a
minikube start --driver=podman --container-runtime=crio --rootless=truein a clean environment is now:
(cutting after 🔥 -- podman has other issues as well..)
post_mod.log
Used to be:
pre_mod.log
In the past, podman load from the cache was not supported since podman treated the name:tag@digest triple,
very differently from docker.. and was unreliable at the ends of cache-invalidation.
Now cache-invalidation is performed against contentDigest(a.k.a. imageID), which is maintained
equally by docker and podman.
Thus allowing us to invalidate images inside podman as well.