Fix: Ensure HealthCheck exec session terminates on timeout #26086
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Honny1 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
Previously, the HealthCheck exec session would not terminate on timeout, allowing the healthcheck to run indefinitely. Fixes: https://issues.redhat.com/browse/RHEL-86096 Signed-off-by: Jan Rodák <[email protected]>
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
|
|
||
| It("podman healthcheck - health timeout", func() { | ||
| ctrName := "c-h-" + RandomString(6) | ||
| podmanTest.PodmanExitCleanly("run", "-d", "--name", ctrName, "--health-cmd", "sleep 100; echo hc-done", "--health-timeout=3s", ALPINE, "top") |
There was a problem hiding this comment.
In order to test the grace period and SIGTERM behavior you likely should install a signal handler to test we get the proper signal
"trap 'echo Received SIGTERM, finishing; exit' SIGTERM; while :; do sleep 1; done"
And you could do a version without the exit to force the SIGKILL as it doesn't exit after SIGTERM then.
| return -1, fmt.Errorf("container %s light exec session with pid: %d error: %v", c.ID(), pid, err) | ||
| } | ||
| case <-time.After(timeout): | ||
| if err := c.ociRuntime.ExecStopContainer(c, session.ID(), uint(timeout)); err != nil { |
There was a problem hiding this comment.
Does this extra stop timeout (grace period) is also what docker is doing?
Previously, the HealthCheck exec session would not terminate on timeout, allowing the healthcheck to run indefinitely.
Because Docker does that, Podman should kill the HealthCheck when it reaches the timeout.
Fixes: https://issues.redhat.com/browse/RHEL-86096
Depends on: #25906
Does this PR introduce a user-facing change?