-
Notifications
You must be signed in to change notification settings - Fork 2.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix "tail 800 lines: journald" flake #14443
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, nice catch!
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: Luap99, 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 |
Could have just removed the -d from command above, but this works wel also. I am surprised this one is not firing all the time. |
No I do not want to remove the |
LGTM, but do we have any clue why this test runs podman with |
You could redirect the output to /dev/null. But anyways... |
Then the test will no longer work, it must print the lines in the container and we cannot redirect the output from the podman command inside the testing suite. |
They are anyway, or at least 800 of them are (the output of |
Removed the -d from the other test since it just prints 3 lines. |
/lgtm |
The test calls podman run -d followed by podman logs. There is no guarantee the the container or conmon has written all its output. Adding an extra podman wait should fix this. Do not remove the -d to not print 1000 unnecessary lines in the logs. Fixes containers#14362 Signed-off-by: Paul Holzinger <[email protected]>
/hold cancel ...and I'd just like to use this PR to confirm that #13936 is working nicely:
|
The test calls podman run -d followed by podman logs. There is no
guarantee the the container or conmon has written all its output.
Adding an extra podman wait should fix this.
Fixes #14362
Does this PR introduce a user-facing change?