Skip to content
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

extended_bin: Avoid using logger #947

Closed
wants to merge 1 commit into from
Closed

Conversation

weiss
Copy link
Contributor

@weiss weiss commented Feb 21, 2025

The user running the release may not have the privileges required to using logger(1). In that case, logger(1) returns non-zero, so the extended start script would fail (due to set -e).

Modern systems (e.g., systemd-based or container solutions) typically expect services to write log messages to the standard output, and the extended start script does this in other places already. Therefore, just don't bother with logger(1) anymore.

The user running the release may not have the privileges required to
using logger. In that case, logger returns non-zero, so the extended
start script would fail (due to "set -e").

Modern systems (e.g., systemd-based or container solutions) typically
expect services to write log messages to the standard output, and the
extended start script does this in other places already. Therefore, just
don't use logger anymore.
@weiss
Copy link
Contributor Author

weiss commented Feb 24, 2025

If you'd rather stick to using logger(1) if possible, I'd suggest to just fall back to echo in case logger(1) exists non-zero (git pull https://github.com/weiss/relx.git fix/catch-logger-failure).

@tsloughter
Copy link
Member

Hm, yes, this is probably for the best.

@tsloughter
Copy link
Member

I do worry if anyone is expecting that log message in their system logs. I kinda doubt it but have no way to guarantee that, which is my only hangup.

@tsloughter
Copy link
Member

I wonder if if ! command -v logger > /dev/null 2>&1 was meat to catch issues calling logger but doesn't work?

This is code that has been carried over for nearly decades now from riak I believe.

@weiss
Copy link
Contributor Author

weiss commented Feb 24, 2025

I do worry if anyone is expecting that log message in their system logs. I kinda doubt it but have no way to guarantee that, which is my only hangup.

Yes I was wondering myself. So #948 is probably the safer solution, and this PR could be closed.

I wonder if if ! command -v logger > /dev/null 2>&1 was meat to catch issues calling logger but doesn't work?

That part makes sure the logger command exists before trying to use it. My user ran into the case where the command does exist but executing it without special privileges failed (/dev/log isn't world-writable on a AlmaLinux 8, see processone/eturnal#84 (comment)).

@ferd
Copy link
Collaborator

ferd commented Feb 24, 2025

Agreed the other version looks like a better plan.

@weiss weiss closed this Feb 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants