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

podman image exists, with vfs.imagestore and image from quay, fails #8176

Closed
edsantiago opened this issue Oct 28, 2020 · 7 comments · Fixed by #8609
Closed

podman image exists, with vfs.imagestore and image from quay, fails #8176

edsantiago opened this issue Oct 28, 2020 · 7 comments · Fixed by #8609
Assignees
Labels
In Progress This issue is actively being worked by the assignee, please do not work on this at this time. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. stale-issue

Comments

@edsantiago
Copy link
Member

I have no idea how to even describe this, so someone please feel free to change the title to a more descriptive one.

Discovered during testing of #8165 -- a podman setup using vfs and vfs.imagestore will, under certain circumstances involving images from quay.io, falsely claim false on image exists. A reproducer is worth a thousand of my words:

#!/bin/bash

set -ex

PROOT=/dev/shm/mypodman
rm -rf $PROOT
mkdir -p $PROOT

# What's really weird is that this test *succeeds* with docker.io
IMAGE=quay.io/libpod/alpine:latest
#IMAGE=docker.io/library/alpine:latest

# Pull something in order to initialize imagestore
# Without this initial pull, the last pull below fails with:
# Error: error opening "/dev/shm/mypodman/cache/vfs-images/images.lock": no such file or directory
PODMAN="./bin/podman --root $PROOT/cache --storage-driver vfs"
$PODMAN pull -q $IMAGE

# And now, switch --root to something empty, and set imagestore to the above.
# Pull the same image, and boom.
# If you comment out the 'pull' below, 'image exists' succeeds.
PODMAN="./bin/podman --storage-opt vfs.imagestore=$PROOT/cache --root $PROOT/root --storage-driver vfs"
$PODMAN pull -q $IMAGE

# Shows two images, differing only in the rightmost 'R/O' column
$PODMAN images -a

# This fails!
$PODMAN image exists alpine
@rhatdan
Copy link
Member

rhatdan commented Oct 28, 2020

/assign

@rhatdan rhatdan added the In Progress This issue is actively being worked by the assignee, please do not work on this at this time. label Oct 28, 2020
@rhatdan rhatdan closed this as completed Oct 31, 2020
edsantiago added a commit to edsantiago/libpod that referenced this issue Nov 2, 2020
PR containers#8178 fixed containers#8176 but didn't remove the Skip(). This
reinstates the test.

Signed-off-by: Ed Santiago <santiago@redhat.com>
@edsantiago
Copy link
Member Author

I don't think this is fixed; the original test continues to fail. So does the reproducer in comment 0.

@edsantiago edsantiago reopened this Nov 2, 2020
@github-actions
Copy link

github-actions bot commented Dec 3, 2020

A friendly reminder that this issue had no activity for 30 days.

@rhatdan
Copy link
Member

rhatdan commented Dec 4, 2020

This test is being screwed up by the mirroring.

If you run images, you see that you have these images.
REPOSITORY TAG IMAGE ID CREATED SIZE R/O
quay.io/libpod/alpine latest 961769676411 15 months ago 5.85 MB false
quay.io/libpod/alpine latest 961769676411 15 months ago 5.85 MB true

But the image exists is looking for
docker.io/library/alpine

Since the image exists does not go through the mirror code, it fails.

@rhatdan
Copy link
Member

rhatdan commented Dec 4, 2020

@vrothberg @mtrmac This is happening in containers/image GetStoreImage() call.

Should this be translating the local users name to the mirror name?

@rhatdan
Copy link
Member

rhatdan commented Dec 4, 2020

		img, err = is.Transport.GetStoreImage(ir.store, ref)

@mtrmac
Copy link
Collaborator

mtrmac commented Dec 4, 2020

@rhatdan I assume by “mirror” you consistently mean vfs.imagestore, don’t you? There is no “mirror name” AFAICS, just two places to look up images with a name, and no way to name explicitly one or the other image if there is more than one in the multiple stores.

At a first glance, it’s not too surprising that docker.io/* and quay.io/* images behave differently (assuming quay.io is not in the unqualified-search-registries list), because the former one uses the

candidates, err := shortnames.ResolveLocally(sys, inputName)
if err != nil {
return "", nil, err
}
for _, candidate := range candidates {
img, err := ir.store.Image(candidate.String())
if err == nil {
return candidate.String(), img, nil
}
}
// Backwards compat: normalize to docker.io as some users may very well
// rely on that.
ref, err := is.Transport.ParseStoreReference(ir.store, inputName)
if err == nil {
img, err = is.Transport.GetStoreImage(ir.store, ref)
if err == nil {
return inputName, img, nil
}
}
path (including the hard-coded docker.io default at the bottom).

For quay.io, this triggers the undocumented findImageInRepotags heuristic (which, ceterum autem censeo , probably should not exist at all, but I can’t find any record of why it exists so I’m not sure).


Just reading the code, without testing it at all: AFAICS in the quay.io case:

  • images, err := ir.GetImages()
    finds both images (same name, different backing stores)
  • findImageInRepoTags finds both as matches, fails with errors.Wrapf(define.ErrMultipleImages, searchName)
  • The caller in
    if err == nil {
    return inputName, repoImage, nil
    }
    return "", nil, errors.Wrapf(ErrNoSuchImage, err.Error())
    just throws away that value and returns ErrNoSuchImage
  • … causing the error value check in
    if errors.Cause(err) == define.ErrMultipleImages {
    return &entities.BoolReport{Value: true}, nil
    to not trigger, and Exists to return false.

rhatdan added a commit to rhatdan/podman that referenced this issue Dec 22, 2020
With additional stores there is a risk that you could have
multiple images with the same name.  IE An older image in a
read/only store versus a newer version in the read/write store.

This patch will ignore multiple images with the same name iff
one is read/write and all of the others are read/only.

Fixes: containers#8176

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
In Progress This issue is actively being worked by the assignee, please do not work on this at this time. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. stale-issue
Projects
None yet
3 participants