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

[installer] Add option to use S3 as docker-registry backend #6912

Merged
merged 2 commits into from
Nov 29, 2021

Conversation

aledbf
Copy link
Member

@aledbf aledbf commented Nov 26, 2021

Description

Enable S3 as backend storage for inCluster docker-registry.
This is useful in at least two scenarios:

  • In EKS, where ECR requires the creation of repositories
  • self-hosted using S3 compatible storage (like Minio) instead of in cluster storage.

Release Notes

[installer] Add option to use S3 as docker-registry backend

@codecov
Copy link

codecov bot commented Nov 26, 2021

Codecov Report

Merging #6912 (b61b534) into main (a36fd97) will decrease coverage by 13.21%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #6912       +/-   ##
==========================================
- Coverage   19.04%   5.83%   -13.22%     
==========================================
  Files           2      13       +11     
  Lines         168    1148      +980     
==========================================
+ Hits           32      67       +35     
- Misses        134    1080      +946     
+ Partials        2       1        -1     
Flag Coverage Δ
components-local-app-app-linux-amd64 ?
components-local-app-app-linux-arm64 ?
components-local-app-app-windows-386 ?
components-local-app-app-windows-amd64 ?
components-local-app-app-windows-arm64 ?
installer-raw-app 5.83% <0.00%> (?)

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

Impacted Files Coverage Δ
installer/pkg/common/ca.go 0.00% <0.00%> (ø)
installer/pkg/common/storage.go 0.00% <0.00%> (ø)
installer/pkg/components/ws-manager/deployment.go 0.00% <0.00%> (ø)
components/local-app/pkg/auth/pkce.go
components/local-app/pkg/auth/auth.go
...components/ws-manager/unpriviledged-rolebinding.go 0.00% <0.00%> (ø)
installer/pkg/components/ws-manager/tlssecret.go 0.00% <0.00%> (ø)
installer/pkg/common/display.go 0.00% <0.00%> (ø)
installer/pkg/common/common.go 4.74% <0.00%> (ø)
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 915cd41...b61b534. Read the comment docs.

@aledbf aledbf changed the title [installer] Add option to use S3 as docker-register backend [installer] Add option to use S3 as docker-registry backend Nov 26, 2021
@aledbf aledbf marked this pull request as ready for review November 26, 2021 23:09
@mrsimonemms
Copy link
Contributor

/lgtm
/hold

I've put it on hold in case @csweichel wants to vary this approach, but it looks good to me

@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: a295ae24be35ccf9a89f4e3c9b002e20a4d94a13

@mrsimonemms
Copy link
Contributor

/approve no-issue

@roboquat
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: MrSimonEmms

Associated issue requirement bypassed by: MrSimonEmms

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@csweichel
Copy link
Contributor

csweichel commented Nov 29, 2021

The feature makes sense to me. I'm just weary of the cost it incurs, i.e. yet another test case.
At least now we have a path how we can remove this without inadvertently (but deliberately) breaking our user's installations.

/hold cancel

@csweichel
Copy link
Contributor

/hold cancel

@roboquat roboquat merged commit 8799724 into main Nov 29, 2021
@roboquat roboquat deleted the aledbf/insts3 branch November 29, 2021 18:07
@roboquat roboquat added deployed: workspace Workspace team change is running in production deployed Change is completely running in production labels Nov 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved deployed: workspace Workspace team change is running in production deployed Change is completely running in production release-note size/M team: workspace Issue belongs to the Workspace team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants