Fix startup health check command behavior - #27857
Conversation
daf5321 to
07f6c53
Compare
|
[NON-BLOCKING] Packit jobs failed. @containers/packit-build please check. Everyone else, feel free to ignore. |
07f6c53 to
609505e
Compare
1c2a037 to
1786799
Compare
|
I've restarted what appears to be flakey tests. The changes LGTM, but I definitely want a head nod from @mheon |
|
Hello @mheon, |
|
I think you need to add an alias for |
Hello, |
|
@mheon Any idea how to proceed here? |
|
@arsenalzp I think Matt meant that the |
|
@arsenalzp Any updates on this? |
Hello, |
|
Hello, |
|
Yes |
|
It was a quite time ago so, as far as I understand, we need to implement overriding of an image health check value by one from CLI. |
Signed-off-by: Oleksandr Krutko <alexander.krutko@gmail.com>
1786799 to
f791617
Compare
Honny1
left a comment
There was a problem hiding this comment.
I did review. My memory is rusty but based on converstion is needed:
- adding
--health-start-intervalas an alias for--health-startup-intervalfor Docker compatibility. - the
--start-periodand other flags can be seted from the image (HEALTHCHECK command). Here is the code that performs the override of image hc: https://github.com/containers/podman/blob/f7aefa46bbe5f0d8b1cfb0ff7b9fcd5fee2a9545/pkg/specgen/generate/container.go#L65
Note for me: Next review check whole hc logic.
|
[NON-BLOCKING] Packit jobs failed. @podman-container-tools/packit-jobs please check. Everyone else, feel free to ignore. |
Signed-off-by: Oleksandr Krutko <alexander.krutko@gmail.com> remove unnecessary conversion in FillOutSpecGen func Signed-off-by: Oleksandr Krutko <alexander.krutko@gmail.com>
5ebd6e3 to
9f883f9
Compare
|
Well Docker's This PR mixes StartupHC and HC by creating a I would not mix |
|
Hello, no this PR doesn't. The second issue the PR fixes - it is startup interval, which doesn't work. You can check corresponding issue link, there are plenty snippets which show how does Docker work and Podman doesn't. Let's summarize:
Here is the observation |
|
@Honny1 let's take a look on the fix was done by my PR: and now check the health: and bit later: So, the container's start-up health is checked every 4 sec, then the container was ready in 8 sec (the file was created) and the start-up health check has been transited to common health check algorithm which runs every 30 sec: Now, let's set the start-up health command: and then check the health: That's exactly what was expected! And now, let's take a look how does it work with the current algorithm ( let's check the health: As you may see the command runs every 30 sec, despite we set the start-up health interval to 4 sec. Another discrepancy: let's check the health in 10 sec: Hmm, the container is still starting, despite of the start-up period was set to 8 sec. The start-up health interval was to 4 sec, however the health check runs every 30 sec (default). |
This PR fixes #27724.
The issue is that health check command is ignored when options
--health-start-period,--health-startup-intervalare set, but--health-startup-cmdis not present.In contrast to
Dockerwhich uses health check command for the startup health check.Checklist
Ensure you have completed the following checklist for your pull request to be reviewed:
commits. (
git commit -s). (If needed, usegit commit -s --amend). The author email must matchthe sign-off email address. See CONTRIBUTING.md
for more information.
Fixes: #00000in commit message (if applicable)make validatepr(format/lint checks)Noneif no user-facing changes)Does this PR introduce a user-facing change?