GUACAMOLE-2086: Add Tomcat HealthCheckValve - #1119
Conversation
dmunozv04
changed the title
|
Fixed, sorry for that |
075340e to
a10f9d8
Compare
| if [ "$HEALTH_CHECK_VALVE_ENABLED" = "true" ]; then | ||
| # Default health check path (/health) | ||
| HEALTH_CHECK_PATH="${HEALTH_CHECK_VALVE_PATH:-/health}" | ||
| # Perform actual health check via curl | ||
| curl --fail --silent --show-error "http://localhost:8080${HEALTH_CHECK_PATH}" || exit 1 |
There was a problem hiding this comment.
A couple of concerns about the HEALTH_CHECK_VALVE_PATH variable:
- It looks like it assumes that an absolute path will be provided?
- What happens if the user provides a relative path (no leading /), both here and in the server.xml file?
There was a problem hiding this comment.
I assumed an absolute path would be provided since it looks like tomcat only supports absolute paths:
https://github.com/apache/tomcat/blob/eaea7503163e44ba7260c9c7b3af3f4ba387d072/java/org/apache/catalina/valves/HealthCheckValve.java#L82
If the user provides a relative path, the health check won't be reachable and curl will complain: "curl: (3) URL rejected: Port number was not a decimal number between 0 and 65535\n"
Maybe a check can be added when loading the variables to warn the user if the path is not absolute.
There was a problem hiding this comment.
Yeah, I would think checking the variable would be the right way to go...
3 participants
This PR allows to set Tomcat's HealthCheckValve with environment variables. The implementation is taken from the RemoteIpValve script.
EDIT: this is useful so orchestrators (like kubernetes) or docker itself can know if the service is ready to accept connections.
The second commit uses these new variables to configure docker's HEALTHCHECK. This could be removed if it shouldn't be enabled by default.
I'm happy to make any changes necessary. Thanks for reviewing this PR.