-
Notifications
You must be signed in to change notification settings - Fork 6
systemd: Use restart counter to determine if service is started #42
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
base: main
Are you sure you want to change the base?
Conversation
This now uses the same unit files as debian.org in their packages. The restart counter is used to check if the service is already started properly or not. The legacy wrapper scripts from Perforce had the same logic.
|
The unit file on EL8: # /usr/lib/systemd/system/puppetserver.service
#
# Local settings can be configured without being overwritten by package upgrades, for example
# if you want to increase puppetserver open-files-limit to 10000,
# you need to increase systemd's LimitNOFILE setting, so create a file named
# "/etc/systemd/system/puppetserver.service.d/limits.conf" containing:
# [Service]
# LimitNOFILE=10000
# You can confirm it worked by running systemctl daemon-reload
# then running systemctl show puppetserver | grep LimitNOFILE
#
[Unit]
Description=puppetserver Service
After=syslog.target network.target nss-lookup.target
[Service]
Type=exec
LogsDirectory=puppetlabs/puppetserver
RuntimeDirectory=puppetlabs/puppetserver
EnvironmentFile=/etc/sysconfig/puppetserver
User=puppet
TimeoutStartSec=300
TimeoutStopSec=60
Restart=on-failure
StartLimitBurst=5
PrivateTmp=true
# https://tickets.puppetlabs.com/browse/EZ-129
# Prior to systemd v228, TasksMax was unset by default, and unlimited. Starting in 228 a default of '512'
# was implemented. This is low enough to cause problems for certain applications. In systemd 231, the
# default was changed to be 15% of the default kernel limit. This explicitly sets TasksMax to 4915,
# which should match the default in systemd 231 and later.
# See https://github.com/systemd/systemd/issues/3211#issuecomment-233676333
TasksMax=4915
#set default privileges to -rw-r-----
UMask=027
# RUNTIME_DIRECTORY is set by systemd
# https://www.freedesktop.org/software/systemd/man/latest/systemd.exec.html#RuntimeDirectory=
ExecStartPre=sh -c "echo -n 0 > ${RUNTIME_DIRECTORY}/restart"
ExecStart=/usr/lib/jvm/jre-17/bin/java $JAVA_ARGS -Dlogappender=F1 \
'-XX:OnOutOfMemoryError=kill -9 %p' -XX:+CrashOnOutOfMemoryError \
-XX:ErrorFile="${LOGS_DIRECTORY}/puppetserver_err_pid%p.log" \
-cp "${INSTALL_DIR}/puppet-server-release.jar" \
clojure.main \
-m puppetlabs.trapperkeeper.main \
--config "${CONFIG}" \
--bootstrap-config "${BOOTSTRAP_CONFIG}" \
--restart-file ${RUNTIME_DIRECTORY}/restart \
$TK_ARGS
KillMode=process
# wait until the service is actually up
ExecStartPost=sh -c "sleep 1; while ! head -c1 ${RUNTIME_DIRECTORY}/restart | grep -q '^1'; do kill -0 $MAINPID && sleep 1 || exit 1; done"
ExecReload=sh -c "echo -n 0 > ${RUNTIME_DIRECTORY}/restart"
ExecReload=kill -HUP $MAINPID
ExecReload=sh -c "while ! head -c1 ${RUNTIME_DIRECTORY}/restart | grep -q '^1'; do kill -0 $MAINPID && sleep 1 || exit 1; done"
SuccessExitStatus=143
[Install]
WantedBy=multi-user.target |
|
I gave this a quick local test and it's working fine. However I have no time right now to kick of the acceptance tests with that. It would be great if a few people could test this and provide feedback. |
| <% end -%> | ||
|
|
||
| <% unless EZBake::Config[:debian][:pre_start_action].empty? -%> | ||
| PermissionsStartOnly=true |
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.
This would run the ExecStartPre command as root and could result into permission errors, so I removed it
|
|
||
| <% unless EZBake::Config[:debian][:pre_start_action].empty? -%> | ||
| PermissionsStartOnly=true | ||
| <% EZBake::Config[:debian][:pre_start_action].each do |action| -%> |
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.
We don't use those hooks for openvox-server/db, so I removed them to make the whole template more readable.
austb
left a comment
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.
One question, but does not need to change.
👍🏻 LGTM
|
|
||
| ExecReload=sh -c "echo -n 0 > ${RUNTIME_DIRECTORY}/restart" | ||
| ExecReload=kill -HUP $MAINPID | ||
| ExecReload=sh -c "while ! head -c1 ${RUNTIME_DIRECTORY}/restart | grep -q '^1'; do kill -0 $MAINPID && sleep 1 || exit 1; done" |
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.
Was omitting an initial sleep here intentional? I don't think it matters too much because checking the file contents is a cheap operation and then it will sleep anyways, the difference just caught my eye.
|
|
||
| ExecReload=sh -c "echo -n 0 > ${RUNTIME_DIRECTORY}/restart" | ||
| ExecReload=kill -HUP $MAINPID | ||
| ExecReload=sh -c "while ! head -c1 ${RUNTIME_DIRECTORY}/restart | grep -q '^1'; do kill -0 $MAINPID && sleep 1 || exit 1; done" |
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.
same as above, just marking this in case any changes are made.
This now uses the same unit files as debian.org in their packages. The restart counter is used to check if the service is already started properly or not. The legacy wrapper scripts from Perforce had the same logic.