-
Notifications
You must be signed in to change notification settings - Fork 16
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
base: eap-xp5-dev
Are you sure you want to change the base?
[JBEAP-27281](XP 5) - Fix Liveness and Startup probes definition (initial delay and startup path) #73
Conversation
…conds to 10 Signed-off-by: Fabio Burzigotti <[email protected]>
readinessProbe: | ||
httpGet: | ||
path: /health/ready | ||
port: admin | ||
initialDelaySeconds: 10 | ||
startupProbe: | ||
httpGet: | ||
path: /health/live |
There was a problem hiding this comment.
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
charts/eap-xp5/values.yaml
Outdated
@@ -32,14 +32,14 @@ deploy: | |||
httpGet: | |||
path: /health/live | |||
port: admin | |||
initialDelaySeconds: 60 | |||
initialDelaySeconds: 10 |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, removed.
charts/eap-xp5/values.yaml
Outdated
initialDelaySeconds: 10 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, done.
Signed-off-by: Fabio Burzigotti <[email protected]>
08aa4ea
to
62882d4
Compare
@fabiobrz are these changes planned for XP5? I see that they are not yet merged. |
It appears that this one will be delayed after XP5 release. Keeping it un-merged. |
A potential fix for https://issues.redhat.com/browse/JBEAP-27281 (if confirmed).