-
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
Set flags to test 'logs -f' with journald driver #12118
Conversation
`logs -f` with `journald` is supported only when `journald` events backend is used. To pass system tests using `logs -f` in an environment where `events_logger` is not set to `journald` in `containers.conf`, this fix sets `--events-backend` or `--log-driver` temporally. Signed-off-by: Hironori Shiina <[email protected]>
Test failures look like network flakes, I've hit rerun |
# Since PR#10431, 'logs -f' with journald driver is only supported with journald events backend. | ||
if [[ $driver = "journald" ]]; then | ||
run_podman info --format '{{.Host.EventLogger}}' >/dev/null | ||
if [[ $output != "journald" ]]; then |
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.
Probably late-night brain malfunction on my part, but should this be ==
rather then !=
?
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.
I think !=
is correct. When the event logger from containers.conf
is NOT journald
, I would like to add --events-backend jouranld
so that logs -f
works.
Thanks.
@edsantiago PTAL |
Tested via: # cat /etc/containers/containers.conf
[containers]
log_driver = "journald"
[engine]
events_logger = "file" Confirmed that tests 035-logs and 130-kill fail on main, and pass under this PR. Confirmed that all other system tests behave the same (as in: mostly pass, but fail due to some new I feel super uncomfortable about the number of incompatible (and untested) config permutations we're having. |
/lgtm Thank you @hshiina |
/approve |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hshiina, rhatdan 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 |
Signed-off-by: Hironori Shiina [email protected]
What this PR does / why we need it:
logs -f
withjournald
is supported only whenjournald
events backend is used. To pass system tests usinglogs -f
in an environment whereevents_logger
is not set tojournald
incontainer.conf
, this fix set--events-backend
or--log-driver
temporally.How to verify it
035-logs.bats
withcontainers.conf
whereevents_logger
is notjournald
.130-kill.bats
withcontainers.conf
whereevents_logger
is notjournald
andlog_driver
isjournald
.Which issue(s) this PR fixes:
None
Special notes for your reviewer: