Skip to content

Commit

Permalink
Prefer read/write images over read/only images
Browse files Browse the repository at this point in the history
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: #8176

Signed-off-by: Daniel J Walsh <dwalsh@redhat.com>
  • Loading branch information
rhatdan committed Dec 22, 2020
1 parent cfdb8fb commit e577ddf
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 13 deletions.
57 changes: 45 additions & 12 deletions libpod/image/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ import (
// a match on name:tag
func findImageInRepotags(search imageParts, images []*Image) (*storage.Image, error) {
_, searchName, searchSuspiciousTagValueForSearch := search.suspiciousRefNameTagValuesForSearch()
var results []*storage.Image
type Candidate struct {
name string
image *Image
}
var candidates []Candidate
for _, image := range images {
for _, name := range image.Names() {
d, err := decompose(name)
Expand All @@ -29,23 +33,52 @@ func findImageInRepotags(search imageParts, images []*Image) (*storage.Image, er
continue
}
_, dName, dSuspiciousTagValueForSearch := d.suspiciousRefNameTagValuesForSearch()
if dName == searchName && dSuspiciousTagValueForSearch == searchSuspiciousTagValueForSearch {
results = append(results, image.image)
if dSuspiciousTagValueForSearch != searchSuspiciousTagValueForSearch {
continue
}
// account for registry:/somedir/image
if strings.HasSuffix(dName, "/"+searchName) && dSuspiciousTagValueForSearch == searchSuspiciousTagValueForSearch {
results = append(results, image.image)
continue
if dName == searchName || strings.HasSuffix(dName, "/"+searchName) {
candidates = append(candidates, Candidate{
name: name,
image: image,
})
}
}
}
if len(results) == 0 {
return &storage.Image{}, errors.Errorf("unable to find a name and tag match for %s in repotags", searchName)
} else if len(results) > 1 {
return &storage.Image{}, errors.Wrapf(define.ErrMultipleImages, searchName)
if len(candidates) == 0 {
return nil, errors.Errorf("unable to find a name and tag match for %s in repotags", searchName)
}

// If more then one candidate and the candidates all have same name
// and only one is read/write return it.
// Othewise return error with the list of candidates
if len(candidates) > 1 {
var (
rwImage *Image
rwImageCnt int
)
names := make(map[string]bool)
for _, c := range candidates {
names[c.name] = true
if !c.image.IsReadOnly() {
rwImageCnt++
rwImage = c.image
}
}
// If only one name used and have read/write image return it
if len(names) == 1 && rwImageCnt == 1 {
return rwImage.image, nil
}
keys := []string{}
for k := range names {
keys = append(keys, k)
}
if rwImageCnt > 1 {
return nil, errors.Wrapf(define.ErrMultipleImages, "found multiple read/write images %s", strings.Join(keys, ","))
} else {
return nil, errors.Wrapf(define.ErrMultipleImages, "found multiple read/only images %s", strings.Join(keys, ","))
}
}
return results[0], nil
return candidates[0].image.image, nil
}

// getCopyOptions constructs a new containers/image/copy.Options{} struct from the given parameters, inheriting some from sc.
Expand Down
1 change: 0 additions & 1 deletion test/e2e/exists_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ var _ = Describe("Podman image|container exists", func() {
Expect(session).Should(Exit(0))
})
It("podman image exists in local storage by short name", func() {
Skip("FIXME-8165: shortnames don't seem to work with quay (#8176)")
session := podmanTest.Podman([]string{"image", "exists", "alpine"})
session.WaitWithDefaultTimeout()
Expect(session).Should(Exit(0))
Expand Down

0 comments on commit e577ddf

Please sign in to comment.