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

Introduce Sidecar Args #1437

Merged
merged 6 commits into from
Feb 16, 2023
Merged

Introduce Sidecar Args #1437

merged 6 commits into from
Feb 16, 2023

Conversation

dvaldivia
Copy link
Collaborator

@dvaldivia dvaldivia commented Feb 6, 2023

Introduces an initContainer and a sidecar container for MinIO to retrieve the MINIO_ARGS for the tenant and to monitor the configurations secret and update it locally as soon as it changes.

Additional this sidecars will validate if root credentials are missing and if so, error out.

Screenshot 2023-02-07 at 2 14 43 PM

cmd/operator/app_commands.go Show resolved Hide resolved
pkg/apis/minio.min.io/v2/constants.go Outdated Show resolved Hide resolved
Signed-off-by: Daniel Valdivia <18384552+dvaldivia@users.noreply.github.com>
Signed-off-by: Daniel Valdivia <18384552+dvaldivia@users.noreply.github.com>
Signed-off-by: Daniel Valdivia <18384552+dvaldivia@users.noreply.github.com>
Signed-off-by: Daniel Valdivia <18384552+dvaldivia@users.noreply.github.com>
Signed-off-by: Daniel Valdivia <18384552+dvaldivia@users.noreply.github.com>
@cniackz
Copy link
Contributor

cniackz commented Feb 16, 2023

Hello @dvaldivia I am starting to review and test this PR, I want to take my time to review this truly, so please merge PR if needed otherwise I will place my feedback soon!

@cniackz
Copy link
Contributor

cniackz commented Feb 16, 2023

Cesars-MacBook-Pro:operator cniackz$ oc get serviceaccount -n tenant-lite
NAME              SECRETS   AGE
builder           1         3h4m
default           1         3h4m
deployer          1         3h4m
minio-operator    1         161m
storage-lite-sa   1         18m <------------ I can see the new service account being created

And because there is new Service Account, then we need extra line for the priviledged access in OpenShift:

oc adm policy add-scc-to-user privileged -n tenant-lite -z storage-lite-sa

Or have proper constraints for the security context, anyway, the important part is that Tenant can be deployed and initialized in OpenShift in this change:

Screenshot 2023-02-16 at 1 11 39 PM

Also as a side note, used operator-ca-tls in the meantime, since we still face TLS issue here, but will be solved in #1450

My next step is to keep exploring and test in k8s as well.

@cniackz
Copy link
Contributor

cniackz commented Feb 16, 2023

Also I can see the 3 containers here and the volumes being used:

Screenshot 2023-02-16 at 1 15 10 PM

Let's see what happen if MinIO Server is terminated...

@cniackz
Copy link
Contributor

cniackz commented Feb 16, 2023

It is very quickly restored, quicker than before:

Screenshot 2023-02-16 at 1 16 19 PM

@cniackz
Copy link
Contributor

cniackz commented Feb 16, 2023

Great, I can reach MinIO Console Service as well via Route:

Screenshot 2023-02-16 at 1 18 13 PM

In general, this change is not breaking anything so far but just making things better and faster!!!!

Copy link
Contributor

@cniackz cniackz left a comment

Choose a reason for hiding this comment

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

Great Job @dvaldivia thanks for this big Change!!! 👍

@dvaldivia dvaldivia dismissed harshavardhana’s stale review February 16, 2023 22:31

license added to files

@dvaldivia dvaldivia merged commit 299b00d into minio:master Feb 16, 2023
@dvaldivia dvaldivia deleted the sidecar-args branch February 16, 2023 22:32
@pjuarezd pjuarezd mentioned this pull request Mar 19, 2023
@djwfyi djwfyi mentioned this pull request Mar 30, 2023
8 tasks
@fgleixner
Copy link

Hi,
i just tried to understand, why the minio operator switch --cluster-domain does not work any more. All defaults to cluster.local. All tenants are broken. Is it possible, that introducing sidecars and deleting the function GetEnvHandler in http_handlers.go introduced this bug? #1535 and #1544

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.

8 participants