logs: k8s-file: restore poll sleep #10742
Conversation
Fix the suprious "Error: nil" messages. Also add some more context to logged error messages which makes error sources more obvious. Signed-off-by: Valentin Rothberg <[email protected]>
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: vrothberg The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
vrothberg
added
the
needs-backport
|
Needs a backport to v3.2. @containers/podman-maintainers PTAL |
Commit 84b55ee attempted to fix a race waiting for the container died event. Previously, Podman slept for duration of the polling frequence which I considerred to be a mistake. As it turns out, I was mistaken since the file logger will, in fact, NOT read until EOF and then stop logging but stop logging immediately _after_ it woke up. [NO TESTS NEEDED] as the race condition cannot be hit reliably. Fixes: containers#10675 Signed-off-by: Valentin Rothberg <[email protected]>
| // read the file until EOF. | ||
| // Make sure to wait at least for the poll duration | ||
| // before stopping the file logger (see #10675). | ||
| time.Sleep(watch.POLL_DURATION) |
There was a problem hiding this comment.
I'm always worried about putting a hard sleep into a function like this, but I'm not thinking of a better way atm.
There was a problem hiding this comment.
Same here. I really did not like it which is why I removed it previously. I read the docs of the library we're using but they did not reflect reality :(
|
LGTM |
|
/lgtm |
github-actions
bot
added
the
locked - please file new issue/PR
Commit 84b55ee attempted to fix a race waiting for the container
died event. Previously, Podman slept for duration of the polling
frequence which I considerred to be a mistake. As it turns out, I was
mistaken since the file logger will, in fact, NOT read until EOF and
then stop logging but stop logging immediately after it woke up.
[NO TESTS NEEDED] as the race condition cannot be hit reliably.
Fixes: #10675
Signed-off-by: Valentin Rothberg [email protected]