8000 c8d/push: Fix fallback single-manifest push not creating a tag by vvoland · Pull Request #50199 · moby/moby · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

c8d/push: Fix fallback single-manifest push not creating a tag #50199

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

Merged
merged 2 commits into from
Jun 13, 2025

Conversation

vvoland
Copy link
Contributor
@vvoland vvoland commented Jun 13, 2025

After pushing the multi-platform index fails due to missing content, we retry with the single-platform manifest.

While the target descriptor was changed for the second push, the actual target digested reference still pointed to the original multi-platform index.

Obviously, with the fallback that didn't really work correctly, because the multi-platform index is not pushed.

This commit fixes the issue by updating the target reference to point to the single-platform manifest.

(I split it into 2 commits to make it easier to see what the fix exactly was)

containerd image store: Fix `docker push`
8000
 not creating a tag on the remote repository.

After pushing the multi-platform index fails due to missing content, we
retry with the single-platform manifest. While the target descriptor was
changed for the second push, the actual target digested reference still
pointed to the original multi-platform index. Obviously, with the
fallback that didn't really work correctly, because the multi-platform
index is not pushed.

This commit fixes the issue by updating the target reference to point to
the single-platform manifest.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland vvoland added this to the 28.3.0 milestone Jun 13, 2025
@vvoland vvoland self-assigned this Jun 13, 2025
@vvoland vvoland added area/distribution kind/bugfix PR's that fix bugs containerd-integration Issues and PRs related to containerd integration labels Jun 13, 2025
@@ -186,6 +186,19 @@ func (i *ImageService) pushRef(ctx context.Context, targetRef reference.Named, p
orgTarget := target
target = newTarget
pp.TurnNotStartedIntoUnavailable()

// Annotate ref with digest to push only push tag for single digest
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some grammar issue in the comment 😅

thaJeztah
thaJeztah previously approved these changes Jun 13, 2025
Copy link
Member
@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

just a nit about the comment, but looks like the other place also had that

@thaJeztah thaJeztah dismissed their stale review June 13, 2025 12:47

See comment

Just a refactor of the previous commit to reduce duplication.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
@vvoland vvoland force-pushed the c8d-push-fixtag branch from 4f327fe to 987b8a8 Compare 8000 June 13, 2025 12:51
Copy link
Member
@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vvoland vvoland merged commit 9a9cade into moby:master Jun 13, 2025
172 checks passed
@nwalters512
Copy link
nwalters512 commented Jun 24, 2025

👋 any estimate on when this fix will make it into a Docker Desktop for Mac release? This is causing issues with pushing images to AWS ECR (and presumably any registry): aws/containers-roadmap#2627

For anyone else coming across this, adding an explicit --platform ... flag when pushing did fix things.

@vvoland
Copy link
Contributor Author
vvoland commented Jun 25, 2025

Hi, it will be available in the next Docker Desktop release (4.43) which should be released in the first week of July.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/distribution containerd-integration Issues and PRs related to containerd integration impact/changelog kind/bugfix PR's that fix bugs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

'docker push' doesn't push tag when using containerd snapshotter
3 participants
0