Prefer read/write images over read/only images

[rhatdan]

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. It will
still return errors if only multiple read/only images or multiple
read/write images exists. Multipl read/write images should be
impossible.

Fixes: #8176

Signed-off-by: Daniel J Walsh dwalsh@redhat.com

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [rhatdan]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[rhatdan]

@edsantiago @mtrmac PTAL

[rhatdan]

@containers/podman-maintainers PTAL

[vrothberg]

We need a test for it. Too easy to regress if we change change code.

[edsantiago]

We already have a test; it's disabled. @rhatdan can you try removing the Skip and see if CI passes?

podman/test/e2e/exists_test.go

Line 41 in 70284b1

Skip("FIXME-8165: shortnames don't seem to work with quay (#8176)")

[mtrmac]

If #8176 (comment) is right that the define.ErrMultipleImages check doesn’t work, this PR does not yet fix all of that bug report.

[rhatdan]

@mtrmac PTAL
I changed to return the multiple "R/O" images error if R/W image is not found and there are multiple images.

[mtrmac]

@rhatdan Still #8609 (comment) .

[rhatdan]

> So if, searching for alpine, there is a read-only example.com/alpine, and a read-write example.net/alpine, this will silently ignore 
> the ambiguity and only return the read-write one. That doesn’t look right to me.

With additional stores this can happen, since the underlying containers/storage will show an older version of alpine. In this case we will prefer the read/write store, since this was either pulled in a newer version of ":latest" or rebuilt/tagged a newer version of latest. This is the situation that @edsantiago created to see this problem in the first place.

[mtrmac]

With additional stores this can happen, since the underlying containers/storage will show an older version of alpine.

I’m not talking about different versions, but entirely different images. These are example.com/alpine and something-entirely-different-and-not-example.com/alpine.

(Your argument for accepting the newer image if both have exactly the same tag matching the user input, instead of failing on any change to the image, sounds very reasonable. I’m not at all sure about matching by ID. But that’s not my primary concern.)

[mtrmac]

ACK overall.

---

Still outstanding:

If #8176 (comment) is right that the define.ErrMultipleImages check doesn’t work, this PR does not yet fix all of that bug report.

or maybe split that into a separate tracked issue?

[baude]

/lgtm