image save: make output tarball OCI compliant by cpuguy83 · Pull Request #44598 · moby/moby

cpuguy83 · 2022-12-07T00:25:16Z

Moves blobs for the tarball into the OCI compliant paths (blobs/<alg>/<digest>)
Removes old VERSION file which has no meaning and can't be used anyway in the new format
Adds oci-layout, oci manifest, and index files to the outputted tars

For the new files this just goes ahead and uses OCI media types (instead of old docker types) since these are new and should not break anything.

All this works because the old docker layout is dependent on the manifest.json file at the root of the tar which has paths to each of the blobs in the tarball. The change updates the paths in the manifest to point to the new paths.

This is the same technique used by buildkit to support both OCI tar formats as well as legacy docker ones.

thaJeztah · 2022-12-07T12:53:35Z

Looks like there's some tests to adjust (or perhaps separate tests)

=== FAIL: amd64.integration-cli TestDockerCLISaveLoadSuite/TestSaveCheckTimes (0.04s)
    docker_cli_save_load_test.go:127: assertion failed: error is not nil: exit status 1: failed to save repo with image ID and 'repositories' file: , exit status 1
    --- FAIL: TestDockerCLISaveLoadSuite/TestSaveCheckTimes (0.04s)

=== FAIL: amd64.integration-cli TestDockerCLISaveLoadSuite/TestSaveDirectoryPermissions (0.79s)
    docker_cli_save_load_test.go:302: assertion failed: error is not nil: open /tmp/save-layers-with-directories3203220919/image-extraction-dir/blobs/layer.tar: no such file or directory: failed to open /tmp/save-layers-with-directories3203220919/image-extraction-dir/blobs/layer.tar: open /tmp/save-layers-with-directories3203220919/image-extraction-dir/blobs/layer.tar: no such file or directory
    --- FAIL: TestDockerCLISaveLoadSuite/TestSaveDirectoryPermissions (0.79s)

=== FAIL: amd64.integration-cli TestDockerCLISaveLoadSuite/TestSaveRepoWithMultipleImages (0.84s)
    docker_cli_save_load_test.go:266: assertion failed: 
        --- actual
        +++ expected
          []string(
        - 	nil,
        + 	{
        + 		"1c35c441208254cb7c3844ba95a96485388cef9ccc0646d562c7fc026e04c807",
        + 		"af4544b16613a2573c47ad04472359062ae9a0968bd5cfb9fe271861df9fe286",
        + 		"c4305557e19f493d6f6eec7a544c2ae248ab17528a8b34fe9339cba8d70059e4",
        + 	},
          )
        : archive does not contains the right layers: got [], expected [1c35c441208254cb7c3844ba95a96485388cef9ccc0646d562c7fc026e04c807 af4544b16613a2573c47ad04472359062ae9a0968bd5cfb9fe271861df9fe286 c4305557e19f493d6f6eec7a544c2ae248ab17528a8b34fe9339cba8d70059e4], output: "sha256:1c35c441208254cb7c3844ba95a96485388cef9ccc0646d562c7fc026e04c807"
    --- FAIL: TestDockerCLISaveLoadSuite/TestSaveRepoWithMultipleImages (0.84s)

AkihiroSuda · 2022-12-07T16:18:32Z

+	ociLayoutFilename = "oci-layout"
+	ociLayoutContent  = `{"imageLayoutVersion": "1.0.0"}`
 )



Can we just use https://github.com/containerd/containerd/blob/main/images/archive/exporter.go ?
For containerd-integration mode.

I believe that's the plan.

tianon

Generally looks OK to me! I made a couple minor comments, but I don't think it's anything very serious. I'm both looking forward to having/using this and to this code going away with the containerd integration. 😄

This moves the blobs around so they follow the OCI spec. Note that because docker reads paths from the manifest.json inside the tar this is not a breaking change. This does, however, remove the old layer "VERSION" file which had a big "why is this even here" in the code comments. I suspect it does not matter at all even for really old versions of Docker. In any case it is a useless file for any even relatively modern version of Docker. Signed-off-by: Brian Goff <cpuguy83@gmail.com>

cpuguy83 · 2023-05-24T01:18:37Z

 	assert.NilError(c, err, "failed to save repo with image ID and 'repositories' file: %s, %v", out, err)
 }

-func (s *DockerCLISaveLoadSuite) TestSaveCheckTimes(c *testing.T) {


Note, these tests were incorrectly poking at the saved tarball (assuming a file location instead of looking at the manifest).
I had to do major surgery on these so I just moved them over to integration/.

thaJeztah

Went through some of the changes (but perhaps need to take another look later); left some comments / suggestions / questions inline, but overall looking 👍

thaJeztah · 2023-05-24T06:14:05Z

+		f, err := tarfs.Open(p)
+		assert.NilError(t, err)
+
+		entries, err := listTar(f)


Wondering if this listTar could (not for this PR) also handled by your tar2go (fs.Glob(tarfs, "dev/*") and/or fs.WalkDir(tarfs)) 🤩

This makes the output of `docker save` fully OCI compliant. When using the containerd image store, this code is not used. That exporter will just use containerd's export method and should give us the output we want for multi-arch images. Signed-off-by: Brian Goff <cpuguy83@gmail.com>

cpuguy83 · 2023-05-30T19:06:11Z

This is all updated.

thaJeztah

LGTM

AkihiroSuda · 2024-01-12T13:22:51Z

+				Size:      int64(len(imageDescr.image.RawJSON())),
+				Platform:  &imgPlat,
+			},
+			Layers: foreign,


Non-foreign layers were accidentally dropped?

[v25] docker save produces an OCI manifest with no layers #47065

Starting from Docker 25.0 , the save command will dump the image layers in OCI compatible format: moby/moby#44598 This breaks our tarball build that was using the 'layer.tar' file to contruct the final tarball. Let's make the target a bit smart so it can handle all variants by parsing the docker manifest.json file to find the layer and use it. The 'make tarball' now requires jq for simplicity. Signed-off-by: Djalal Harouni <tixxdz@gmail.com>

Starting from Docker 25.0 , the save command will dump the image layers in OCI compatible format: moby/moby#44598 This breaks our tarball build that was using the 'layer.tar' file to construct the final tarball. Let's make the target a bit smart so it can handle all variants by parsing the docker manifest.json file to find the layer and use it. The 'make tarball' now requires jq for simplicity. Signed-off-by: Djalal Harouni <tixxdz@gmail.com>

[ Upstream main 5cc9bce ] Starting from Docker 25.0 , the save command will dump the image layers in OCI compatible format: moby/moby#44598 This breaks our tarball build that was using the 'layer.tar' file to construct the final tarball. Let's make the target a bit smart so it can handle all variants by parsing the docker manifest.json file to find the layer and use it. The 'make tarball' now requires jq for simplicity. Signed-off-by: Djalal Harouni <tixxdz@gmail.com>

cpuguy83 requested review from tianon and tonistiigi December 7, 2022 00:25

AkihiroSuda reviewed Dec 7, 2022

View reviewed changes

thaJeztah added status/2-code-review area/images Image Service labels Apr 3, 2023

tianon approved these changes Apr 7, 2023

View reviewed changes

Comment thread image/tarexport/save.go Outdated

Comment thread image/tarexport/save.go

Comment thread image/tarexport/save.go Outdated

cpuguy83 force-pushed the save_tar_oci branch 4 times, most recently from f8f7e23 to 8c5db61 Compare May 24, 2023 01:16

cpuguy83 commented May 24, 2023

View reviewed changes

cpuguy83 force-pushed the save_tar_oci branch from 8c5db61 to 9859d86 Compare May 24, 2023 01:19

thaJeztah requested changes May 24, 2023

View reviewed changes

thaJeztah mentioned this pull request May 24, 2023

c8d: ImageService.softImageDelete: use OCI and containerd constants #45609

Merged

cpuguy83 force-pushed the save_tar_oci branch 2 times, most recently from 8f28912 to 0bc8bde Compare May 28, 2023 20:33

cpuguy83 force-pushed the save_tar_oci branch from 0bc8bde to 1e55ace Compare May 28, 2023 20:35

tianon approved these changes Jun 8, 2023

View reviewed changes

thaJeztah approved these changes Jun 9, 2023

View reviewed changes

thaJeztah added this to the 25.0.0 milestone Jun 9, 2023

thaJeztah added the impact/changelog label Jun 9, 2023

thaJeztah merged commit f139017 into moby:master Jun 9, 2023

neersighted mentioned this pull request Jun 29, 2023

[24.0 backport] c8d/images: handle images without manifests for default platform #45849

Merged

cpuguy83 deleted the save_tar_oci branch June 29, 2023 19:16

thaJeztah mentioned this pull request Nov 7, 2023

c8d: Daemon crashes when using ImageLoad #46746

Closed

vvoland mentioned this pull request Nov 21, 2023

could not find image config wagoodman/dive#452

Open

AkihiroSuda reviewed Jan 12, 2024

View reviewed changes

Chekov2k mentioned this pull request Jan 22, 2024

Docker-ce 25.0.0 breaks image squashing goldmann/docker-squash#236

Closed

tbroyer mentioned this pull request Jan 22, 2024

Unable to analyze the image using dive wagoodman/dive#498

Closed

pkcakeout mentioned this pull request Jan 24, 2024

Importing of containers exported with Docker 25.0.0 fails DIAGNijmegen/rse-grand-challenge#3195

Closed

This was referenced Jan 24, 2024

[rawhide+podman-next] podman push/pull registry roundtrip changes the image containers/podman#20611

Open

Make the "docker-archive" format of "podman save" OCI-compatible following Docker 25 release containers/podman#21347

Open

thaJeztah mentioned this pull request Jan 29, 2024

After update of Docker Desktop (4.27.0), cant scan any local image anymore docker/scout-cli#90

Closed

tixxdz mentioned this pull request Feb 28, 2024

tetragon: tarball build breaks on docker 25.0 cilium/tetragon#2163

Closed

sudo-bmitch mentioned this pull request Apr 2, 2024

[Issue] Image import / push to ECR fails with invalid JSON error regclient/regclient#709

Closed

justinba1010 mentioned this pull request May 21, 2024

Docker Buildx Does Not Seem To Respect Output Type Flag docker/buildx#2469

Closed

natalieparellano mentioned this pull request May 23, 2024

Pack Build Fails with Extensions buildpacks/pack#2164

Closed

romaincabassot mentioned this pull request Jun 7, 2024

Does not work with Docker >= 25? arminc/clair-scanner#139

Open

nightlark mentioned this pull request Aug 20, 2024

update to support docker metadata object llnl/Surfactant#213

Closed

sankantsu mentioned this pull request Nov 27, 2024

[API doc] Image tarball format description of /images/get API seems outdated. #48967

Closed

vvoland mentioned this pull request Apr 16, 2025

docs/api: update image tarball format #49815

Merged

Conversation

cpuguy83 commented Dec 7, 2022

Uh oh!

thaJeztah commented Dec 7, 2022

Uh oh!

AkihiroSuda Dec 7, 2022

Choose a reason for hiding this comment

Uh oh!

cpuguy83 Dec 7, 2022

Choose a reason for hiding this comment

Uh oh!

tianon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cpuguy83 May 24, 2023

Choose a reason for hiding this comment

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thaJeztah May 24, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cpuguy83 commented May 30, 2023

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

AkihiroSuda Jan 12, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants