Repository navigation
pm-stack(drills): kill-collector chaos drill, first slice of #1283 - #1823
Conversation
scripts/kill-collector-drill.sh stops the sandbox fleet-feed collector, polls the aggregator /api/health until the feed age crosses FEED_STALE_MS (15 min, the #1148 watchdog threshold), then always restores (trap + explicit restore + immediate collection run) and confirms the feed is fresh again. Interlocks: flock against overlap, incident-flag refusal, and refusal to start on an already-stale feed (a real incident is not a drill canvas). A passed drill comments on #1283; a drill whose signal does not fire files a finding issue under epic #1184 with its evidence. systemd service/timer units ship in pm-stack/aggregator/systemd in the collector units' style (weekly, flock prevents overlap).
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 28 minutes. View limit details
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 |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b339aef3c0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| sudo -n systemctl start "$COLLECTOR_TIMER" 2>/dev/null || log "WARN: could not start $COLLECTOR_TIMER" | ||
| sudo -n systemctl start "$COLLECTOR_SERVICE" 2>/dev/null || log "WARN: could not start $COLLECTOR_SERVICE" | ||
| } | ||
| trap restore EXIT INT TERM |
There was a problem hiding this comment.
If SIGTERM or SIGINT arrives during the preconditions, the handler restores the collector and sets RESTORED=1, but then returns instead of terminating the script. Execution subsequently stops the timer and service, while both the explicit restore and EXIT handler skip restoration because that flag is already set. An isolated mock reproduction confirmed the sequence of starts followed by stops with no later restart, leaving the collector disabled. Use termination handlers that exit after cleanup rather than resuming the drill.
Useful? React with 👍 / 👎.
| if [ "$AGE" -gt "$STALE_MS" ]; then | ||
| SIGNAL=1 |
There was a problem hiding this comment.
When the feed timestamp becomes old but the aggregator incorrectly continues reporting healthy services and degraded=false, this condition still sets SIGNAL=1: it checks only the age calculated locally by feed_facts, not the aggregator's response to staleness. A mocked run with exactly that payload completed with outcome=passed. This means the drill passes for the silent-health regression it is intended to detect. Require the relevant feed-backed health states and degradation signal to reflect staleness in the captured response before declaring success.
Useful? React with 👍 / 👎.
| sudo -n systemctl start "$COLLECTOR_TIMER" 2>/dev/null || log "WARN: could not start $COLLECTOR_TIMER" | ||
| sudo -n systemctl start "$COLLECTOR_SERVICE" 2>/dev/null || log "WARN: could not start $COLLECTOR_SERVICE" | ||
| } | ||
| trap restore EXIT INT TERM |
There was a problem hiding this comment.
If the timer is inactive, the health endpoint is unreadable, or the feed is already stale, the precondition exits through die, but this already-installed EXIT trap starts both the timer and collector anyway. A mocked inactive-timer run confirmed both start commands despite reporting refusal. This violates the stated no-touch precondition behavior and can restart a deliberately stopped collector or alter a real incident before any drill action occurred. Gate cleanup on whether this run has begun stopping the collector, or arm it immediately before that operation after all preconditions pass.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f73e1eb951
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if [ "$AGE" -ge 0 ] && [ "$AGE" -le "$STALE_MS" ]; then | ||
| CONFIRMED=1 |
There was a problem hiding this comment.
If restarting the collector timer fails but the immediate service run succeeds, this freshness check declares restoration successful even though recurring collection remains stopped. An isolated mock reproduced the timer-start warning followed by outcome=passed and exit status 0. The one-off refresh only temporarily masks the outage because fleet-feed-collector.timer supplies the recurring collection cadence. Require successful timer restoration before setting CONFIRMED=1, and retry failed restoration; systemctl --help documents is-active as “Check whether units are active.”
Useful? React with 👍 / 👎.
| sleep 60 | ||
| WAITED=$((WAITED + 60)) | ||
| FACTS="$(health_json | feed_facts)" |
There was a problem hiding this comment.
If /api/health starts timing out after the collector is stopped, WAITED excludes the time spent fetching health. Each health_json call can consume 90 seconds across three 25-second transfers and retry sleeps (curl --help all describes --max-time as “Maximum time allowed for transfer”). Consequently, the default 25-minute limit permits roughly 62.5 minutes before restoration, exceeding the 45-minute TimeoutStartSec in systemd/kill-collector-drill.service and reaching service termination before the normal failure report. An accelerated mock confirmed that a one-minute limit waits 150 seconds before restoration. Use an elapsed-time deadline that includes requests and retries.
Useful? React with 👍 / 👎.
…core-1823 Rename fleet-built providers to leading-underscore ids (#1823)
First slice of #1283 (epic #1184), queue task 2667: the kill-collector chaos drill.
What
pm-stack/aggregator/scripts/kill-collector-drill.sh— stops the sandboxfleet-feed-collector(the fleet feed's sole writer, ~5 min cadence), polls the aggregator's/api/healthuntil the feed age crossesFEED_STALE_MS(15 min — the impl(fleet): staleness watchdog - surface dead/stale stack inputs as NEEDS ORCHESTRATOR instead of silent decay #1148 watchdog threshold insrc/health.ts/src/sources/live.ts), captures the health payload at the signal moment, then always restores: trap on EXIT/INT/TERM plus an explicit restore that restarts the timer and runs the collector immediately, with a post-drill health read confirming the feed is fresh again.flock(no overlap, manual or scheduled), an incident-flag file that refuses the drill during a real incident, and a start precondition that the feed is currently fresh — an already-stale feed is a real incident, not a drill canvas. Health reads retry (the aggregator's first health assembly can be slow to prime; a drill never acts on one timed-out read — the first live attempt refused safely on exactly this before the retry was added).~/.local/state/pm-drills/; a passed drill comments its result on design(pm-stack): chaos/rotation drills — scheduled game-days (kill collector, age the feed, drop a service) verifying banners, watchdogs, and self-healing fire #1283; a drill whose signal does not fire files a finding issue under epic epic(dashboard): PM stack coherence - single-URL, fresh data, triage-able, hierarchical #1184 with the drill evidence.pm-stack/aggregator/systemd/kill-collector-drill.{service,timer}— weekly (Sun 04:17 + randomized delay), in the same style as the collector's own units. Post-merge step: install the units on the sandbox and enable the timer once this script has landed in the feed clone the unit's ExecStart points at.Live verification (2026-10-28, from this branch's script)
~/.local/state/pm-drills/kill-collector-health-at-signal.json.passedposted on design(pm-stack): chaos/rotation drills — scheduled game-days (kill collector, age the feed, drop a service) verifying banners, watchdogs, and self-healing fire #1283 by the drill itself.Refs #1283.