Skip to content
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

feat(oci-layout): support in oras cp #748

Merged
merged 32 commits into from
Jan 17, 2023
Merged

Conversation

qweeah
Copy link
Contributor

@qweeah qweeah commented Jan 13, 2023

Related to #378

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@codecov-commenter
Copy link

codecov-commenter commented Jan 13, 2023

Codecov Report

Attention: Patch coverage is 23.33333% with 69 lines in your changes missing coverage. Please review.

Project coverage is 63.96%. Comparing base (9013115) to head (440731e).
Report is 445 commits behind head on main.

Files with missing lines Patch % Lines
cmd/oras/internal/option/target.go 19.76% 69 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #748      +/-   ##
==========================================
- Coverage   70.12%   63.96%   -6.17%     
==========================================
  Files          18       19       +1     
  Lines         616      702      +86     
==========================================
+ Hits          432      449      +17     
- Misses        153      222      +69     
  Partials       31       31              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@shizhMSFT shizhMSFT requested review from shizhMSFT and FeynmanZhou and removed request for shizhMSFT January 13, 2023 06:05
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target_test.go Outdated Show resolved Hide resolved
cmd/oras/internal/display/print.go Outdated Show resolved Hide resolved
cmd/oras/cp.go Outdated Show resolved Hide resolved
cmd/oras/cp.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
cmd/oras/cp.go Outdated

Example - Copy the artifact tagged with 'v1' from repository 'localhost:5000/net-monitor' to 'localhost:5000/net-monitor-copy' with multiple tags and concurrency level tuned
oras cp --concurrency 6 localhost:5000/net-monitor:v1 localhost:5000/net-monitor-copy:v1,tag2,tag3
Example - Copy the artifacts:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@FeynmanZhou I have grouped examples by features, can you help review, thanks!

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
cmd/oras/cp.go Outdated
Comment on lines 52 to 73
Example - Copy the artifact tagged 'v1':
oras cp localhost:5000/net-monitor:v1 localhost:5000/net-monitor-copy:v1 # copy between repositories
oras cp localhost:5000/net-monitor:v1 localhost:5000/net-monitor-copy # copy without tagging in the destination
oras cp --to-oci localhost:5000/net-monitor:v1 test:v1 # download into an OCI layout folder 'test'
oras cp --from-oci test:v1 localhost:5000/net-monitor:v1 # upload from an OCI layout folder 'test'
oras cp --from-oci test.tar:v1 localhost:5000/net-monitor:v1 # upload from an OCI layout tar archive 'test.tar'

Example - Copy the artifact tagged 'v1' and its referrers:
oras cp -r localhost:5000/net-monitor:v1 localhost:5000/net-monitor-copy:v1 # copy between repositories
oras cp -r --to-oci localhost:5000/net-monitor:v1 test:v1 # download into an OCI image layout folder 'test'
oras cp -r --from-oci test:v1 localhost:5000/net-monitor:v1 # upload from an OCI image layout folder 'test'

Example - Copy certain platform of the artifact 'v1':
oras cp --platform linux/arm/v5 localhost:5000/net-monitor:v1 localhost:5000/net-monitor-copy:v1 # copy between repositories
oras cp --platform linux/arm/v5 --to-oci localhost:5000/net-monitor:v1 test:v1 # download into an OCI layout folder 'test'
oras cp --platform linux/arm/v5 --from-oci test:v1 localhost:5000/net-monitor:v1 # upload from an OCI layout folder 'test'

Example - Copy the artifact 'v1' with multiple tags:
oras cp localhost:5000/net-monitor:v1 localhost:5000/net-monitor-copy:tag1,tag2,tag3 # copy between repositories
oras cp --concurrency 10 localhost:5000/net-monitor:v1 localhost:5000/net-monitor-copy:tag1,tag2,tag3 # copy between repositories with concurrency level tuned
oras cp localhost:5000/net-monitor:v1 test:tag1,tag2,tag3 --to-oci # download into an OCI layout folder 'test'
oras cp test:v1 localhost:5000/net-monitor-copy:tag1,tag2,tag3 --from-oci # upload from an OCI layout folder 'test'
Copy link
Contributor

@shizhMSFT shizhMSFT Jan 13, 2023

Choose a reason for hiding this comment

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

Those examples will be wrapped by terminals, rendering hard to read for users.

@FeynmanZhou we might need to have a better way to present the examples.

cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/cp.go Outdated Show resolved Hide resolved
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
cmd/oras/internal/option/target.go Show resolved Hide resolved
cmd/oras/cp.go Outdated
oras cp --concurrency 6 localhost:5000/net-monitor:v1 localhost:5000/net-monitor-copy:v1,tag2,tag3
Example - Copy the artifact tagged 'v1':
oras cp localhost:5000/net-monitor:v1 localhost:5000/net-monitor-copy:v1 # copy between repositories
oras cp localhost:5000/net-monitor:v1 localhost:5000/net-monitor-copy # copy without tagging in the destination
Copy link
Member

Choose a reason for hiding this comment

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

I think from a customers perspective --to-oci would be to an oci registry. I feel like it would be more intuitive to say --to-file or a little worse to me --to-directory

Copy link
Member

Choose a reason for hiding this comment

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

Thinking about it more, I like the --to=format and --from=format better because that would leave the possibility of other formats other than registry and directory. Having a TGZ format would be very handy for example. Having an input of stdin or stndout might be convenient as well.

Copy link
Contributor Author

@qweeah qweeah Jan 15, 2023

Choose a reason for hiding this comment

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

The oci image layout is not a format but a layout, see spec

So for short flags, how about --from-layout, --to-layout for oras cp and --layout to other commands?

--layout is equivalent to --layout type=oci. To support more layouts in the future, we can add --layout type=docker (just taking docker image layout as an example, doesn't mean we need to support it)

Copy link
Contributor Author

@qweeah qweeah Jan 15, 2023

Choose a reason for hiding this comment

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

Can you elaborate on the scenarios of using an input of stdin? To me the UX sounds looks like below:

some-cmd | oras cp - localhost/owner/name:v1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also for uploading .tgz image layout, I might take a closer look tomorrow to see if it's already supported or what we need to do to support it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Having an input of stdin or stndout might be convenient as well.

Currently, oras / oras-go requires random access for read and write for performance considerations, and thus stdin and stdout are not supported. Maybe we can consider that after oras v1.0.0, trading convenience off performance.

Copy link
Member

Choose a reason for hiding this comment

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

I don't want to get too side tracked by all the possible formats and destinations, but also don't want to design into a corner. It definitely might be convenient to support stdin/stdout for example, but maybe that should be done in another command. The tgz thing also might make more sense for another command since tgz input/output make sense for multiple repositories.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It definitely might be convenient to support stdin/stdout for example, but maybe that should be done in another command. The tgz thing also might make more sense for another command since tgz input/output make sense for multiple repositories.

@TerryHowe Can you help create enhancement issues with detailed scenario for those? We can continue discussion there.

Copy link
Member

@FeynmanZhou FeynmanZhou Jan 16, 2023

Choose a reason for hiding this comment

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

I think from a customers perspective --to-oci would be to an oci registry. I feel like it would be more intuitive to say --to-file or a little worse to me --to-directory.

@TerryHowe To avoid ambiguity, how about using --to-oci-layout and --from-oci-layout in oras copy respectively?

Copy link
Member

Choose a reason for hiding this comment

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

cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Show resolved Hide resolved
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
cmd/oras/cp.go Outdated Show resolved Hide resolved
cmd/oras/cp.go Outdated Show resolved Hide resolved
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
cmd/oras/internal/fileref/unix.go Outdated Show resolved Hide resolved
cmd/oras/internal/fileref/windows_test.go Show resolved Hide resolved
cmd/oras/internal/fileref/windows.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Outdated Show resolved Hide resolved
cmd/oras/internal/option/target.go Show resolved Hide resolved
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah qweeah changed the title feat: support OCI image layout in oras cp feat(oci-layout): support in oras cp Jan 17, 2023
Copy link
Contributor

@shizhMSFT shizhMSFT left a comment

Choose a reason for hiding this comment

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

LGTM

@shizhMSFT shizhMSFT merged commit 6447e8e into oras-project:main Jan 17, 2023
TerryHowe pushed a commit to TerryHowe/oras that referenced this pull request Feb 2, 2023
Related to oras-project#378

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants