New image manifest format #18785

aaronlehmann · 2015-12-19T01:04:13Z

This PR adds support for the new manifest format ("schema2") specified in https://github.com/docker/distribution/blob/master/docs/spec/manifest-v2-2.md.

The new format allows end-to-end content addressability. The existing v2 manifest format puts image configurations in "v1Compatibility" strings that use the same data model and non-content-addressable ID scheme that the legacy v1 protocol uses. Supporting this format with the content addressable image/layer model in Docker 1.10 involves hacks to do things like generate fake v1 IDs. The new format carries the actual image configuration as a blob, so push/pull transfers an exact copy of the image.

The schema2 format also includes the concept of a "manifest list". This is a kind of meta-manifest that points to the appropriate manifests for different platforms. It will eventually allow pulling the same image name, tag, or digest across different platforms, and getting a version of the image built for the correct platform. The tools to create and push these manifest lists don't exist yet, but assuming this PR is merged for 1.10, 1.10 will support these multi-architecture images.

The transition to the new manifest format preserves backward compatibility. Engines which support the new format will always try to push schema2 manifests; however, if the remote registry doesn't support the format, it will fall back to using the schema1 format. When pulling, the engine indicates in an Accept header if it can accept the new format. If it can't, and it's pulling an image that the manifest has a schema2 manifest for, the registry will transparently convert the manifest to schema1 format (note this is not supported for pull-by-digest).

One nice side effect of this PR is that the hacks described above for assembling legacy manifests are moved out of the engine code into vendored distribution code. The distribution APIs now have a ManifestBuilder interface that abstracts away the job of creating a manifest in either the old or new format.

Support for the new format will be coming soon on Docker Hub. For now, testing push and pull with schema2 manifests requires building registry from the master branch of github.com/docker/distribution.

thaJeztah · 2015-12-19T18:16:41Z

Tentatively added this to the 1.10 milestone, thanks @aaronlehmann

tonistiigi · 2015-12-22T17:44:07Z

distribution/registry.go

@@ -126,17 +125,7 @@ func NewV2Repository(ctx context.Context, repoInfo *registry.RepositoryInfo, end
 }

 func digestFromManifest(m *schema1.SignedManifest, name reference.Named) (digest.Digest, int, error) {


This helper doesn't seem very useful anymore. At least, there is no need to have name as input and err as output.

dmcgowan · 2015-12-22T23:42:26Z

design +1, this has already been approved and implemented on the distribution side.

icecrime · 2015-12-23T00:35:48Z

Design LGTM

aaronlehmann · 2015-12-24T00:03:47Z

I've brought this up to date by rebasing it on top of #18889. When #18889 is merged, the rebase will just involve dropping the first commit.

thaJeztah · 2016-01-06T00:35:40Z

ping @aaronlehmann #18889 was merged, so this needs to be rebased

aaronlehmann · 2016-01-06T18:39:25Z

Rebased

dmcgowan · 2016-01-06T21:37:06Z

Dockerfile

+# Install two versions of the registry. The first is an older version that
+# only supports schema1 manifests. The second is a newer version that supports
+# both. This allows integration-cli tests to cover push/pull with both schema1
+# and schema1 manifests.


and schema2 manifests

aaronlehmann · 2016-01-08T01:36:19Z

This is no longer WIP. The distribution bits have been merged upstream and the version vendored here is off the master branch.

thaJeztah · 2016-01-08T15:49:32Z

ping @tonistiigi could you review this?

tonistiigi · 2016-01-08T21:34:37Z

image/rootfs_windows.go

@@ -27,7 +27,7 @@ func (r *RootFS) BaseLayerID() string {

 // ChainID returns the ChainID for the top layer in RootFS.
 func (r *RootFS) ChainID() layer.ChainID {
-	baseDiffID, _ := digest.FromBytes([]byte(r.BaseLayerID())) // can never error
+	baseDiffID := digest.FromBytes([]byte(r.BaseLayerID())) // can never error


Remove comment

icecrime · 2016-01-10T04:22:32Z

distribution/push_v2.go

 }

 // blobSumAlreadyExists checks if the registry already know about any of the
 // blobsums passed in the "blobsums" slice. If it finds one that the registry
 // knows about, it returns the known digest and "true".
-func blobSumAlreadyExists(ctx context.Context, blobsums []digest.Digest, repo distribution.Repository, layersPushed *pushMap) (digest.Digest, bool, error) {
-	layersPushed.Lock()
-	for _, dgst := range blobsums {


Sorry, why is this block gone?

Moved to Upload, line 233.

I see, thanks!

icecrime · 2016-01-10T18:43:07Z

LGTM

vdemeester · 2016-01-10T18:56:49Z

LGTM 🐹

icecrime · 2016-01-10T19:48:24Z

I don't think there is any docs impact there: merging \o/

thaJeztah · 2016-01-10T19:49:35Z

@aaronlehmann I don't think this has user-facing changes? So docs should be good, but please let us know if we overlooked something there, then we can do a docs follow up

icecrime · 2016-01-10T20:14:00Z

Mmmm Windows CI is broken again 😢 Ping @jfrazelle @mikedougherty @jhowardmsft: can we merge anyway or do you want to give it look?

lowenna · 2016-01-11T16:45:03Z

Looking now and restarting failing nodes...

lowenna · 2016-01-11T17:56:15Z

@mikedougherty @jfrazelle

This seems a different error to other Windows CI failures. It's consistently hitting this:

09:53:43 + git clone https://github.com/BurntSushi/toml.git /tmp/tmp.7KvnKpADjP/src/github.com/BurntSushi/toml
09:53:43 Cloning into '/tmp/tmp.7KvnKpADjP/src/github.com/BurntSushi/toml'...
09:53:43 Removing intermediate container 63e2e9e2b088
09:53:43 SECURITY WARNING: You are building a Docker image from Windows against a non-Windows Docker host. All files and directories added to build context will have '-rwxr-xr-x' permissions. It is recommended to double check and reset permissions for sensitive files and directories.
09:53:43 [8] System error: read parent: connection reset by peer

This is happening on multiple nodes, which are working with other PRs. Hence I think it's something in this PR rather than CI itself. Can you guys see anything obvious that I'm missing?

aaronlehmann · 2016-01-11T18:03:07Z

This PR changes the Dockerfile, while most do not. It seems that something goes wrong while doing the initial docker build. However, it fails in a step that's well past my change. I suspect that if you clear the build cache, you would see the same problem with other PRs.

BTW, @tonistiigi has tested this PR manually on Windows.

lowenna · 2016-01-11T19:06:42Z

Re-started on node 1 which I've just cleaned out the build cache on.

aaronlehmann · 2016-01-11T19:58:35Z

Suceessful windows run: https://jenkins.dockerproject.org/job/Windows-PRs/20105/console

New image manifest format

Concurrent uploads which share layers worked correctly as of moby#18353, but unfortunately moby#18785 caused a regression. This PR removed the logic that shares digests between different push sessions. This overlooked the case where one session was waiting for another session to upload a layer. This commit adds back the ability to propagate this digest information, using the distribution.Descriptor type because this is what is received from stats and uploads, and also what is ultimately needed for building the manifest. Surprisingly, there was no test covering this case. This commit adds one. It fails without the fix. See recent comments on moby#9132. Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com>

Concurrent uploads which share layers worked correctly as of moby#18353, but unfortunately moby#18785 caused a regression. This PR removed the logic that shares digests between different push sessions. This overlooked the case where one session was waiting for another session to upload a layer. This commit adds back the ability to propagate this digest information, using the distribution.Descriptor type because this is what is received from stats and uploads, and also what is ultimately needed for building the manifest. Surprisingly, there was no test covering this case. This commit adds one. It fails without the fix. See recent comments on moby#9132. Signed-off-by: Aaron Lehmann <aaron.lehmann@docker.com> (cherry picked from commit 5c99eeb)

GordonTheTurtle added area/distribution status/0-triage labels Dec 19, 2015

thaJeztah added this to the 1.10 milestone Dec 19, 2015

thaJeztah added status/1-design-review and removed status/0-triage labels Dec 19, 2015

tonistiigi reviewed Dec 22, 2015
View reviewed changes

aaronlehmann force-pushed the new-manifest branch 2 times, most recently from fa99803 to 170e191 Compare December 22, 2015 23:25

icecrime added status/2-code-review and removed status/1-design-review labels Dec 23, 2015

aaronlehmann force-pushed the new-manifest branch from 170e191 to 6d83ad7 Compare December 24, 2015 00:02

thaJeztah mentioned this pull request Dec 24, 2015

Add verification to import [closes #10385] #18896

Closed

BrianBland mentioned this pull request Jan 5, 2016

Cross repository push #19109

Merged

aaronlehmann force-pushed the new-manifest branch from 6d83ad7 to f9a1e6b Compare January 6, 2016 18:39

dmcgowan reviewed Jan 6, 2016
View reviewed changes

aaronlehmann force-pushed the new-manifest branch 2 times, most recently from 11fa69e to bde2e44 Compare January 8, 2016 01:10

aaronlehmann changed the title ~~New image manifest format - WIP~~ New image manifest format Jan 8, 2016

aaronlehmann force-pushed the new-manifest branch from bde2e44 to cc5167b Compare January 8, 2016 01:35

tonistiigi reviewed Jan 8, 2016
View reviewed changes

icecrime reviewed Jan 10, 2016
View reviewed changes

icecrime added status/3-docs-review and removed status/2-code-review labels Jan 10, 2016

icecrime added status/4-merge impact/changelog impact/distribution and removed status/3-docs-review labels Jan 10, 2016

icecrime pushed a commit that referenced this pull request Jan 11, 2016

Merge pull request #18785 from aaronlehmann/new-manifest

f11b6a2

New image manifest format

icecrime merged commit f11b6a2 into moby:master Jan 11, 2016

aaronlehmann deleted the new-manifest branch January 11, 2016 20:02

aaronlehmann mentioned this pull request Mar 1, 2016

Fix concurrent uploads that share layers #20831

Merged

duyanghao mentioned this pull request Sep 21, 2016

Same image ID with different digests #22225

Closed

andyxning mentioned this pull request Sep 9, 2017

Making targets for multi-arch kubernetes/kube-state-metrics#252

Merged

mkumatag mentioned this pull request Jan 31, 2018

Use pause manifest image kubernetes/kubernetes#59056

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

New image manifest format #18785

New image manifest format #18785

aaronlehmann commented Dec 19, 2015

thaJeztah commented Dec 19, 2015

tonistiigi Dec 22, 2015

dmcgowan commented Dec 22, 2015

icecrime commented Dec 23, 2015

aaronlehmann commented Dec 24, 2015

thaJeztah commented Jan 6, 2016

aaronlehmann commented Jan 6, 2016

dmcgowan Jan 6, 2016

aaronlehmann commented Jan 8, 2016

thaJeztah commented Jan 8, 2016

tonistiigi Jan 8, 2016

icecrime Jan 10, 2016

aaronlehmann Jan 10, 2016

icecrime Jan 10, 2016

icecrime commented Jan 10, 2016

vdemeester commented Jan 10, 2016

icecrime commented Jan 10, 2016

thaJeztah commented Jan 10, 2016

icecrime commented Jan 10, 2016

lowenna commented Jan 11, 2016

lowenna commented Jan 11, 2016

aaronlehmann commented Jan 11, 2016

lowenna commented Jan 11, 2016

aaronlehmann commented Jan 11, 2016

		@@ -126,17 +125,7 @@ func NewV2Repository(ctx context.Context, repoInfo *registry.RepositoryInfo, end
		}

		func digestFromManifest(m *schema1.SignedManifest, name reference.Named) (digest.Digest, int, error) {

New image manifest format #18785

New image manifest format #18785

Conversation

aaronlehmann commented Dec 19, 2015

thaJeztah commented Dec 19, 2015

tonistiigi Dec 22, 2015

Choose a reason for hiding this comment

dmcgowan commented Dec 22, 2015

icecrime commented Dec 23, 2015

aaronlehmann commented Dec 24, 2015

thaJeztah commented Jan 6, 2016

aaronlehmann commented Jan 6, 2016

dmcgowan Jan 6, 2016

Choose a reason for hiding this comment

aaronlehmann commented Jan 8, 2016

thaJeztah commented Jan 8, 2016

tonistiigi Jan 8, 2016

Choose a reason for hiding this comment

icecrime Jan 10, 2016

Choose a reason for hiding this comment

aaronlehmann Jan 10, 2016

Choose a reason for hiding this comment

icecrime Jan 10, 2016

Choose a reason for hiding this comment

icecrime commented Jan 10, 2016

vdemeester commented Jan 10, 2016

icecrime commented Jan 10, 2016

thaJeztah commented Jan 10, 2016

icecrime commented Jan 10, 2016

lowenna commented Jan 11, 2016

lowenna commented Jan 11, 2016

aaronlehmann commented Jan 11, 2016

lowenna commented Jan 11, 2016

aaronlehmann commented Jan 11, 2016