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

Upgrade to distribution (registry) v3 alpha #19784

Merged
merged 8 commits into from
Jan 26, 2024

Conversation

AaronDewes
Copy link

@AaronDewes AaronDewes commented Jan 2, 2024

Comprehensive Summary of your change

This includes all the benefits of the v3 distribution, but also all breaking changes.

Most notably, Image Manifest v2 Schema v1 support has been dropped, as well as the oss and swift storage drivers.

Currently, this still relies on v2's github.com/docker/distribution through indirect dependencies.

This PR is a draft because distribution v3 isn't stable yet. However, I am sucessfully running Harbor of this branch.

Issue being fixed

None

Please indicate you've done the following:

  • Well Written Title and Summary of the PR
  • Label the PR as needed. "release-note/ignore-for-release, release-note/new-feature, release-note/update, release-note/enhancement, release-note/community, release-note/breaking-change, release-note/docs, release-note/infra, release-note/deprecation"
  • Accepted the DCO. Commits without the DCO will delay acceptance.
  • Made sure tests are passing and test coverage is added if needed.
  • Considered the docs impact and opened a new docs issue or PR with docs changes if needed in website repository.

@AaronDewes AaronDewes requested a review from a team as a code owner January 2, 2024 17:33
@AaronDewes AaronDewes marked this pull request as draft January 2, 2024 17:33
Copy link

codecov bot commented Jan 3, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (9e5efc9) 70.63% compared to head (55de925) 45.25%.

Additional details and impacted files

Impacted file tree graph

@@                    Coverage Diff                    @@
##           distribution_v3_test   #19784       +/-   ##
=========================================================
- Coverage                 70.63%   45.25%   -25.39%     
=========================================================
  Files                       747      244      -503     
  Lines                     95833    13332    -82501     
  Branches                      0     2720     +2720     
=========================================================
- Hits                      67696     6033    -61663     
+ Misses                    24469     6998    -17471     
+ Partials                   3668      301     -3367     
Flag Coverage Δ
unittests 45.25% <100.00%> (-25.39%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...ponents/label-selector/label-selector.component.ts 68.18% <100.00%> (ø)

... and 989 files with indirect coverage changes

@wy65701436
Copy link
Contributor

thanks @AaronDewes , I'm considering using a separate branch for your changes. This approach allows us to conduct thorough testing to prevent any potential regressions.

@MinerYang
Copy link
Contributor

Hi @AaronDewes ,

Thanks for your contribution! Would you mind changing the merge branch from main to distribution_v3_test. We would like to do regression test based on this branch

@AaronDewes AaronDewes changed the base branch from main to distribution_v3_test January 9, 2024 05:26
@AaronDewes
Copy link
Author

Hi @AaronDewes ,

Thanks for your contribution! Would you mind changing the merge branch from main to distribution_v3_test. We would like to do regression test based on this branch

Sure, I'll also rebase this later.

@AaronDewes
Copy link
Author

The files that depend on "github.com/docker/distribution/registry/client/auth/challenge" still need refactoring to drop that dependency; but I can do that in a separate PR if you already want to start testing this.

alrs and others added 5 commits January 9, 2024 12:45
Signed-off-by: Lars Lehtonen <lars.lehtonen@gmail.com>
Signed-off-by: Yang Jiao <yang.jiao@broadcom.com>
Co-authored-by: Yang Jiao <yang.jiao@broadcom.com>
fixes goharbor#19429

Signed-off-by: stonezdj <daojunz@vmware.com>
Co-authored-by: stonezdj <daojunz@vmware.com>
Signed-off-by: Yang Jiao <yang.jiao@broadcom.com>
Co-authored-by: Yang Jiao <yang.jiao@broadcom.com>
remove the unused the part from makefile

Signed-off-by: wang yan <wangyan@vmware.com>
Signed-off-by: Yang Jiao <yang.jiao@broadcom.com>
@wy65701436
Copy link
Contributor

As for the separate branch, we can proceed by merging the PR and then focus on regression testing. If any issues are identified during testing, we can address them promptly. Additionally, there may be a need to contribute the changes upstream.

@MinerYang Im OK to merge it if the CI passes, and start the regression testing.

ShengqiWang and others added 2 commits January 15, 2024 06:09
* fix artifact page bug

* update testcase
This includes all the benefits of the v3 distribution, but also all breaking changes.

Most notably, Image Manifest v2 Schema v1 support has been dropped, as well as the `oss` and `swift` storage drivers.

Currently, this still relies on v2's github.com/docker/distribution/registry/client/auth/challenge, because that code has been removed from the public API in v3.

Signed-off-by: Aaron Dewes <aaron.dewes@protonmail.com>
@AaronDewes
Copy link
Author

@wy65701436 I fixed the build failures, but DCO fails now after I rebased on master. Can you merge master into distribution_v3_test first, so the DCO bot passes? And the CI requires another approval because I pushed a commit and I'm a first time contributor.

@MinerYang
Copy link
Contributor

Hi @AaronDewes ,
Please help to fix the DCO via git config and amend your commit with -s

@AaronDewes
Copy link
Author

@MinerYang My commit is signed off. The problem is that this PR contains commits from master not yet in the distribution_v3_test branch. The commits in master are not from me and not all signed off. You or another project member need to merge master into ``distribution_v3_test`.

@Vad1mo Vad1mo added the release-note/ignore-for-release Do not include PR or Issue for release notes label Jan 26, 2024
@Vad1mo Vad1mo merged commit caee762 into goharbor:distribution_v3_test Jan 26, 2024
15 of 18 checks passed
@AaronDewes AaronDewes deleted the distribution-v3 branch January 26, 2024 21:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants