-
Notifications
You must be signed in to change notification settings - Fork 18.6k
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.
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
Remove support for pulling v2 schema1 #42300
base: master
Are you sure you want to change the base?
Remove support for pulling v2 schema1 #42300
Conversation
2a95a46
to
09e5b4d
Compare
Looks like we have one test depending on an ancient image;
Image is literally just an
|
I just recalled that I already looked at that test / image at some point; #34832. Let me give it another go Original image was built from https://github.com/appropriate/docker-nc/tree/master/latest |
09e5b4d
to
4078112
Compare
4078112
to
63aab4c
Compare
This comment has been minimized.
This comment has been minimized.
8db2387
to
531034e
Compare
Whoop; CI is green now, so that's a start |
We discussed this PR in the maintainers meeting. In general, everyone seemed to agree that (permitted there's no large use of these images) it should be ok to remove this. The provisional schema 2 v1 format became deprecated when v2 was introduced in 2015 (distribution/distribution#1068). Originally, support in Docker was meant to be removed in 2018 (#37874), but extended for some time to allow users and registries to migrate (#39365 / docker-archive#284 / docker/cli#1956). We briefly discussed an issue we encountered some time ago where (due to some bug on Docker Hub?) images were pushed (or converted on pull?) as schema 2 v1 images, but @tianon mentioned that that issue was resolved, and hasn't been seen for quite a while, so probably not a concern. Some other concerns were raised by @tonistiigi about the actual removal:
For the above, it was suggested if we could synchronize this with containerd (both deprecating and removing v2 schema 1). @cpuguy83 started a discussion with the containerd maintainers (copying from the slack conversation);
From @dmcgowan:
From @fuweid:
So questions from the above:
@justincormack ^^ perhaps you have some input on this? Let me also /cc
|
@vrothberg @mtrmac PTAL |
|
I agree, I think it should be fine; we've deprecated it long time ago now, and have been delaying removal multiple times. Let me also check with the containerd maintainers if they would be ok with deprecating (not removing) this. I think it's good to warn users early, to raise awareness of this deprecated version of the spec. (cc @dmcgowan @dims @fuweid)
Thanks @giuseppe! I wasn't sure who was the right person to ping 😅 |
Thank you for pulling us in.
Totally agree 😄 I will reach out to Quay and ask if they have stats on schema 1 images. I am always happy to add things but am terrified of removals in fear of breaking users. |
WRT c/image, it’s quite true that the conversion from schema1 (having to compute diffID values) is a hassle and it’s a bit attractive to drop that code, but the code already exists, and the users affected by these changes need some path forward, so I’m inclined to keep the code (of the generic copy pipeline, at least) as is, including the current schema1 support. (New transports might easily choose not to support schema1, and the generic copy pipeline’s conversion implementation would deal with that.) Registries are already turning off schema1 (by default?), which seems sufficient to push the ecosystem forward. It might be worth removing schema1 from the default list of formats to try converting to… My primary wish in this transition is to have distribution/distribution#2925 , to allow clients to report useful error messages. Just dropping the format support from the clients entirely is a way to make that moot, I suppose. |
… and schema1-only on-premise server deployments? The public quay.io transition was relatively recent, I have no idea what that means about the transition schedule of private instances. Private, internet-unknown registries that are unlikely to upgrade are my primary worry WRT the client changes. |
81a06bb
to
08ecf2a
Compare
e7fa171
to
8c77d9a
Compare
8c77d9a
to
cc546af
Compare
Change looks good, what is remaining to get this in? |
cc546af
to
85248a7
Compare
bd67369
to
395a769
Compare
a3ef07d
to
b0e7d8c
Compare
b0e7d8c
to
6f719ac
Compare
Signed-off-by: Sebastiaan van Stijn <[email protected]>
Manifest v2 schema1 was deprecated in 4866f5139a1 and this commit removes the push code for v2 schema1. This reverts commit f695e98, adjusted for changes that were made since daemon: do not mkdir trust directory Remove push tests and move UUID tests to integration Partial revert of f23a51a. Only the schema1 push tests are removed but the schema1 pull tests are still desired. The UUID test is moved from integration-cli to integration. Signed-off-by: Tibor Vass <[email protected]> Signed-off-by: Sebastiaan van Stijn <[email protected]>
Also remove the DOCKER_ALLOW_SCHEMA1_PUSH_DONOTUSE from Jenkins Signed-off-by: Sebastiaan van Stijn <[email protected]>
This image format is only used for docker save / docker load. Signed-off-by: Sebastiaan van Stijn <[email protected]>
This format-check was only used for v1 images. Signed-off-by: Sebastiaan van Stijn <[email protected]>
6f719ac
to
8591fed
Compare
Last commit is a quick push / reminder of what needs to be done:
WIP: remove DOCKER_ALLOW_SCHEMA1_PUSH_DONOTUSE hack
This PR will still fail, as we need to find a different solution to provision a "v2 schema1" registry to test pulling these manifests.
Some solutions;
Manifest v2 schema1 was deprecated in 4866f51 (#39365) and this PR
removes the push code for v2 schema1.
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)