@thaJeztah I tried to do that, but distribution code also depends on libtrust. I believe the dependency can be completely removed with containerd integration.

Codecov Report

❗ No coverage uploaded for pull request base (master@a30990b). Click here to learn what that means.
The diff coverage is 6.89%.

@@            Coverage Diff            @@
##             master   #39384   +/-   ##
=========================================
  Coverage          ?   37.32%           
=========================================
  Files             ?      609           
  Lines             ?    45177           
  Branches          ?        0           
=========================================
  Hits              ?    16863           
  Misses            ?    26032           
  Partials          ?     2282

Updated

Probably there are people who are still using an older version of Quay w/o support for schema2

ref: moby/buildkit#409

Hi @tiborvass, TL of Quay here

If manifest V2_1 was deprecated in a PR only 24 days ago, why is the push code being removed so quickly afterwards?

Ignoring the (long but interesting) reasons as to why Quay.io hasn't fully enabled support for V2_2 yet (which I'll be discussing in a talk publicly sometime), this fast of a removal seems counter to standard enterprise deprecation guidelines.

Speaking only for myself, I think it would be better to keep V2_1 push for the time being and emit a warning message indicating that an older manifest version is being used and that it will be removed in a future release. In fact, this is what was done when Docker moved from protocol V1 to protocol V2, which allowed users to continue using older versions of registries, but with knowledge that they should upgrade soon.

Hi @josephschorr!

I believe we're on the same page.

A bit of context though: the short-lived v2schema1 was deprecated 3+ years ago in favor of v2schema2 and although we did not have a timeline for complete removal, the intention was clear that v2schema2 deprecates v2schema1. If I'm not mistaken Quay was involved in discussions to migrate to v2schema2 for a long time, and I'm glad to see it happening in the beta program.

When #37874 was merged, it not only removed dead code from v1 but also code from v2schema1 which is mainly used by Quay today and would have broken our users who rely on Quay.

We caught that in time so we reverted the v2schema1 removal in #39365 to prevent breaking our users, while making sure this time we properly and officially deprecate v2schema1 despite the fact that all players know they should be using v2schema2. That PR does exactly what you describe, adding a warning that v2 schema1 will be removed in a future release: https://github.com/moby/moby/pull/39365/files#diff-8448734ad4bcad711a430d27d303ac16R161

We cherry-picked that into the next release of Docker as well.

What may also cause confusion is that the master branch of moby went back and forth by removing v2schema1, reverting+warning, and this PR that's reverting the revert. I agree that doing the revert+warning in Docker's release branch would have been simpler. The end goal is for the next release to be the last release with v2schema1 support and have a deprecation warning, but the release after that will have v2schema1 removed. And because that will be based off of moby's current master branch, that is why it is being removed in this PR on moby master.

I strongly hope that Quay's beta program that migrates to v2schema2 is a sign that the timelines for removing v2schema1 will align for both of us.

Hi @tiborvass,

although we did not have a timeline for complete removal, the intention was clear that v2schema2 deprecates v2schema1

Definitely, although the lack of communication to end-users presented issues for them to understand why older registry versions no longer work, as the "manifest invalid" error is quite cryptic to those who don't know about the change. I'm glad this is being fixed now! :)

What may also cause confusion is that the master branch of moby went back and forth by removing v2schema1, reverting+warning, and this PR that's reverting the revert

Yeah, this is definitely what confused me and I apologize if I came off as rude in my previous message as a result; I was under the (mistaken) impression that this PR was going to once again break users on older installations without warning.

I strongly hope that Quay's beta program that migrates to v2schema2 is a sign that the timelines for removing v2schema1 will align for both of us.

So, the code is released to our on premises customers and they are using it without issue (AFAIK). Quay.io's delay in full release has, unfortunately, to do with an unrelated issue we've been addressing, but once that issue is fully addressed, we'll be completing the rollout to all our users. I'm hopeful this is no more than a few weeks away, but as we have to ensure performance and stability at all costs, its possible we find other blockers as we go along.

On a personal note, I do highly appreciate this change - its making the delays on our side much less stressful, so thank you! :)

Hi @tiborvass,

We have customers that have pulled the code with this warning and are now seeing it when pulling manifest V2_1 images, which I believe is a mistake; it was my understanding that the V2_1 code being removed in the future was only for push, and that support would remain for pull to support older images that were pushed to registries years ago.

Any ideas if we could get the message changed to only apply on push?

@tiborvass Ping! Any updates on this?

@josephschorr opened #39736

@tiborvass Thanks!

Just chatted with @dmcgowan and the changes in this PR are desired for going into PR #38738

Did a quick rebase and fixed the gotest.tools import paths to use v3

One more thing to fix looks like;

ERRO Running error: buildssa: analysis skipped: errors in package: [/go/src/github.com/docker/docker/integration-cli/docker_cli_daemon_test.go:21:2: "runtime" imported but not used] 

Seeing a number of failures having tag invalid;

=== RUN   TestDockerSchema1RegistrySuite/TestPullImageWithAliases
    --- FAIL: TestDockerSchema1RegistrySuite/TestPullImageWithAliases (0.25s)
        cli.go:29: assertion failed: 
            Command:  /usr/local/cli/docker push 127.0.0.1:5000/dockercli/busybox:recent
            ExitCode: 1
            Error:    exit status 1
            Stdout:   The push refers to repository [127.0.0.1:5000/dockercli/busybox]
            1be74353c3d0: Preparing
            1be74353c3d0: Pushed
            
            Stderr:   tag invalid
            
            
            Failures:
            ExitCode was 1 expected 0
            Expected no error
        check_test.go:223: [d152d09ddf8a9] daemon is not started

Also need to look at #41095 to see if we can remove that

Looks like it's because we push to a v1 registry to verify that pulling still works;

Discussing what options we have with @tonistiigi - we could store the registry state in an image and pull that so that we don't have to push to those registries first (or perhaps prepare an image with the registry + data); need to think of options we have there, and what would work best

closing this for now; a rebased version of this is in #42300, but closed for now; #42300 (comment)

Discussing this in the maintainers meeting, and there are still concerns about deprecating this before containerd and others deprecate it, so the request is to instead create a PR to disable it by default, and make support for this opt-in.

Closing this for now