Replace systemctl status with systemctl is-active
#1652
+16
−12
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
bastelfreak
requested review from
a team,
SimonHoenscheid,
alexjfisher,
deric,
ekohl and
smortex
as code owners
bastelfreak
added a commit
to bastelfreak/puppet-foreman
that referenced
this pull request
Dec 13, 2025
`systemctl status $service` will output a bunch of text. Some of this
text, depending on the locale, contains unicode characters:
```
● postgresql.service - PostgreSQL RDBMS
Loaded: loaded (/lib/systemd/system/postgresql.service; enabled; preset: enabled)
Active: active (exited) since Wed 2025-09-24 21:40:00 UTC; 2 months 18 days ago
Main PID: 1028 (code=exited, status=0/SUCCESS)
CPU: 3ms
Notice: journal has been rotated since unit was started, output may be incomplete.
```
`●` is unicode. Puppet will fail with:
```
Error: /Stage[main]/Postgresql::Server::Reload/Postgresql::Server::Instance::Reload[main]/Exec[postgresql_reload_main]: Failed to call refresh: invalid byte sequence in US-ASCII
Error: /Stage[main]/Postgresql::Server::Reload/Postgresql::Server::Instance::Reload[main]/Exec[postgresql_reload_main]: invalid byte sequence in US-ASCII
```
`systemctl` will print `*` if it's executed with `LC_ALL=C`, but we
cannot assume that this locale is always used.
The correct command to check if a service is up and running is
`is-active`, not `status`. `status` is for humans only. the systemd
provider for the service type also uses `is-active`. See
https://github.com/puppetlabs/puppet/blob/main/lib/puppet/provider/service/systemd.rb#L201-L203
Since we only care about the exit code, we can even add `--quiet`. See
https://www.freedesktop.org/software/systemd/man/latest/systemctl.html#is-active%20PATTERN%E2%80%A6
the puppetlabs/postgresql CI still tests on Puppet 7. yumrepo_core v3.0.0 doesn't support it anymore.
bastelfreak
added a commit
to theforeman/puppet-foreman
that referenced
this pull request
Dec 13, 2025
deric
approved these changes
Dec 16, 2025
smortex
approved these changes
Dec 17, 2025
Collaborator
smortex
left a comment
smortex
left a comment
There was a problem hiding this comment.
LGTM!
Not sure if we should have the last 2 commits merged, but the first one is definitively needed!
Collaborator
Author
|
Yeah, I hope someone fixes the CI and removes the puppet 7 checks soon :( |
Collaborator
I was thinking about clicking the merge button to unbreak the module… Tests are not mandatory for brave people |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replace systemctl
statuswith systemctlis-activesystemctl status $servicewill output a bunch of text. Some of this text, depending on the locale, contains unicode characters:●is unicode. Puppet will fail with:systemctlwill print*if it's executed withLC_ALL=C, but we cannot assume that this locale is always used.The correct command to check if a service is up and running is
is-active, notstatus.statusis for humans only. the systemd provider for the service type also usesis-active. See https://github.com/puppetlabs/puppet/blob/main/lib/puppet/provider/service/systemd.rb#L201-L203Since we only care about the exit code, we can even add
--quiet. See https://www.freedesktop.org/software/systemd/man/latest/systemctl.html#is-active%20PATTERN%E2%80%A6Summary
Ensure the service detection works with all locales.
Additional Context
Add any additional context about the problem here.
Related Issues (if any)
Mention any related issues or pull requests.
Checklist
puppet apply)