SEP-1970: Add clear_sentinels.sh for supervisorctl-driven schema re-runs

[yyyyyyyan]

Summary

Adds sidecar/clear_sentinels.sh, an operator-invoked script that invalidates named schema steps' sentinels ahead of a supervisorctl re-run, and documents the one supported sequence for re-applying a migration inside a running container.

entrypoint.sh (PID 1) clears all four /tmp/migrate-<step>.ok markers before exec supervisord, which covers every container start and restart. A supervisorctl restart never re-enters PID 1: each one-shot removes its own marker only as the first word of its own sh -c command, i.e. after it has been spawned, concurrently with a restarted API's wait_for_schema.sh gate. The gate could therefore observe the previous run's marker and release its app against a schema this run has not re-applied. (Inferred from code inspection — not reproduced or QA-observed.)

• sidecar/clear_sentinels.sh (new, mode 100755; copied into the image's app directory beside wait_for_schema.sh with --chmod=550, as the gate is). Takes one or more bare schema-step names, the same convention wait_for_schema.sh takes. It validates every argument against its own SCHEMA_STEPS list before removing anything, then removes each named marker and prints what it cleared. An unrecognized name exits 2 having removed nothing; an empty argument list exits 2 with a usage: line. Names are matched whole, so a program name (migrate-sep), a two-word argument ("sep inventory"), an empty argument and a path-like one (../sep) are all refused before any path is built.
• sidecar/README.md gains a "Re-running a schema step inside a running container" subsection carrying the sequence (clear the step's sentinel, then restart the one-shot together with the API programs), what the clear's exit status obliges the operator to do — including the recovery when an rm fails partway — why the container reads unhealthy until the step republishes, and which supervisorctl output is expected noise (and why supervisorctl's own exit status is not the verdict either: the start half faults on the very ERROR (abnormal termination) line the operator is told to ignore). It also documents Celery separately: celery-worker carries no schema gate and celery-beat is gated only on the APIs answering, so when the re-run step owns tables Celery reads, both are stopped before the clear and started after the sentinel returns — naming them in the restart call would start them roughly 24s in (supervisord flips a gated API to RUNNING once its startsecs elapses, while its shell is still inside the gate) and the ungated worker would consume tasks against the incomplete schema. Also adds the script to the "What it contains" table and qualifies the Health paragraph that previously said flatly that "a restart cannot release a gate on the previous run's markers" — true of a container restart, not of a supervisorctl one.
• .github/workflows/ci.yml adds sidecar/clear_sentinels.sh to the python: path filter, and an executable-bit assertion for it to the image smoke test. The filter already enumerates the other six sidecar files the sidecar suite asserts on, precisely because a skipped job counts as green in ci-success; without the entry, a future PR editing only this shell file would merge with the 18 new tests unrun.
• Comments in sidecar/entrypoint.sh and sidecar/supervisord.conf no longer state that the supervisorctl path merely races; both now name the script and the documented sequence.
• tests/sidecar/test_clear_sentinels.py (new, 18 cases) runs the script directly against a uuid-prefixed copy of itself: argument validation, subset clearing, idempotence, a repeated step, an rm that cannot succeed, the image COPY and its mode, and a control/experiment pair showing that a stale sentinel releases a restarted gate while a cleared one holds it on exactly that step.
• tests/sidecar/conftest.py now owns the constants the sidecar test modules shared by hand (CONTAINERFILE, ENTRYPOINT, GATE, SCHEMA_STEPS, SENTINEL_PREFIX, BUDGET_ASSIGNMENT); test_entrypoint.py, test_wait_for_api.py and test_wait_for_schema.py import them instead of redeclaring them. Imports only, plus one fixture docstring in test_wait_for_schema.py whose enumeration of where the sentinel path is hardcoded would otherwise have become incomplete — no test body changed.

PID 1's clearing is unchanged. entrypoint.sh's rm -f lines are byte-identical and no supervisord.conf command= line was touched; the only edits to those two files are comments. The change is otherwise additive: a new file in the image, new documentation, and new tests. No settings, API, schema or task payload is affected.

Deviation from the ticket's Implementation Notes

The notes (explicitly "suggestions … not requirements") proposed extracting the sentinel step list into one place both entrypoint.sh and the new script could read. This ships the script with its own literal list plus a test pinning it to the supervisord program table instead — the same guard every other step-list site in the side-car already has (healthcheck.sh's list, the three gate invocations, and the entrypoint's own four paths are each pinned this way). A shared list could not actually be one place: the seven supervisord.conf sites are sh -c '…' strings inside an ini file the ticket marks unchanged, so extracting would unify two or three of nine sites while the rest stayed test-guarded anyway, and it would put a second shipped file on PID 1's errexit path. This was put to the maintainer as a four-way choice during planning and this option was selected.

Tested

Automated coverage lives in tests/sidecar/test_clear_sentinels.py and is not repeated here. The scenarios below need a running side-car container, which could not be built in the implementing environment, so they are described rather than spelled out as command lines — the exact commands are in the "Re-running a schema step inside a running container" section of sidecar/README.md.

• In a running side-car container, clearing the sep step through the new script exits 0, reports the step it cleared, and leaves the other three markers in /tmp untouched.
• Passing the supervisord program name migrate-sep instead of the bare step name exits non-zero, names the accepted step names, and removes no marker.
• After clearing sep and restarting migrate-sep together with the three API programs, the stop half reports migrate-sep: ERROR (not running) for the already-exited one-shot without changing the command's exit status; sep's sentinel reappears once the step finishes, and all three /health endpoints answer 200.
• While the sep sentinel is absent the container reports unhealthy, and it returns to healthy once the step republishes the marker.
• A full container restart still brings the container up healthy, confirming the PID 1 clearing path is unaffected.
• With the Celery programs stopped for the re-run and started once the sentinel returns, no task is lost (the broker holds them) and the worker comes back against the re-applied schema.

Known minor issues

• The new suite anchors its markers under the real /tmp (uuid-scoped: /tmp/migrate-<uuid>-<step>.ok) rather than under tmp_path. Isolation is airtight regardless — prefixed_copy asserts the shipped script contains exactly one /tmp/migrate- occurrence before substituting, so a copy can never retain the real prefix, and teardown only unlinks the four known paths under its own prefix. Moving to tmp_path would remove the shared-namespace dependency but changes nothing about correctness, so it was left as the plan specified.

Checklist

• New/modified functions have type hints and rST docstrings
• New tests added for new features or bug fixes
• Database migrations generated if models changed (make makemigrations) — N/A, no models changed
• User-facing changes documented (README, inline help, UI text)
• Configuration changes documented with examples — N/A, no configuration changed

[Copilot]

🟡 Changes recommended

The operational documentation incorrectly describes how an expected supervisorctl stop error affects the command’s exit status.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Adds a safe operator workflow for rerunning side-car schema steps without stale sentinels releasing API gates.

Changes:

• Adds validated sentinel clearing and container packaging.
• Documents the rerun and Celery coordination procedure.
• Adds comprehensive tests and CI coverage.

File summaries

File	Description
sidecar/clear_sentinels.sh	Validates schema steps and removes their sentinels.
sidecar/README.md	Documents the supported rerun procedure.
sidecar/Containerfile.sidecar	Installs the executable helper.
sidecar/entrypoint.sh	References the new manual workflow.
sidecar/supervisord.conf	Clarifies restart sentinel handling.
.github/workflows/ci.yml	Adds CI triggering and image smoke coverage.
changelog.d/SEP-1970.added.md	Records the operator-facing feature.
tests/sidecar/conftest.py	Centralizes shared side-car test constants.
tests/sidecar/test_clear_sentinels.py	Tests validation, removal, gating, and packaging.
tests/sidecar/test_entrypoint.py	Reuses shared constants.
tests/sidecar/test_wait_for_api.py	Reuses the shared Containerfile path.
tests/sidecar/test_wait_for_schema.py	Reuses shared schema-gate constants.

Review details

• Files reviewed: 12/12 changed files
• Comments generated: 1
• Review effort level: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[yyyyyyyan]

Automated QA — PASS

Built the side-car image from the PR head commit and ran it against a real Postgres + PMM instance, then exercised the documented "re-running a schema step inside a running container" sequence directly:

• Clearing the sep sentinel through the script exits 0, reports what it cleared, and leaves the other three untouched. clear_sentinels.sh sep → exit 0, [clear_sentinels] cleared: sep; /tmp/migrate-{beat,inventory,tasks}.ok still present.

• Passing the supervisord program name migrate-sep instead of the bare step name is rejected. clear_sentinels.sh migrate-sep → exit 2, expected one of: sep inventory tasks beat, no marker removed.

• The documented restart sequence behaves exactly as described. supervisorctl restart migrate-sep sep inventory tasks printed migrate-sep: ERROR (not running) on the stop half without changing the command's exit status (0); sep's sentinel reappeared within a couple of seconds; all three API health endpoints came back 200.

  
  
  

• The container reports unhealthy while the sentinel is absent, and recovers once it republishes. Docker's aggregate health flipped to unhealthy roughly 75 seconds after the clear (5 consecutive failed probes at a 15s interval), matching the documented window, then returned to healthy once the sentinel came back and Celery was restarted.

• A full container restart still comes up healthy, confirming the PID 1 clearing path is unaffected by the new script. docker restart re-created all four sentinels through entrypoint.sh, and all three API programs and health endpoints came back up cleanly.

• No task is lost while Celery is stopped for the re-run. Stopped celery-worker/celery-beat, queued a task, and confirmed it sat in the broker (LLEN celery = 1) for the duration of the clear+restart. After starting Celery back up, the worker consumed and completed the task against the re-applied schema (LLEN back to 0; task logged as succeeded).

[github-actions]

Coverage report

This PR does not seem to contain any modification to coverable code.