Skip to content

Commit 706bcf2

Browse files
committed
pull,load: use *Image instead of re-resolving via name
Following commit fixes a `race` condition in `libimage` because in `Pull(` after performing `copy` from remote sources it agains attempts to resolve image via `LookupImage`, any operation between `copy` and `LookupImage` can remove `name` from the recently pulled image. Causing race in builds. This issue was discoverd while working on PR containers/buildah#5971 ``` buildah build -t test --jobs=2 --skip-unused-stages=false . ``` Containerfile ``` FROM quay.io/jitesoft/alpine RUN arch FROM --platform=linux/arm64 quay.io/jitesoft/alpine AS foreign ``` Following commit also addresses the commit containers@e2324dd by performing the neccessary refactor. No functional change in public exposed API, exisiting tests should pass as-is. [NO NEW TESTS NEEDED] Signed-off-by: flouthoc <[email protected]>
1 parent a56094c commit 706bcf2

File tree

3 files changed

+99
-85
lines changed

3 files changed

+99
-85
lines changed

libimage/image_test.go

+1-5
Original file line numberDiff line numberDiff line change
@@ -61,15 +61,11 @@ func TestImageFunctions(t *testing.T) {
6161
// Just make sure that the ID has 64 characters.
6262
require.True(t, len(image.ID()) == 64, "ID should be 64 characters long")
6363

64-
// Make sure that the image we pulled by digest is the same one we
65-
// pulled by tag.
66-
require.Equal(t, origDigest.String(), image.Digest().String(), "digests of pulled images should match")
67-
6864
// NOTE: we're recording two digests. One for the image and one for the
6965
// manifest list we chose it from.
7066
digests := image.Digests()
7167
require.Len(t, digests, 2)
72-
require.Equal(t, origDigest.String(), digests[0].String(), "first recorded digest should be the one of the image")
68+
require.Contains(t, digests, origDigest, "original digest string should be present in the digests array of new pulled image")
7369

7470
// containers/podman/issues/12729: make sure manifest lookup returns
7571
// the correct error for both digests.

libimage/load.go

+7-3
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@ func (r *Runtime) doLoadReference(ctx context.Context, ref types.ImageReference,
3030
case dockerArchiveTransport.Transport.Name():
3131
images, err = r.loadMultiImageDockerArchive(ctx, ref, &options.CopyOptions)
3232
default:
33-
images, err = r.copyFromDefault(ctx, ref, &options.CopyOptions)
33+
_, images, err = r.copyFromDefault(ctx, ref, &options.CopyOptions)
3434
}
3535
return images, ref.Transport().Name(), err
3636
}
@@ -49,6 +49,9 @@ func (r *Runtime) LoadReference(ctx context.Context, ref types.ImageReference, o
4949
// Load loads one or more images (depending on the transport) from the
5050
// specified path. The path may point to an image the following transports:
5151
// oci, oci-archive, dir, docker-archive.
52+
//
53+
// Load returns a string slice with names of recently loaded images.
54+
// If images are unnamed in the source, it returns a string slice of image IDs instead.
5255
func (r *Runtime) Load(ctx context.Context, path string, options *LoadOptions) ([]string, error) {
5356
logrus.Debugf("Loading image from %q", path)
5457

@@ -142,7 +145,8 @@ func (r *Runtime) loadMultiImageDockerArchive(ctx context.Context, ref types.Ima
142145
// should.
143146
path := ref.StringWithinTransport()
144147
if err := fileutils.Exists(path); err != nil {
145-
return r.copyFromDockerArchive(ctx, ref, options)
148+
_, names, err := r.copyFromDockerArchive(ctx, ref, options)
149+
return names, err
146150
}
147151

148152
reader, err := dockerArchiveTransport.NewReader(r.systemContextCopy(), path)
@@ -163,7 +167,7 @@ func (r *Runtime) loadMultiImageDockerArchive(ctx context.Context, ref types.Ima
163167
var copiedImages []string
164168
for _, list := range refLists {
165169
for _, listRef := range list {
166-
names, err := r.copyFromDockerArchiveReaderReference(ctx, reader, listRef, options)
170+
_, names, err := r.copyFromDockerArchiveReaderReference(ctx, reader, listRef, options)
167171
if err != nil {
168172
return nil, err
169173
}

0 commit comments

Comments
 (0)