fix: Container with Notify=healthy respect HealthStartPeriod larger than TimeoutStartSec.
#27631
Conversation
…han TimeoutStartSec Signed-off-by: Mujib Ahasan <[email protected]>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: Mujib-Ahasan 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 |
Honny1
left a comment
Honny1
left a comment
There was a problem hiding this comment.
I reviewed the code and have some suggestions for improvements. Please also add a test for this.
libpod/container_internal.go
Outdated
| } | ||
|
|
||
| msg := fmt.Appendf(nil, "EXTEND_TIMEOUT_USEC=%d", step.Microseconds()) | ||
| if _, err := conn.Write(msg); err != nil { |
There was a problem hiding this comment.
Can sending messages be simplified using notifyproxy.SendMessage?
libpod/container_internal.go
Outdated
| extension := 30 * time.Second | ||
| timerFreq := 25 * time.Second | ||
|
|
||
| if c.config.SdNotifySocket != "" && needsExtension { |
There was a problem hiding this comment.
What if c.config.SdNotifySocket is empty?
notifyproxy.SendMessage should already handle that.
Signed-off-by: Mujib Ahasan <[email protected]>
|
Hey @Honny1, could you please be more specific about the test case?? I simplified the message sending as you mentioned, but confused about the test coverage. |
|
Cockpit tests failed for commit 0876822. @martinpitt, @jelly, @mvollmer please check. |
|
Sure. I mean a test case that reproduces the original issue. It should fail without your fix and pass with it. |
2 participants
HealthStartPeriod > 0HealthStartPeriodby sending periodicEXTEND_TIMEOUT_USECmessagesHealthStartPeriod.copyInternalfunction https://github.com/containers/container-libs/blob/a3b0f19c3ea12cb02f0b11788489c52d4fb02e96/common/libimage/copier.go#L392-L421suggested by maintainers.
closes #27290
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?