Skip to content

feat: add flag to skip deleting obsolete referrers index#957

Merged
qweeah merged 48 commits into
oras-project:mainfrom
qweeah:gc-referrers
Jul 3, 2023
Merged

feat: add flag to skip deleting obsolete referrers index#957
qweeah merged 48 commits into
oras-project:mainfrom
qweeah:gc-referrers

Conversation

@qweeah

@qweeah qweeah commented May 29, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:
This PR adds a flag to oras attach, oras cp and oras manifest push. The flag can be used to specify whether to clean up old referrers index when referrers API is not available.

There is not a breaking change and by default, mentioned commands will still do referrers cleanup.

Depends on merging of #967.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #954, fixes #952, fixes #915

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?

@codecov-commenter

codecov-commenter commented May 29, 2023 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #957 (757f2ae) into main (f9aec91) will increase coverage by 0.30%.
The diff coverage is 85.71%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@            Coverage Diff             @@
##             main     #957      +/-   ##
==========================================
+ Coverage   81.08%   81.39%   +0.30%     
==========================================
  Files          53       54       +1     
  Lines        2792     2816      +24     
==========================================
+ Hits         2264     2292      +28     
+ Misses        358      355       -3     
+ Partials      170      169       -1
Impacted Files Coverage Δ
cmd/oras/internal/option/referrers.go 55.55% <55.55%> (ø)
cmd/oras/internal/errors/errors.go 100.00% <100.00%> (ø)
cmd/oras/root/attach.go 85.48% <100.00%> (+2.98%) ⬆️
cmd/oras/root/cp.go 86.86% <100.00%> (+0.49%) ⬆️
cmd/oras/root/manifest/push.go 77.27% <100.00%> (+4.40%) ⬆️

@qweeah qweeah marked this pull request as ready for review June 1, 2023 00:33
Wwwsylvia
Wwwsylvia reviewed Jun 2, 2023
View reviewed changes
Comment thread cmd/oras/internal/option/referrers.go
@qweeah qweeah changed the title feat!: add flag for referrers garbage collection feat!: add flag to skip deleting obsolete referrers index Jun 2, 2023
qweeah added 15 commits June 6, 2023 01:32
@qweeah
feat!: add flag for referrers GC
8b22600 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
update go mod
7b7e5a4 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
fix e2e
e27d723 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
add e2e
57b4e75 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
update deps
adfcca1 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
update dep
cacd6bb 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
update
f0ecebf 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
update version
ffbd8bc 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
mod tidy
d5f1937 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
fix e2e
abae524 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
rename flag
06d382d 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
fix: default to true
86f3b1d 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
fix e2e
19c4516 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
change default value to false
623dc99 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
fix e2e
aa73bc9 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah
fix e2e
aa91493 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@qweeah qweeah changed the title feat!: add flag to skip deleting obsolete referrers index feat: add flag to skip deleting obsolete referrers index Jun 6, 2023
@qweeah
refactor: simplify e2e scripts and github action
06af37b 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@shizhMSFT shizhMSFT added this to the v1.1.0 milestone Jun 28, 2023
@qweeah qweeah mentioned this pull request Jun 28, 2023
Closed
1 task
shizhMSFT
shizhMSFT reviewed Jun 29, 2023
View reviewed changes

@shizhMSFT shizhMSFT left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What about the manifest delete path?

Comment thread cmd/oras/internal/option/referrers.go Outdated
@qweeah

qweeah commented Jun 29, 2023 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

What about the manifest delete path?

Referrers index deletion happens after manifest deletion right? If the registry already supports deletion and the token used covers manifest deletion, why would referrers index fail?

Also why would user want to skip referrers index deletion when manifest deletion is successful?

@shizhMSFT

shizhMSFT commented Jun 30, 2023

Copy link
Copy Markdown
Contributor

Referrers index deletion happens after manifest deletion right? If the registry already supports deletion and the token used covers manifest deletion, why would referrers index fail?

LoL. Makes sense.

shizhMSFT
shizhMSFT reviewed Jun 30, 2023
View reviewed changes
Comment thread cmd/oras/internal/option/referrers.go Outdated
@qweeah
update comment
757f2ae 
Signed-off-by: Billy Zha <jinzha1@microsoft.com>
shizhMSFT
shizhMSFT approved these changes Jul 3, 2023
View reviewed changes

@shizhMSFT shizhMSFT left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

shizhMSFT
shizhMSFT approved these changes Jul 3, 2023
View reviewed changes

@shizhMSFT shizhMSFT left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@qweeah qweeah merged commit ba60b99 into oras-project:main Jul 3, 2023
This was referenced Jul 5, 2023
Closed
Merged
@shizhMSFT shizhMSFT mentioned this pull request Jul 26, 2023
Closed
qweeah added a commit to qweeah/oras that referenced this pull request Aug 3, 2023
@qweeah
Revert "feat: add flag to skip deleting obsolete referrers index (ora…
078dd8e 
…s-project#957)"

This reverts commit ba60b99.

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
shizhMSFT pushed a commit to shizhMSFT/oras that referenced this pull request Aug 3, 2023
@qweeah @shizhMSFT
Revert "feat: add flag to skip deleting obsolete referrers index (ora…
fb3dd72 
…s-project#957)"

This reverts commit ba60b99.

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
shizhMSFT pushed a commit to shizhMSFT/oras that referenced this pull request Aug 3, 2023
@qweeah @shizhMSFT
feat: add flag to skip deleting obsolete referrers index (oras-projec…
5e6d3bf 
…t#957)

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
Co-authored-by: Terry Howe <tlhowe@amazon.com>
shizhMSFT pushed a commit to shizhMSFT/oras that referenced this pull request Aug 3, 2023
@qweeah @shizhMSFT
Revert "feat: add flag to skip deleting obsolete referrers index (ora…
6f6c9c1 
…s-project#957)"

This reverts commit ba60b99.

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
qweeah added a commit to qweeah/oras that referenced this pull request Aug 3, 2023
@qweeah
revert: "feat: add flag to skip deleting obsolete referrers index (or…
d25f652 
…as-project#957)"

This reverts commit ba60b99.

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
qweeah added a commit to qweeah/oras that referenced this pull request Aug 3, 2023
@qweeah
revert: "feat: add flag to skip deleting obsolete referrers index (or…
73b531d 
…as-project#957)"

This reverts commit ba60b99.

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
qweeah added a commit to qweeah/oras that referenced this pull request Aug 3, 2023
@qweeah
revert: "feat: add flag to skip deleting obsolete referrers index (or…
06700fb 
…as-project#957)"

This reverts commit ba60b99.

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
qweeah added a commit that referenced this pull request Aug 3, 2023
@qweeah
revert: "feat: add flag to skip deleting obsolete referrers index (#957…
43e4832 
…)" (#1046)

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@ZhongRuoyu ZhongRuoyu mentioned this pull request Aug 7, 2023
Merged
qweeah added a commit to qweeah/oras that referenced this pull request Aug 9, 2023
@qweeah
revert: "feat: add flag to skip deleting obsolete referrers index (or…
b3ac818 
…as-project#957)" (oras-project#1046)

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
qweeah added a commit to qweeah/oras that referenced this pull request Aug 9, 2023
@qweeah
revert: "feat: add flag to skip deleting obsolete referrers index (or…
44c61b9 
…as-project#957)" (oras-project#1046)

Signed-off-by: Billy Zha <jinzha1@microsoft.com>
@shizhMSFT shizhMSFT mentioned this pull request Jan 8, 2024
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Wwwsylvia Wwwsylvia Wwwsylvia left review comments

@shizhMSFT shizhMSFT shizhMSFT approved these changes

@sajayantony sajayantony Awaiting requested review from sajayantony sajayantony is a code owner

@SteveLasker SteveLasker Awaiting requested review from SteveLasker

@TerryHowe TerryHowe Awaiting requested review from TerryHowe TerryHowe is a code owner

Assignees

No one assigned

Labels

None yet

Projects

No open projects
ORAS Project Board-2023
Status: Done

Milestone

v1.1.0

4 participants

@qweeah @codecov-commenter @shizhMSFT @Wwwsylvia