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

[JBEAP-27281](XP 5) - Fix Liveness and Startup probes definition (initial delay and startup path) #73

Open
wants to merge 2 commits into
base: eap-xp5-dev
Choose a base branch
from

Conversation

fabiobrz
Copy link

@fabiobrz fabiobrz commented Jun 11, 2024

A potential fix for https://issues.redhat.com/browse/JBEAP-27281 (if confirmed).

@fabiobrz fabiobrz changed the title Fix.live startup initial delay [JBEAP-27281](XP 5) - Fix Liveness and Startup probes definition (initial delay and startup path) Jun 11, 2024
readinessProbe:
httpGet:
path: /health/ready
port: admin
initialDelaySeconds: 10
startupProbe:
httpGet:
path: /health/live
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 to point to this URL

@@ -32,14 +32,14 @@ deploy:
httpGet:
path: /health/live
port: admin
initialDelaySeconds: 60
initialDelaySeconds: 10
Copy link
Contributor

Choose a reason for hiding this comment

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

initialDelaySeconds of the livenessProbe should be removed as a startupProbe is defined

Copy link
Author

Choose a reason for hiding this comment

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

Ok, removed.

initialDelaySeconds: 10
Copy link
Contributor

Choose a reason for hiding this comment

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

With that setting, k8s will wait at least 10 seconds and at most 40 seconds (10 (initialDelaySeconds) + 3 (failureThreshold) * 10 (periodSeconds) to check that the server is started.

40 seconds might not be enough for some EAP XP applications and I'd prefer we extend that to 1 minute.
User have the ability to define their own thresholds but I'd prefer that by default, we wait more rather than not enough and risk false positive startup failure.

Could you add:

failureThreshold: 5

to your PR please?

Copy link
Author

Choose a reason for hiding this comment

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

+1, done.

@fabiobrz fabiobrz force-pushed the fix.live-startup-initial-delay branch from 08aa4ea to 62882d4 Compare June 11, 2024 14:35
@fabiobrz fabiobrz requested a review from jmesnil June 11, 2024 14:58
@jfdenise
Copy link
Collaborator

@fabiobrz are these changes planned for XP5? I see that they are not yet merged.

@jfdenise
Copy link
Collaborator

It appears that this one will be delayed after XP5 release. Keeping it un-merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants