Warn when running in a container without being PID 1#48479
Warn when running in a container without being PID 1#48479EspenRoth wants to merge 1 commit intokeycloak:mainfrom
Conversation
|
|
||
| # Warn if running in a container without being PID 1, as signals may not be forwarded correctly and graceful shutdown may not work | ||
| if [ "$KC_RUN_IN_CONTAINER" = "true" ] && [ "$$" != "1" ]; then | ||
| echo "WARNING: Keycloak is running inside a container, but is not PID 1. Graceful shutdown may not work. Use 'exec' in your entrypoint script to ensure signals are forwarded correctly. See https://www.keycloak.org/server/containers for more details." |
There was a problem hiding this comment.
The only additional consideration here is that this warning shouldn't matter for non-server mode commands (import, export, boostrap-admin). Is it worth accounting for this?
There was a problem hiding this comment.
Thanks for taking a look! Yeah I did not consider that. Will modify to exclude those scenarios
0797b9f to
110d400
Compare
| case "$1" in | ||
| -D*) SERVER_OPTS="$SERVER_OPTS ${OPT}";; | ||
| *) CONFIG_ARGS="$CONFIG_ARGS ${OPT}" | ||
| [ -z "$KC_CMD" ] && KC_CMD="$1" |
There was a problem hiding this comment.
Unfortunately this assumption won't generally hold. You are allowed to put arguments like -v or -cf ahead of command.
We could refine this to be the first argument without a leading -, or to avoid any additional arugment parsing in kc.sh we could instead set the initiating pid as an environment variable or system property and let the java logic emit the warning.
There was a problem hiding this comment.
@shawkins I think I've addressed this concern if you could take another look?
There was a problem hiding this comment.
Let me kick off the ci run. I'll try to think of any additional corner cases.
When KC_RUN_IN_CONTAINER=true but the process is not PID 1, graceful shutdown may fail silently because signals are not forwarded correctly. This adds a warning at startup to alert users to use exec in their entrypoint scripts. Closes keycloak#48059 Signed-off-by: Espen Roth <[email protected]>
110d400 to
d1b6524
Compare
Summary
When
KC_RUN_IN_CONTAINER=truebut the Keycloak process is not PID 1, graceful shutdown may fail silently because signals such asSIGTERMare not forwarded correctly. This can lead to cache inconsistencies or data loss.This change adds a startup warning in
kc.shto alert users to useexecin their entrypoint scripts, with a link to the existing container documentation that already describes the correct approach. The warning is only emitted for server commands (start,start-dev) and is skipped for non-server commands such asimport,export, andbootstrap-admin.Changes
quarkus/dist/src/main/content/bin/kc.sh— added PID 1 check with warning message printed at startup when running in a container without being PID 1; command detection refined to identify the first non-flag argument so that flags like-vor-cfpreceding the subcommand are handled correctlyquarkus/tests/integration/src/test/java/org/keycloak/it/cli/dist/JavaOptsScriptTest.java— added three tests covering the presence and absence of the warningTest plan
testContainerNonPid1Warning— asserts warning appears whenKC_RUN_IN_CONTAINER=trueand command isstarttestNoContainerNonPid1Warning— asserts warning is absent when env var is not settestContainerNonPid1WarningAbsentForNonServerCommands— asserts warning is absent for non-server commands (e.g.export)JavaOptsScriptTesttests passspotless:checkpassesNotes
Claude Code was used to assist in writing the code, tests, and PR description for this contribution. All changes have been reviewed and are fully understood by the author.
Closes #48059