-
Notifications
You must be signed in to change notification settings - Fork 18.8k
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
Conversation
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>
daemon/containerd/image_push.go
Outdated
@@ -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 |
There was a problem hiding this comment.
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 😅
There was a problem hiding this 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
Just a refactor of the previous commit to reduce duplication. Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
4f327fe
to
987b8a8
Compare
8000
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
👋 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 |
Hi, it will be available in the next Docker Desktop release (4.43) which should be released in the first week of July. |
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)