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

Add Startup Probe to Tenant #1315

Merged
merged 1 commit into from
Oct 13, 2022

Conversation

dvaldivia
Copy link
Collaborator

Adds a configurable Startup Probe at tenant level which in turns gets passed to each pool and pod.

Essentially reverts #1284 since we ran into problems where startup time was longer than the hardcoded 30 seconds which ended up killing the pods, there's a known issue on GKE that causes pods to take up to 10 minutes to start if they are lacking a readiness endpoint and mixed with GKE load balancers. thus we need a configurable startup probe rather than a defaulted one.

⚠️ THE CRD IS MASSIVE BECASE WE UPDATED DEPENDENCIES TO KUBERNETES v1.25

Signed-off-by: Daniel Valdivia 18384552+dvaldivia@users.noreply.github.com

Signed-off-by: Daniel Valdivia <18384552+dvaldivia@users.noreply.github.com>
@dvaldivia
Copy link
Collaborator Author

dvaldivia commented Oct 12, 2022

heads up @hrenard I'm reverting your change in favor of a more configurable approach

@dvaldivia dvaldivia merged commit 813b649 into minio:master Oct 13, 2022
@hrenard
Copy link
Contributor

hrenard commented Oct 14, 2022

Thanks for the heads-up !
Indeed, hard-coded timeout can be a problem.
We should add some documentation because now there is no startup probe by default which is less stable in most cases.

I think that a startupProbeTimeout and a disableStartupProbe options is a better user experience. For me an operator should abstract some knowledge (like the startup probe endpoint) to reduce the risk of a user config breaking from one version to another.

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.

4 participants