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.

Sign up for GitHub

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

Jump to bottom

Implement multitenancy #50

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Implement multitenancy #50

wants to merge 3 commits into from

Conversation

Donatien26
Copy link
Collaborator

@Donatien26 Donatien26 commented Aug 26, 2024 •
edited
Loading

This PR allow to:

  • Hability to add labelSelector to filter which resource a S3Operator instance must manage
  • Hability to manage multiple s3 Instance

@Donatien26 Donatien26 force-pushed the implement-multitenancy branch from 444d4f4 to 34b85b3 Compare August 26, 2024 14:16
@sathieu
Copy link
Collaborator

sathieu commented Sep 13, 2024 •
edited
Loading

This PR allow to:

* Hability to add labelSelector to filter which resource a S3Operator instance must manage

* Hability to manage multiple s3 Instance

I think those two features are orthogonal. I would go for 2️⃣ only.

EDIT:

Proposed roadmap:

  • add s3InstanceRef, if empty the controller args are used (for backward compat) (this PR)
  • deprecate args to configure default instance
  • BREAKING: s3InstanceRef mandatory, and remove args to configure default instance

sathieu
sathieu reviewed Sep 13, 2024
View reviewed changes
api/v1alpha1/s3instance_types.go Show resolved Hide resolved
api/v1alpha1/bucket_types.go Outdated Show resolved Hide resolved
api/v1alpha1/path_types.go Show resolved Hide resolved
api/v1alpha1/policy_types.go Show resolved Hide resolved
api/v1alpha1/s3instance_types.go Outdated Show resolved Hide resolved
api/v1alpha1/s3instance_types.go Outdated Show resolved Hide resolved
api/v1alpha1/s3user_types.go Show resolved Hide resolved
@Donatien26 Donatien26 force-pushed the implement-multitenancy branch 3 times, most recently from c4bda93 to bc9818a Compare September 19, 2024 13:34
sathieu
sathieu requested changes Sep 26, 2024
View reviewed changes
Copy link
Collaborator

@sathieu sathieu left a comment

Choose a reason for hiding this comment

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

Here is another set of reviews. Sorry for the added work.

I think the default instance stuff should be removed, the default instance could be added to the chart easily.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
api/v1alpha1/s3instance_types.go Outdated Show resolved Hide resolved
api/v1alpha1/s3instance_types.go Outdated Show resolved Hide resolved
api/v1alpha1/s3instance_types.go Outdated Show resolved Hide resolved
controllers/s3instance_controller.go Outdated Show resolved Hide resolved
controllers/s3instance_controller.go Outdated Show resolved Hide resolved
controllers/s3instance_controller.go Outdated Show resolved Hide resolved
controllers/s3instance_controller.go Outdated Show resolved Hide resolved
internal/utils/utils.go Outdated Show resolved Hide resolved
@Donatien26 Donatien26 force-pushed the implement-multitenancy branch 13 times, most recently from ae58bdd to f7549e0 Compare October 18, 2024 13:39
sathieu
sathieu requested changes Oct 18, 2024
View reviewed changes
Copy link
Collaborator

@sathieu sathieu left a comment

Choose a reason for hiding this comment

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

A few remaining comments. This is almost done 👍

README.md Show resolved Hide resolved
controllers/s3instance_controller.go Outdated Show resolved Hide resolved
controllers/s3instance_controller.go Outdated Show resolved Hide resolved
controllers/s3instance_controller.go Outdated Show resolved Hide resolved
controllers/s3instance_controller.go Outdated Show resolved Hide resolved
internal/s3/s3ClientCache.go Outdated Show resolved Hide resolved
@Donatien26 Donatien26 force-pushed the implement-multitenancy branch 5 times, most recently from 6362c60 to de342d8 Compare November 5, 2024 13:05
@Donatien26
Copy link
Collaborator Author

TODO don't encode base64 cert

sathieu
sathieu reviewed Nov 5, 2024
View reviewed changes
controllers/bucket_controller.go Outdated Show resolved Hide resolved
controllers/path_controller.go Outdated Show resolved Hide resolved
phlg
phlg reviewed Nov 5, 2024
View reviewed changes
api/v1alpha1/path_types.go Show resolved Hide resolved
config/samples/s3.onyxia.sh_v1alpha1_s3instance.yaml Outdated Show resolved Hide resolved
controllers/bucket_controller.go Outdated Show resolved Hide resolved
controllers/bucket_controller.go Outdated Show resolved Hide resolved
controllers/bucket_controller.go Outdated Show resolved Hide resolved
controllers/s3instance_controller.go Outdated Show resolved Hide resolved
main.go Outdated Show resolved Hide resolved
@Donatien26 Donatien26 force-pushed the implement-multitenancy branch from de342d8 to 4a29bd1 Compare November 7, 2024 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@phlg phlg phlg left review comments

@sathieu sathieu sathieu requested changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Donatien26 @sathieu @phlg