From 171a2b6cb9bf6f8e448fb74c5d4996e18dd13453 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Miloslav=20Trma=C4=8D?= Date: Sat, 25 Jul 2020 22:08:00 +0200 Subject: [PATCH] Finally, share a tarfile.Reader across archiveSource objects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In archive.Reader, embed a reference to tarfile.Reader to the created ImageReference objects, and use them in NewImageSource. Signed-off-by: Miloslav Trmač --- docker/archive/reader.go | 4 ++-- docker/archive/src.go | 17 +++++++++++++---- docker/archive/transport.go | 23 +++++++++++++++-------- docker/archive/transport_test.go | 2 +- 4 files changed, 31 insertions(+), 15 deletions(-) diff --git a/docker/archive/reader.go b/docker/archive/reader.go index 82ebd9ffa1..7c58487e38 100644 --- a/docker/archive/reader.go +++ b/docker/archive/reader.go @@ -49,14 +49,14 @@ func (r *Reader) List() ([][]types.ImageReference, error) { if !ok { return nil, errors.Errorf("Invalid tag %s (%s): does not contain a tag", tag, parsedTag.String()) } - ref, err := NewReference(r.path, nt) + ref, err := newReference(r.path, nt, -1, r.archive) if err != nil { return nil, errors.Wrapf(err, "Error creating a reference for tag %#v in manifest item @%d", tag, imageIndex) } refs = append(refs, ref) } if len(refs) == 0 { - ref, err := NewIndexReference(r.path, imageIndex) + ref, err := newReference(r.path, nil, imageIndex, r.archive) if err != nil { return nil, errors.Wrapf(err, "Error creating a reference for manifest item @%d", imageIndex) } diff --git a/docker/archive/src.go b/docker/archive/src.go index e850b4b4e4..7acca210ef 100644 --- a/docker/archive/src.go +++ b/docker/archive/src.go @@ -15,11 +15,20 @@ type archiveImageSource struct { // newImageSource returns a types.ImageSource for the specified image reference. // The caller must call .Close() on the returned ImageSource. func newImageSource(ctx context.Context, sys *types.SystemContext, ref archiveReference) (types.ImageSource, error) { - archive, err := tarfile.NewReaderFromFile(sys, ref.path) - if err != nil { - return nil, err + var archive *tarfile.Reader + var closeArchive bool + if ref.archiveReader != nil { + archive = ref.archiveReader + closeArchive = false + } else { + a, err := tarfile.NewReaderFromFile(sys, ref.path) + if err != nil { + return nil, err + } + archive = a + closeArchive = true } - src := tarfile.NewSource(archive, true, ref.ref, ref.sourceIndex) + src := tarfile.NewSource(archive, closeArchive, ref.ref, ref.sourceIndex) return &archiveImageSource{ Source: src, ref: ref, diff --git a/docker/archive/transport.go b/docker/archive/transport.go index de30b22f77..384a84df19 100644 --- a/docker/archive/transport.go +++ b/docker/archive/transport.go @@ -6,6 +6,7 @@ import ( "strconv" "strings" + "github.com/containers/image/v5/docker/internal/tarfile" "github.com/containers/image/v5/docker/reference" ctrImage "github.com/containers/image/v5/image" "github.com/containers/image/v5/transports" @@ -48,6 +49,9 @@ type archiveReference struct { // If not -1, a zero-based index of the image in the manifest. Valid only for sources. // Must not be set if ref is set. sourceIndex int + // If not nil, must have been created from path (but archiveReader.path may point at a temporary + // file, not necesarily path precisely). + archiveReader *tarfile.Reader } // ParseReference converts a string, which should not start with the ImageTransport.Name prefix, into an Docker ImageReference. @@ -86,21 +90,23 @@ func ParseReference(refString string) (types.ImageReference, error) { } } - return newReference(path, nt, sourceIndex) + return newReference(path, nt, sourceIndex, nil) } // NewReference returns a Docker archive reference for a path and an optional reference. func NewReference(path string, ref reference.NamedTagged) (types.ImageReference, error) { - return newReference(path, ref, -1) + return newReference(path, ref, -1, nil) } // NewIndexReference returns a Docker archive reference for a path and a zero-based source manifest index. func NewIndexReference(path string, sourceIndex int) (types.ImageReference, error) { - return newReference(path, nil, sourceIndex) + return newReference(path, nil, sourceIndex, nil) } -// newReference returns a docker archive reference for a path and an optional reference or sourceIndex. -func newReference(path string, ref reference.NamedTagged, sourceIndex int) (types.ImageReference, error) { +// newReference returns a docker archive reference for a path, an optional reference or sourceIndex, +// and optionally a tarfile.Reader matching path. +func newReference(path string, ref reference.NamedTagged, sourceIndex int, + archiveReader *tarfile.Reader) (types.ImageReference, error) { if strings.Contains(path, ":") { return nil, errors.Errorf("Invalid docker-archive: reference: colon in path %q is not supported", path) } @@ -114,9 +120,10 @@ func newReference(path string, ref reference.NamedTagged, sourceIndex int) (type return nil, errors.Errorf("Invalid docker-archive: reference: index @%d must not be negative", sourceIndex) } return archiveReference{ - path: path, - ref: ref, - sourceIndex: sourceIndex, + path: path, + ref: ref, + sourceIndex: sourceIndex, + archiveReader: archiveReader, }, nil } diff --git a/docker/archive/transport_test.go b/docker/archive/transport_test.go index 008a026618..e0328f9c98 100644 --- a/docker/archive/transport_test.go +++ b/docker/archive/transport_test.go @@ -134,7 +134,7 @@ func TestNewReference(t *testing.T) { // Complete coverage testing of the private newReference here as well ntRef := namedTaggedRef(t, "busybox:latest") - _, err = newReference("path", ntRef, 0) + _, err = newReference("path", ntRef, 0, nil) assert.Error(t, err) }