-
Notifications
You must be signed in to change notification settings - Fork 108
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
Remove crontab #3369
Merged
Merged
Remove crontab #3369
Conversation
This file contains 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
PBENCH-1086 Use systemd to periodically run the pbench-index service instead of requiring that we generate a crontab file and install components to use it. We add the `sd_notify` service to the server shell to better synchronize with `systemd` so that it knows our service isn't "running" once we've been exec-d, but only when we tell it. I also added some `STATUS` updates, although these don't matter much unless someone is watching with `systemctl status`. The critical bit is that we issue `READY=1` when we want `systemd` to consider the server "active". This triggers the new `pbench-index.timer`, which will begin to run every minute (more or less as with `crontab`). The `sd_notify` delay ensures that the server will have initialized PostgreSQL and Elasticsearch to avoid any conflicts. This introduces the `Type = notify` and `NotifyAccess` settings. (I chose `all` as the `NotifyAccess` because it appears that we're now seeing notifications from `gunicorn` as well, and `all` allows `systemd` to recogize `sd_notify` calls from children of the main service PID instead of writing a warning into the journal.) We use `User = pbench` system services rather than user services principally because the latter would complicate the build and deployment without giving any real benefits. Because `buildah` builds images without an `initd` pid 1, the necessary `loginctl enable-linger pbench` and `systemctl --user` commands aren't available during container build. While I experimented with deferring these until we stand up the real container during deployment, that never felt "clean". Note that I removed the pidfile and kill configuration from the service file. These are actually unnecessary unless the service is doing something unusual: instead we simply tell `systemd` to use `SIGTERM` on the primary service PID. (Though even that isn't strictly necessary as `SIGTERM` is the default.)
dbutenhof
added
Server
Installation
Containerization
Of and relating to the process of setting up and maintaining container images
labels
Apr 4, 2023
webbnh
previously approved these changes
Apr 4, 2023
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.
Looks great! Just small stuff and nits.
webbnh
approved these changes
Apr 4, 2023
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.
👍
ndokos
approved these changes
Apr 5, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
Containerization
Of and relating to the process of setting up and maintaining container images
Installation
Server
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.
PBENCH-1086
Use systemd to periodically run the pbench-index service instead of requiring that we generate a crontab file and install components to use it.
We add the
sd_notify
service to the server shell to better synchronize withsystemd
so that it knows our service isn't "running" once we've been exec-d, but only when we tell it. I also added someSTATUS
updates, although these don't matter much unless someone is watching withsystemctl status
. The critical bit is that we issueREADY=1
when we wantsystemd
to consider the server "active". This triggers the newpbench-index.timer
, which will begin to run every minute (more or less as withcrontab
). Thesd_notify
delay ensures that the server will have initialized PostgreSQL and Elasticsearch to avoid any conflicts. This introduces theType = notify
andNotifyAccess
settings. (I choseall
as theNotifyAccess
because it appears that we're now seeing notifications fromgunicorn
as well, andall
allowssystemd
to recogizesd_notify
calls from children of the main service PID instead of writing a warning into the journal.)We use
User = pbench
system services rather than user services principally because the latter would complicate the build and deployment without giving any real benefits. Becausebuildah
builds images without aninitd
pid 1, the necessaryloginctl enable-linger pbench
andsystemctl --user
commands aren't available during container build. While I experimented with deferring these until we stand up the real container during deployment, that never felt "clean".Note that I removed the pidfile and kill configuration from the service file. These are actually unnecessary unless the service is doing something unusual: instead we simply tell
systemd
to useSIGTERM
on the primary service PID. (Though even that isn't strictly necessary asSIGTERM
is the default.)