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

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

Open
thomasjm opened this issue Jul 29, 2024 · 7 comments · May be fixed by #19344
Open

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

thomasjm opened this issue Jul 29, 2024 · 7 comments · May be fixed by #19344

Comments

@thomasjm
Copy link
Contributor

thomasjm commented Jul 29, 2024

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
Copy link
Collaborator

afbjorklund commented Jul 29, 2024

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
Copy link
Contributor Author

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
Copy link
Collaborator

afbjorklund commented Jul 29, 2024

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
Copy link
Contributor Author

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
Copy link
Collaborator

afbjorklund commented Jul 29, 2024

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
Copy link

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

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

  • After 90d of inactivity, lifecycle/stale is applied
  • After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
  • After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

  • Mark this issue as fresh with /remove-lifecycle stale
  • Close this issue with /close
  • Offer to help out with Issue Triage

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

/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 27, 2024
@thomasjm
Copy link
Contributor Author

/remove-lifecycle stale

There was some discussion over this recently

@k8s-ci-robot k8s-ci-robot removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Oct 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants