refactor(compose): drop the idle cron container, restore the pre_start hook - #79
Conversation
…t hook Two containers existed only to work around tooling. moodle-cron ran `sleep infinity` and did nothing else. Ofelia's job-exec needs a running container, so a service was added purely to be exec'd into - while the application container, which runs anyway, could host the job just as well. The labels move there and the service is gone. job-run would also avoid the idle container, but it needs every volume, network and environment variable repeated in labels: a second copy of the service definition, free to drift from the first. db-init went back to a one-shot service earlier because the compose on the CI runners rejected pre_start. That was the wrong repair - it changed the code to suit an outdated linter. netresearch/.github#420 makes the lint validate against current compose, so the hook returns: compose models init containers as pre_start, the step stays subordinate to the service, and it no longer shows up as a peer in `compose ps`. Neither decision was written down anywhere before; both now carry their reasoning in the file. Verified on a full stack: five containers instead of six, pre_start creates database and user, the Moodle install completes, ofelia registers "moodle.moodle-cron" and runs it to completion (failed: false), 19 scheduled tasks show a lastruntime, and the site answers 200. yamllint and markdownlint are clean. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_01FRHeDxbgUnv868eNhVCCsr Agent-Host: 32116e Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
|
Warning Review limit reachedNext included review available in 19 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.OpenSSF Scorecard
Scanned Files
|
Both compose-using jobs inherited whatever version the runner image ships. That version rejects `pre_start`, so the syntax check failed on a file that is valid, and the stack test would have failed to start it at all. docker/setup-compose-action, SHA-pinned, installs the current release in both jobs. The org-wide lint got the same treatment in netresearch/.github#420. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_01FRHeDxbgUnv868eNhVCCsr Agent-Host: 32116e Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
The previous commit added the action but left out the input, and the job log shows why that was not enough: the step ran, printed docker info and installed nothing, so `docker compose version` still reported the runner's v2.38.2 and the validation failed on pre_start exactly as before. Assisted-by: claude-code:claude-opus-5 Agent-Session: https://claude.ai/code/session_01FRHeDxbgUnv868eNhVCCsr Agent-Host: 32116e Signed-off-by: Sebastian Mendel <info@sebastianmendel.de>
|
|
Copilot is out of quota account-wide, so no bot review can land on this head. CodeRabbit left no inline findings and SonarCloud's quality gate passed. I reviewed the diff myself. The substance was verified on a running stack rather than argued from the file: five containers instead of six, One failure on the way was not mine: Worth recording for the next reader: the two CI commits here exist because the runner's preinstalled compose (v2.38) rejects Self-review: 28108f0 |
1 participant
Merging this removes a container that existed only to be idle, and puts the database bootstrap back where compose says it belongs. The stack runs five containers instead of six, and both decisions finally carry a written reason.
The idle cron container
moodle-cronransleep infinityand nothing else. Ofelia'sjob-execneeds a running container to exec into, so a service was added purely to be that target — while the application container, which runs anyway, can host the job. The labels move there and the service is gone.job-runwould also avoid the idle container, but it needs every volume, network and environment variable repeated in labels: a second copy of the service definition, free to drift from the first.job-execon a container that already exists has no such duplication.The pre_start hook
The bootstrap was written as a
pre_starthook, which is how compose models init containers, and then reverted to a one-shot service because the compose on the CI runners rejected the key. That was the wrong repair — it changed working code to suit an outdated linter. netresearch/.github#420 makes the lint validate against current compose, so the hook returns: the step stays subordinate to the service instead of appearing as a peer incompose ps.Verified
On a full stack, not from the file: five containers instead of six,
pre_startcreates database and user, the Moodle install completes, ofelia registersmoodle.moodle-cronand runs it to completion (failed: false, error: none), 19 scheduled tasks show alastruntime— so Moodle actually processed the runs — and the site answers 200. yamllint and markdownlint report nothing.Note
Neither arrangement was documented anywhere before; the README described that there was a dedicated cron container, never why. Both now carry their reasoning inline, including why
job-runwas not chosen.Assisted by claude-code:claude-opus-5 — Session