Skip to content

push: inherit distribution sources from parent#9029

Merged
mxpv merged 4 commits intocontainerd:mainfrom
dmcgowan:push-inherit-distribution-sources
Sep 7, 2023
Merged

push: inherit distribution sources from parent#9029
mxpv merged 4 commits intocontainerd:mainfrom
dmcgowan:push-inherit-distribution-sources

Conversation

@dmcgowan
Copy link
Copy Markdown
Member

@dmcgowan dmcgowan commented Aug 30, 2023

When a blob does not exist locally, rather than erroring on info lookup, inherit the parent distribution sources. Push is able to succeed even if the blob does not exist locally when a cross repository mount is done. This is a common operation pushing a multi-platform image to the same registry but different namespace.

This gets around a premature not found error on push when looking up the source labels. The push API was not designed to abort push so early in the process, any not found should only error the process when coming from the content open.

Update the ctr defaults to something more sensible, this reduces the number of user errors on push after pull.

@dmcgowan dmcgowan added cherry-pick/1.6.x cherry-pick/1.7.x Change to be cherry picked to release/1.7 branch labels Aug 30, 2023
@dmcgowan
Copy link
Copy Markdown
Member Author

dmcgowan commented Aug 30, 2023

Marking this a cherry-pick for 1.6 and 1.7, although we could skip cherry-picking the ctr commit. The push change could be considered a bug fix since the current behavior is not correct.

thaJeztah
thaJeztah reviewed Aug 30, 2023
View reviewed changes
Copy link
Copy Markdown
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.

overall looks good; I left a suggestion for the new interface that's designed; happy to hear what you think.

Comment thread remotes/handlers.go Outdated
Comment thread remotes/handlers.go Outdated
@dmcgowan
content: add InfoProvider interface
f8c789f 
Signed-off-by: Derek McGowan <derek@mcg.dev>
@dmcgowan
Update use of content.Infoprovider
24aca53 
Signed-off-by: Derek McGowan <derek@mcg.dev>
@dmcgowan
push: inherit distribution sources from parent
20f7930 
When a blob does not exist locally, rather than erroring on info
lookup, inherit the parent distribution sources. Push is able
to succeed even if the blob does not exist locally when a cross
repository mount is done. This is a common operation pushing a
multi-platform image to the same registry but different namespace.

Signed-off-by: Derek McGowan <derek@mcg.dev>
@dmcgowan
ctr: pull or fetch image metadata by default
b32638e 
The metadata is small and useful for viewing all platforms
for an image and enabling push back to the same registry.

Signed-off-by: Derek McGowan <derek@mcg.dev>
@dmcgowan dmcgowan force-pushed the push-inherit-distribution-sources branch 2 times, most recently from a6acfa4 to b32638e Compare August 31, 2023 18:13
@henry118
Copy link
Copy Markdown
Member

henry118 commented Sep 1, 2023

Could you please help me understand why do we need to introduce the --skip-metadata option and enable AllMetadata by default? Thanks!

@dmcgowan
Copy link
Copy Markdown
Member Author

dmcgowan commented Sep 5, 2023 •
edited
Loading

Could you please help me understand why do we need to introduce the --skip-metadata option and enable AllMetadata by default? Thanks!

It is to make ctr images pull and ctr images push symmetric operations, as most users expect. Even though we don't support ctr as an end user tool, I think it will help with some confusion and provide a better example for other implementations.

@mxpv mxpv merged commit c13f47a into containerd:main Sep 7, 2023
@dmcgowan dmcgowan mentioned this pull request Sep 11, 2023
Merged
@vvoland vvoland mentioned this pull request Oct 20, 2023
Merged
@dmcgowan dmcgowan added cherry-picked/1.7.x PR commits are cherry-picked into release/1.7 branch and removed cherry-pick/1.7.x Change to be cherry picked to release/1.7 branch labels Oct 26, 2023
@samuelkarp
Copy link
Copy Markdown
Member

samuelkarp commented Dec 1, 2023

/cherrypick release/1.6

@k8s-infra-cherrypick-robot
Copy link
Copy Markdown

k8s-infra-cherrypick-robot commented Dec 1, 2023

@samuelkarp: #9029 failed to apply on top of branch "release/1.6":

Applying: content: add InfoProvider interface
Applying: Update use of content.Infoprovider
Using index info to reconstruct a base tree...
A	pkg/display/manifest_printer.go
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): pkg/display/manifest_printer.go deleted in HEAD and modified in Update use of content.Infoprovider. Version Update use of content.Infoprovider of pkg/display/manifest_printer.go left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 Update use of content.Infoprovider
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Details

In response to this:

/cherrypick release/1.6

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@vvoland vvoland mentioned this pull request Dec 1, 2023
Merged
@vvoland
Copy link
Copy Markdown
Contributor

vvoland commented Dec 1, 2023

@samuelkarp opened a manual cherry-pick: #9453

@thaJeztah thaJeztah added cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch and removed cherry-pick/1.6.x labels Dec 1, 2023
@zhenxuanxin zhenxuanxin mentioned this pull request Dec 13, 2023
Closed
Thompson1985

This comment was marked as spam.

Sign in to view

@dmcgowan dmcgowan deleted the push-inherit-distribution-sources branch April 20, 2024 00:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztah thaJeztah thaJeztah left review comments

@mxpv mxpv mxpv approved these changes

@estesp estesp estesp approved these changes

@henry118 henry118 henry118 approved these changes

@vvoland vvoland vvoland approved these changes

+1 more reviewer

@Thompson1985 Thompson1985 Thompson1985 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch cherry-picked/1.7.x PR commits are cherry-picked into release/1.7 branch

Projects

Pull Request Review
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@dmcgowan @henry118 @samuelkarp @k8s-infra-cherrypick-robot @vvoland @mxpv @estesp @thaJeztah @Thompson1985