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: update existing index (index update) #1476

Merged
merged 16 commits into from
Sep 13, 2024

Conversation

wangxiaoxuan273
Copy link
Contributor

@wangxiaoxuan273 wangxiaoxuan273 commented Aug 13, 2024

What this PR does / why we need it:

oras manifest index update

Current status output:

root# oras manifest index update 
wxxdevreg.azurecr.io/test@sha256:2b8bab8c7d73d178fa66c391458e5a441e09ab47bb85c3b4936bd94ac07c9132 --add s390x --add sha256:89fa64341cccaf9b1ede4d06abf5217c785adb6cbb40827f7488db5d97f3d3c8 --remove sha256:a1f5af96aa0a2d2e77bb6f18174353d7fa96700a5fac76a9f460a44f6999330c --merge temp02 --tag temp03 --tag temp04
Fetching  sha256:2b8bab8c7d73d178fa66c391458e5a441e09ab47bb85c3b4936bd94ac07c9132
Fetched   sha256:2b8bab8c7d73d178fa66c391458e5a441e09ab47bb85c3b4936bd94ac07c9132
Fetching  s390x
Fetched   sha256:fd0201b8b0d7cdde2b9e8bcf5278b49668f1dd377a01ec1e2ef0e57382c25a0d s390x
Added     sha256:fd0201b8b0d7cdde2b9e8bcf5278b49668f1dd377a01ec1e2ef0e57382c25a0d s390x
Fetching  sha256:89fa64341cccaf9b1ede4d06abf5217c785adb6cbb40827f7488db5d97f3d3c8
Fetched   sha256:89fa64341cccaf9b1ede4d06abf5217c785adb6cbb40827f7488db5d97f3d3c8
Added     sha256:89fa64341cccaf9b1ede4d06abf5217c785adb6cbb40827f7488db5d97f3d3c8
Fetching  temp02
Fetched   sha256:a4c7b57cea489149bc30e7ecce464ea16ac44ea7dcf734ccfeadd4f6846b175c temp02
Merged    sha256:a4c7b57cea489149bc30e7ecce464ea16ac44ea7dcf734ccfeadd4f6846b175c temp02
Removed   sha256:a1f5af96aa0a2d2e77bb6f18174353d7fa96700a5fac76a9f460a44f6999330c
Updated   sha256:79812a00bb50fc45e56e1744a5201a40364a417bfe444156f3209f3d451b7ecc
Pushed    [registry] wxxdevreg.azurecr.io/test
Tagged temp04
Tagged temp03
Digest: sha256:79812a00bb50fc45e56e1744a5201a40364a417bfe444156f3209f3d451b7ecc

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Part of #1053

This PR builds on #1475

Please check the following list:

  • Does the affected code have corresponding tests, e.g. unit test, E2E test?
  • Does this change require a documentation update?
  • Does this introduce breaking changes that would require an announcement or bumping the major version?
  • Do all new files have an appropriate license header?

Copy link

codecov bot commented Aug 13, 2024

Codecov Report

Attention: Patch coverage is 91.78082% with 12 lines in your changes missing coverage. Please review.

Project coverage is 86.00%. Comparing base (2808ea1) to head (229d4c8).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
cmd/oras/root/manifest/index/update.go 92.42% 5 Missing and 5 partials ⚠️
cmd/oras/root/manifest/index/create.go 81.81% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1476      +/-   ##
==========================================
+ Coverage   85.83%   86.00%   +0.17%     
==========================================
  Files         116      117       +1     
  Lines        4073     4210     +137     
==========================================
+ Hits         3496     3621     +125     
- Misses        345      351       +6     
- Partials      232      238       +6     

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

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.

Please mark oras manifest index command set as "experimental".

@wangxiaoxuan273 wangxiaoxuan273 changed the title feat: add manifests to index (index update --add) feat: add manifests to index (index update) Aug 16, 2024
@wangxiaoxuan273 wangxiaoxuan273 force-pushed the index-update-add branch 2 times, most recently from ef20bdf to c2ecf10 Compare August 20, 2024 10:31
@wangxiaoxuan273 wangxiaoxuan273 changed the title feat: add manifests to index (index update) feat: update existing index (index update) Aug 21, 2024
@wangxiaoxuan273 wangxiaoxuan273 force-pushed the index-update-add branch 3 times, most recently from e939b85 to f20427d Compare August 27, 2024 04:56
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
@qweeah
Copy link
Contributor

qweeah commented Sep 9, 2024

Found two lines in the demo output very misleading:

...
Updated   sha256:79812a00bb50fc45e56e1744a5201a40364a417bfe444156f3209f3d451b7ecc
Pushed    [registry] wxxdevreg.azurecr.io/test@sha256:2b8bab8c7d73d178fa66c391458e5a441e09ab47bb85c3b4936bd94ac07c9132
...

Shouldn't sha256:79812a00bb50fc45e56e1744a5201a40364a417bfe444156f3209f3d451b7ecc be pushed?

@wangxiaoxuan273
Copy link
Contributor Author

Found two lines in the demo output very misleading:

...
Updated   sha256:79812a00bb50fc45e56e1744a5201a40364a417bfe444156f3209f3d451b7ecc
Pushed    [registry] wxxdevreg.azurecr.io/test@sha256:2b8bab8c7d73d178fa66c391458e5a441e09ab47bb85c3b4936bd94ac07c9132
...

Shouldn't sha256:79812a00bb50fc45e56e1744a5201a40364a417bfe444156f3209f3d451b7ecc be pushed?

Thanks for catching!

cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
@wangxiaoxuan273
Copy link
Contributor Author

Found two lines in the demo output very misleading:

...
Updated   sha256:79812a00bb50fc45e56e1744a5201a40364a417bfe444156f3209f3d451b7ecc
Pushed    [registry] wxxdevreg.azurecr.io/test@sha256:2b8bab8c7d73d178fa66c391458e5a441e09ab47bb85c3b4936bd94ac07c9132
...

Shouldn't sha256:79812a00bb50fc45e56e1744a5201a40364a417bfe444156f3209f3d451b7ecc be pushed?

Fixed the push path output by adding and using a new function getPushPath.

cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update_test.go Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
Copy link
Member

@Wwwsylvia Wwwsylvia left a comment

Choose a reason for hiding this comment

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

LGTM with nit

cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
Copy link
Member

@Wwwsylvia Wwwsylvia left a comment

Choose a reason for hiding this comment

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

LGTM

cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
Xiaoxuan Wang and others added 15 commits September 13, 2024 14:31
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: wangxiaoxuan273 <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
Signed-off-by: Xiaoxuan Wang <[email protected]>
cmd/oras/root/manifest/index/update.go Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
cmd/oras/root/manifest/index/update.go Outdated Show resolved Hide resolved
Signed-off-by: Xiaoxuan Wang <[email protected]>
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 071f060 into oras-project:main Sep 13, 2024
8 checks passed
@wangxiaoxuan273 wangxiaoxuan273 deleted the index-update-add branch September 13, 2024 07:51
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.

4 participants