Skip to content
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

Save kic base image to cache on --download-only if docker isn't running #8366

Closed
priyawadhwa opened this issue Jun 3, 2020 · 5 comments · Fixed by #8417
Closed

Save kic base image to cache on --download-only if docker isn't running #8366

priyawadhwa opened this issue Jun 3, 2020 · 5 comments · Fixed by #8417
Assignees
Labels
co/docker-driver Issues related to kubernetes in container kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Milestone

Comments

@priyawadhwa
Copy link

priyawadhwa commented Jun 3, 2020

Right now, if we run

minikube start --download-only --driver docker

it fails if docker is not running.

We should be able to download all required artifacts even if docker is not yet up. If we detect that docker isn't running, we should save the kic base image as a tarball in the minikube cache and load it if it's there on minikube start

This way we still download all required artifacts when --download-only is passed in.

@priyawadhwa priyawadhwa added kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Jun 3, 2020
@priyawadhwa priyawadhwa added this to the v1.12.0-candidate milestone Jun 3, 2020
@priyawadhwa priyawadhwa self-assigned this Jun 3, 2020
@afbjorklund
Copy link
Collaborator

I don't think that it is loaded from the cache either, so maybe do something like #7766

We also have a timeout-and-crash situation, with the podman driver on a slow connection.
Since for that driver we skip the caching, it is downloaded on "run" - but only gets 4 minutes

It would be great if this new caching solution also works with podman, not only docker...

@afbjorklund afbjorklund added the co/docker-driver Issues related to kubernetes in container label Jun 4, 2020
@priyawadhwa
Copy link
Author

Thanks @afbjorklund ! I'll probably do exactly what you described in #7766 to verify both tag & digest from the loaded image.

I'm guessing there's an analogous command to docker load for podman such that it could use the same tarball?

@tstromberg
Copy link
Contributor

This particular issue also came up on Slack by someone who noted that our guidance on https://minikube.sigs.k8s.io/docs/handbook/offline/ does not currently work for Docker users, because it wasn't trying to load kicbase from the local cache directory.

@afbjorklund
Copy link
Collaborator

afbjorklund commented Jun 6, 2020

I'm guessing there's an analogous command to docker load for podman such that it could use the same tarball?

Yeah, it is sudo -n podman load 😀

The main issue with either command is that they do not preserve digests, as noted in the issue.
We worked around this by patching the "go-containerregistry" library to save and load a digest...

Most likely we should keep these system containers outside of the image cache ? Next to the iso.
Otherwise we risk trying to load them inside the container too, which doesn't really make sense.

Currently they are about the same size (iso/base).
Although the docker image is a lot (5x) bigger on disk.

175M	~/cache/iso/minikube-v1.10.0.iso
REPOSITORY                    TAG                 IMAGE ID            CREATED             SIZE
gcr.io/k8s-minikube/kicbase   v0.0.10             e6bc41c39dc4        5 weeks ago         974MB

There's a lot (30%) to save on better compression.
(due to the much bigger compression window in xz)

$ docker save gcr.io/k8s-minikube/kicbase:v0.0.10 | pigz -9 > kicbase.tgz

314M	kicbase.tgz
$ docker save gcr.io/k8s-minikube/kicbase:v0.0.10 | pixz > kicbase.txz

217M	kicbase.txz

But I guess using gzip is still smaller than e.g. lz4.
(which ends up at 450M, for this 933M tarball)

@priyawadhwa
Copy link
Author

priyawadhwa commented Jun 8, 2020

@afbjorklund thanks! I just opened #8417, which should make it easier to load an image into podman once merged (I included a TODO about that in the PR)

Most likely we should keep these system containers outside of the image cache ? Next to the iso.
Otherwise we risk trying to load them inside the container too, which doesn't really make sense.

I kept the image in the image cache just to take advantage of the existing code for saving images to the cache. I personally don't mind having it there, since we have an explicit list of images that we try to load inside the container so it shouldn't end up in minikube.

I ended up stripping the digest, so both podman/docker should be the same now. The digest is still verified though, as it's included in the name of the tarball.

It would be great to improve the compression of the image tarball, but doesn't that assume that all users have a certain compressor installed (gzip, pixz etc)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
co/docker-driver Issues related to kubernetes in container kind/feature Categorizes issue or PR as related to a new feature. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants