search

kubernetes / minikube

Run Kubernetes locally
https://minikube.sigs.k8s.io/
Apache License 2.0
29.5k stars 4.89k forks source link

"minikube image ls" confusingly adds "docker.io/" prefix #19343

Open thomasjm opened 3 months ago

thomasjm commented 3 months ago

What Happened?

When you load an image into Minikube, and then list the images using minikube image ls, it may sometimes add docker.io/ or docker.io/library/ as a prefix to your image.

I was scratching my head over why this was happening, until I found this code:

https://github.com/kubernetes/minikube/blob/master/pkg/minikube/cruntime/docker.go#L699-L719

These prefixes are added based on a pretty arbitrary criterion -- whether the image name contains a dot or not. Apparently the idea is to force a leading domain name to be on an image, so as to turn busybox:latest into docker.io/library/busybox:latest.

This behavior might have made sense when DockerHub was the only game in town, but I don't think Minikube should a) misrepresent the actual RepoTags coming from Docker or b) privilege the docker.io prefix in this way. After all, you can construct and load container images that have nothing to do with docker.io but will still get prefixed.

When the reported RepoTags are different from the one in the image I loaded, it makes it difficult to verify that an image was loaded correctly. This is especially vexing when combined with the fact that minikube image load sometimes silently fails.

If there's a good reason for this behavior please let me know. I just experimented with removing it and it seems to work fine. PR incoming.

Attach the log file

-

Operating System

Ubuntu

Driver

Docker

afbjorklund commented 3 months ago

The implementation is buggy, but it needs to use long names for some of the runtimes to work properly.

Kubernetes itself adds the prefix, so for instance containerd can't run images created by buildkitd

It is not supposed to do anything weird, like replace qualified images with docker.io or somesuch

thomasjm commented 3 months ago

Wow, thanks for the instant response!

I stripped out the addDockerIO function in the attached PR, and it was only used in one place -- in the ListImages function implementation in cruntime/docker.go. I assumed this function was only used to power the minikube image ls command. You're saying these results are also used internally?

afbjorklund commented 3 months ago

I think it could use some refactoring (minikube image that is), and some better error reporting etc.

But the main idea was to provide a unified interface to all three container runtimes, for load and build.

I am not aware that other pieces of minikube should use it, but the code is quite old so that could be...

Upstream (k8s) uses this wrapper: https://github.com/distribution/reference/blob/main/normalize.go

Qualifying the names is a feature:

$ docker pull busybox
Using default tag: latest
latest: Pulling from library/busybox
Digest: sha256:9ae97d36d26566ff84e8893c64a6dc4fe8ca6d1144bf5b87b2b85a32def253c7
Status: Image is up to date for busybox:latest
docker.io/library/busybox:latest

Other discussions around it include:

https://www.redhat.com/sysadmin/container-image-short-names

https://www.redhat.com/en/blog/be-careful-when-pulling-images-short-name

thomasjm commented 3 months ago

Qualifying the names is a feature:

But if I inspect the pulled image, the long name is not represented in the RepoTags:

$ docker image inspect busybox | jq -c '.[].RepoTags'
["busybox:latest"]

Nor is it contained in the on-disk representation when you do docker save. That's why I think it's confusing that when I do minikube image ls --format json, I get JSON where the repoTags fields are prefixed.

My reading of the Red Hat articles is that short vs. long names are a "pushing and pulling" concern. When you're transferring images, you need a domain name to know where to send/receive them. But when the images are "at rest," stored in the local Docker/Podman storage, they are represented only by their repo tags.

(Of course, all I really want is a way to reliably test if my image has been successfully loaded to the cluster via minikube image save. Or even better, for minikube image save simply to never fail silently.)

afbjorklund commented 3 months ago

Yeah, unfortunately Kubernetes (and e.g. CRI) does not have a position on how to specify/qualify images.

Anyway the feature was added since containerd is a bit more picky about image names than dockerd was.

k8s-triage-robot commented 2 weeks ago

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

You can:

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

thomasjm commented 2 weeks ago

/remove-lifecycle stale

There was some discussion over this recently