quadlet: only log to kmsg when running under systemd#28898
Conversation
| line := fmt.Sprintf("quadlet-generator[%d]: %s", os.Getpid(), s) | ||
|
|
||
| if !logToKmsg(line) || dryRunFlag { | ||
| stderrIsTerminal := term.IsTerminal(int(os.Stderr.Fd())) |
There was a problem hiding this comment.
That is wrong though, there is not requirement whatsoever that someone running quadlet to debug things should have a tty on stderr, in fact someone might want to pipe the output to a file
IF we want to check if we run under systemd there seem to be some env vars documented we could check https://www.freedesktop.org/software/systemd/man/latest/systemd.generator.html#Environment
but even then I would guess systemd-analyze --generators verify might set the same so they cannot be used to know that either?
In any case it seems simplest and safest to just log to both.
There was a problem hiding this comment.
That is still open, I fail to see how this relates to having a tty or not.
a6f1c31 to
9ec4699
Compare
|
|
||
| if !logToKmsg(line) || dryRunFlag { | ||
| stderrIsTerminal := term.IsTerminal(int(os.Stderr.Fd())) | ||
| kmsgOK := false |
There was a problem hiding this comment.
Maybe we should just never log to kmesg if euid != 0.
|
/packit retest-failed |
When the quadlet generator is run manually from a terminal, warnings and errors were written to /dev/kmsg, polluting the kernel log. Write to stderr for interactive runs and keep the kmsg path for systemd invocations (no tty), preserving the existing fallback behavior. Fixes: podman-container-tools#28888 Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
2ce9b56 to
3a36ebf
Compare
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)gofmtandgo vet(make validateprneeds a containerized environment not available locally; CI runs the full validation)Noneif no user-facing changes)Does this PR introduce a user-facing change?
Fixes #28888