Process lifecycle fixes and a running-config dump - #208
Draft
mbertheau wants to merge 13 commits into
Draft
Conversation
make systest-on-podman runs the system tests in a reusable Fedora container so they do not depend on the host's journald, rsyslogd, or AppArmor. make systest writes to the host journal and starts a second rsyslogd with a config under pytest's /tmp. On Ubuntu, AppArmor denies that open. CI already unloads that profile when GITHUB_ACTIONS is set and leaves every other Ubuntu host failing. Tests that only read recorded journal files pass there; the rsyslog sender tests do not. A privileged container and a first-run dnf/pip are the cost of leaving make systest and CI alone. The container is stopped, not deleted, so later runs skip the install. fedora:latest is taken at create time; a leftover container from another tag is replaced. journald is started each run because PID 1 is sleep. podman rm journalpump-systest forces a rebuild if requirements change. Co-authored-by: Cursor <cursoragent@cursor.com>
Construction never enters configure_readers' teardown loop:
JournalPump.__init__ sets self.readers to {} first. Only a process
that already has readers, then gets SIGHUP after the readers object
changes, reaches reader.unregister_from_poll and dies.
This test starts python -m journalpump, waits for systemd READY=1,
rewrites the reader names, and sends SIGHUP. After SIGHUP it
expects RELOADING=1 then READY=1. It fails today with
AttributeError at configure_readers, which is the point:
ServiceDaemon.main does not catch that exception, so the daemon
exits.
An in-process os.kill(getpid(), SIGHUP) would raise too, but would
not show the process-level death operators see.
Co-authored-by: Cursor <cursoragent@cursor.com>
shutdown() unregisters every reader; the run loop then calls _close_stale_readers, which unregisters again. The second call raises KeyError, the process exits 1, and systemd records a failed unit. A SIGTERM that lands in poll() does not hit this: the loop purges the stale fd and registers the reader again. This test holds the WatchdogSec notify so stop() arrives while ping_watchdog is still on the stack, after the first iteration has registered the reader. It fails today with KeyError in unregister_from_poll. Co-authored-by: Cursor <cursoragent@cursor.com>
setup.py points journalpump at journalpump.__main__:main, but that name did not exist. The installed command worked only because importing the module ran JournalPump.run_exit() as a side effect and sys.exit() aborted the wrapper before it looked up main. python -m journalpump kept working; deleting the side effect without adding main would have broken the installed command. Co-authored-by: Cursor <cursoragent@cursor.com>
ServiceDaemon.__init__ loads config and installs SIGHUP/SIGTERM handlers before JournalPump finished constructing. A signal in that window ran shutdown or configure_readers against attributes that did not exist yet. First-start configure_readers skipped those attributes because there were no old readers. SIGTERM did not: it always walked the new readers and called stale_readers.add. Co-authored-by: Cursor <cursoragent@cursor.com>
configure_readers reads both from the process config, not from each reader. A per-reader setting had no effect. msg_buffer_max_bytes was not documented at all. Co-authored-by: Cursor <cursoragent@cursor.com>
SIGHUP used to call reload_config() in the handler, which rebuilt readers and senders under whatever the loop was doing. A JSON error there escaped into the middle of that iteration and killed the process. The handler now only sets a flag. run() applies it at the start of each iteration. A broken config file stays a log line. Co-authored-by: Cursor <cursoragent@cursor.com>
JournalPump.sigterm used to save state, stop senders, and unregister poll fds in the handler, then let the current iteration keep reading and queuing. The inherited handler already sets running = False. That is enough. shutdown and the stale-reader close now run from cleanup, which main() calls after run() returns. Co-authored-by: Cursor <cursoragent@cursor.com>
JournalPump.configure_readers already picks the resume cursor from saved sender state. This method duplicated that logic, including the unfinished "pick oldest cursor" note, and had no callers. Co-authored-by: Cursor <cursoragent@cursor.com>
get_reader was only called to create a missing journald reader. reinit=True was never passed, so the branch that closed and rebuilt an existing reader could not run. Co-authored-by: Cursor <cursoragent@cursor.com>
A missing json_state_file_path means journalpump does not persist cursors. The README said the default was journalpump_state.json in the working directory. Someone who omitted the key would expect positions to survive a restart. Co-authored-by: Cursor <cursoragent@cursor.com>
The input JSON can change on disk after load. After each successful config apply, journalpump writes the dict it is using to $RUNTIME_DIRECTORY/config.json. The shipped unit now sets RuntimeDirectory=journalpump, which exports RUNTIME_DIRECTORY=/run/journalpump. When that variable is unset, journalpump uses /run/journalpump so a non-systemd start still has a well-known path. json_running_config_path overrides both. A write failure is logged and does not stop the daemon. Co-authored-by: Cursor <cursoragent@cursor.com>
mbertheau
force-pushed
the
mbertheau-misc-fixes
branch
from
September 8, 2026 07:11
b19f6e3 to
9db0c3e
Compare
handle_new_config dumps self.config as JSON. Compiling each secret_filter's pattern in place put a _Regexp on that dict, so the dump raised TypeError and the runtime config.json was never written. Production reader configs include secret_filters. Compile onto copies instead. The loaded config stays the JSON that was read. Co-authored-by: Cursor <cursoragent@cursor.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
This is one PR of otherwise separate journalpump process fixes. They are individually very small changes. I decided to avoid the overhead of opening 10 separate PRs for them for now.
SIGHUP used to call
reload_configin the signal handler. A JSON error there killed the process. The handler now sets a flag; the run loop applies it. SIGTERM only stops the loop.shutdownand the stale-reader close run fromcleanupafterrun()returns.ServiceDaemon.__init__used to install those handlers beforeJournalPumphadstale_readersandreader_by_fd. A signal in that window crashed on attributes that did not exist yet.After a successful apply, journalpump writes the config it is using to
$RUNTIME_DIRECTORY/config.json, or/run/journalpump/config.jsonif systemd did not set the variable. The shipped unit setsRuntimeDirectory=journalpump. The input file is not that record: a failed reload leaves the process on the previous apply.The console script entry point had no
main(). The installed command only worked because import ranrun_exit()as a side effect.get_resume_cursorandget_reader(reinit=...)had no callers.make systest-on-podmanruns the system tests in a Fedora container to be independent of local system configuration, which, on Ubuntu for example, needs AppArmor massaging. The SIGHUP and SIGTERM tests startpython -m journalpumpand assert the process stays up or exits 0.The README no longer claims
json_state_file_pathdefaults tojournalpump_state.json. A missing key means no cursor file.msg_buffer_max_lengthandmsg_buffer_max_bytesare documented as top-level, which is whereconfigure_readersreads them.Made with Cursor