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.

Sign up for GitHub

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

Jump to bottom

docker-archive: read+write #998

Closed
wants to merge 37 commits into from
Closed

docker-archive: read+write #998

wants to merge 37 commits into from

Commits on Aug 7, 2020

  1. Remove an obsolete FIXME 

    We do support the legacy format nowadays.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    80c2a00 View commit details
    Browse the repository at this point in the history

  2. Fix a comment 

    Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    a372946 View commit details
    Browse the repository at this point in the history

  3. Move docker/tarfile.Destination to docker/internal/tarfile.Destination 

    ... so that we can add more functionality to it without making it
public API.

Keep the original docker/tarfile.Destination as a compatibility shim.

ManifestItem is moved to the private package to avoid an import cycle,
but remains visible.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    b5d6158 View commit details
    Browse the repository at this point in the history

  4. Use the docker/internal/tarfile.Destination from docker/daemon and do… 

    …cker/archive

... instead of the forwarding shims.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    e1e5452 View commit details
    Browse the repository at this point in the history

  5. Remove deprecated non-SystemContext functions from docker/internal.ta… 

    …rfile

his is a private API now, so we can just drop it; rename the
...With[System]Context varianteto use the shorter name, and update
all callers.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    5a6886c View commit details
    Browse the repository at this point in the history

  6. Introduce Destination.configPath and Destination.physicalLayerPath 

    We will be splitting uses of these paths across two objects, so
make them a private API instead of a copy&pasted convention.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    9680473 View commit details
    Browse the repository at this point in the history

  7. Split docker/internal.tarfile.Writer from Destination 

    ... to eventually allow creating multiple Destinations from a single
Writer.

NOTE: This only splits the implementation into two, mostly at function
boundary; it does NOTHING to support writing multiple images; top-level
metadata is written independently on each Destination.PutManifest .
This will be fixed soon.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    a9938d1 View commit details
    Browse the repository at this point in the history

  8. Move createRepositoriesFile to a bit better place 

    ... to follow the PutManifest flow.

Should not change behavior, nothing about the code was changed.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    1214e24 View commit details
    Browse the repository at this point in the history

  9. Split Writer.createManifest from Destination.PutManifest 

    This still only supports one image, but we will fix that momentarily.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    f4d4799 View commit details
    Browse the repository at this point in the history

  10. Reorganize docker/internal/tarfile.Writer.createManifest a bit 

    ... only to make future commits easier to review.

This introduces a bit of inefficiency (creating an on-stack
ManifestItem only to copy it into a single-element array), but
that will be gone momentarily.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    e5c7ed8 View commit details
    Browse the repository at this point in the history

  11. Move the computation of layerPaths in docker-archive 

    ... from writeLegacyLayerMetadata to the non-legacy createManifest.
Now that we have a dedicated function for computing the path consistently,
introducing another reference to it does not hurt maintainability, and the
small efficiency gain of computing the path only once is not really worth
coupling the legacy/non-legacy code, especially when the legacy code
is going to get more complex.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    efab506 View commit details
    Browse the repository at this point in the history

  12. Implement writing multiple images in the modern format. 

    NOTE: This is not sufficient to create correct multi-image archives
yet, the legacy format is still invalid.

This will eventually allow creating multiple Destinations from a single
Writer.

Should not change behavior for current callers, except that possible
JSON failures are now reported later.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    bfe71f6 View commit details
    Browse the repository at this point in the history

  13. Split createSingleLegacyLayer from writeLegacyLayerMetadata 

    ... so that we can later only do this for layers that haven't been
created yet.

Should not change behavior, apart from timing of some reported errors.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    4f13a6b View commit details
    Browse the repository at this point in the history

  14. Move legacy layer ID computation to a bit later 

    ... because it will soon depend on the rest of layerConfig.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    29ddfdd View commit details
    Browse the repository at this point in the history

  15. Merge writeLegacyMetadata and createRepositoriesFile 

    ... so that the caller does not have to care about lastLayerID and
empty images.

This preserves the current behavior of silently ignoring tags intended
for empty images. That's probably not quite right, but also not a subject
of this PR.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    3bef56d View commit details
    Browse the repository at this point in the history

  16. Implement writing multiple images in the legacy format 

    Now that both the legacy and non-legacy format can incrementally add
images to a single archive, this will allow creating multiple
Destinations from a writer Writer.

Image IDs are now generated differently; that may be observable by
very old Docker versions.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    075bc06 View commit details
    Browse the repository at this point in the history

  17. Separate tarfile.Writer creation from Destination creation 

    At least docker/archive will need to deal with the two
optionally separately, so such a separate constructor must exist;
and the other callers are not much more complex if they separate
it as well.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    c81a536 View commit details
    Browse the repository at this point in the history

  18. Lock docker/internal/tarfile.Writer to support concurrent uses 

    This does not really allow _concurrency_ because we are streaming
the data to a single io.Writer, but it gives us safety against concurrent
callers (hypothetically when copying multiple images, at least).

Note that we do not currently set HasThreadSafePutBlob, although
we could, because the benefit is probably fairly small (basically it would
parallelize creating on-disk copies of streamed inputs with unknown
size or digest); that might be a wrong guess.

Also adds a sanity check against using the Writer after Finish().

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    28fea18 View commit details
    Browse the repository at this point in the history

  19. Split openArchiveForWriting from docker/archive/newImageDestination 

    We will add another caller in the next commit.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    dbc2073 View commit details
    Browse the repository at this point in the history

  20. Finally, introduce docker/archive.Writer 

    ... which allows creating ImageReferece objects that all write
to a shared tarfile.Writer.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    611759b View commit details
    Browse the repository at this point in the history

  21. Fix an error message on docker-archive:path:name@sha256:$digest 

    It's a possible user error, not a supposedly-unreachable internal
error.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    3c9322d View commit details
    Browse the repository at this point in the history

  22. Remove an obsolete FIXME. 

    Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    0d0ea66 View commit details
    Browse the repository at this point in the history

  23. Move docker/tarfile.Source to docker/internal/tarfile.Source 

    ... so that we can add more functionality to it without making it
public API.

Keep the original docker/tarfile.Source as a compatibility shim.

ManifestItem is moved to the private package to avoid an import cycle,
but remains visible.

docker/tarfile/src_test.go should logically be moved as well, but we
can't do that yet due to the import cycle.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    f358587 View commit details
    Browse the repository at this point in the history

  24. Use the docker/internal/tarfile.Source from docker/daemon and docker/… 

    …archive

... instead of the forwarding shims.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    d0f56fb View commit details
    Browse the repository at this point in the history

  25. Remove deprecated non-SystemContext functions from docker/internal/ta… 

    …rfile

This is a private API now, so we can just drop it; rename the
...With[System]Context variants to use the shorter names, and update
all callers.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    4859364 View commit details
    Browse the repository at this point in the history

  26. Split docker/internal/tarfile.Reader from Source 

    ... to eventually allow creating multiple Sources from a single
Reader (= a temporary file containing a seekable/uncompressed
copy of the archive).

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    de0ae52 View commit details
    Browse the repository at this point in the history

  27. Separate tarfile.Reader creation from Source creation 

    At least docker/archive will need to deal with the two
optionally separately, so such a separate constructor must exist;
and the other callers are not much more complex if they separate
it as well.

This will also allow us to add reference lookup without having
to duplicate the API.

As a concession to the simpler callers, add a closeArchive parameter
(currently always true) that allows them not to worry about
the lifetime of the tarfile.Reader.

Should not change behavior.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    a9767e7 View commit details
    Browse the repository at this point in the history

  28. Read the tarfile manifest already when initializing tarfile.Reader 

    Every caller will need that data; this way it can be shared across
several consumers, and we don't need to synchronize access/creation
of the parsed data structure.

Should not change behavior, except that errors are now reported
earlier.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    ca4e06a View commit details
    Browse the repository at this point in the history

  29. Turn tarfile.Source.LoadTarManifest into a TarManifest 

    We can drop the unused error return value now.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    dbd4761 View commit details
    Browse the repository at this point in the history

  30. Allow choosing an image from tarfile.Reader by reference 

    We already accept the syntax for docker-archive: references,
now implement the lookup instead of warning and ignoring the value.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    7029860 View commit details
    Browse the repository at this point in the history

  31. Introduce docker-archive:path:@Index syntax for reading untagged images 

    Add support for path:@Index (e.g. path:@0, path:@1 ...) reference syntax
to docker-archive.

This will allow reading even untagged images from multi-image archives.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    756bef7 View commit details
    Browse the repository at this point in the history

  32. Introduce docker/archive.Reader 

    with only two operations: Close(), and List() which
returns a set of ImageReference objects that allow accessing
the individual images.

For now, use of every reference triggers creation of a new
tarfile.Reader; that will be fixed momentarily.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    4176316 View commit details
    Browse the repository at this point in the history

  33. Finally, share a tarfile.Reader across archiveSource objects 

    In archive.Reader, embed a reference to tarfile.Reader to
the created ImageReference objects, and use them in NewImageSource.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    be68ec7 View commit details
    Browse the repository at this point in the history

  34. Merge branch 'tarfile-read' into HEAD

    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    eca6f25 View commit details
    Browse the repository at this point in the history

  35. Merge branch 'tarfile-write' into HEAD

    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    e21dc22 View commit details
    Browse the repository at this point in the history

  36. Remove an unused constant 

    Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    820918a View commit details
    Browse the repository at this point in the history

  37. Move docker/tarfile/src_test.go to docker/internal/tarfile 

    , closer to the implementation being tested.

Signed-off-by: Miloslav Trmač <mitr@redhat.com>
    @mtrmac
    mtrmac committed Aug 7, 2020
    Configuration menu
    Copy the full SHA
    d7c80f4 View commit details
    Browse the repository at this point in the history