Say that report-only mode makes every merge a manual apply (#99) - #275
Merged
Conversation
…pply (#99) The report-only section explained what the mode does and how to leave it, and never said what it costs while it is on: nothing is applied, so a merge lands on disk and reaches no running process. That is easy to miss precisely because a merge in this mode LOOKS like it landed. `git pull` moves the checkout, the files are there, and the containers bind-mount the directories those files came from. Nothing has re-read them. It caught this change out on its first day, which is why the note is a worked example rather than a caution. The merge installing the agent also added deploy.rules.yaml, so Prometheus ran with 48 alerting rules where the repository had 53 — and the five missing ones were the rules that watch convergence, DeployApplyDisabled among them. The mode had disabled the alerting that reports the mode, and nothing said so. `make reload` fixed it in one hot reload. Also records the trap underneath it: Prometheus re-reads rule files at startup and on POST /-/reload and at nothing else, while Loki polls its rule directory and updates on its own. So Loki being current is actively misleading evidence about Prometheus, and #98's Loki rules did arrive by themselves on the same day Prometheus's did not. deploy-stack.md §Updating gets the same correction. It opens "**Normally you do not.** A merged pull request reaches this host within the hour on its own", which is false while report-only is set — and that is the document someone reads when they want to know whether they still have to deploy. The verification command is `jq`, not `grep -c`, and that is not stylistic: the rules API returns the whole document on one line, so `grep -c` counts the line and answers 1 however many rules are loaded. Both forms were run against the live stack; the first draft of this runbook shipped the broken one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
🟡 Changes recommended
One newly added verification command is misleading because it outputs per-file counts rather than a single total comparable to the API-derived count.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR updates runbooks to explicitly document the operational cost of running host convergence in report-only mode: merges can land on disk without being applied to running services, so operators must manually apply/reload to make changes take effect.
Changes:
- Clarifies in
converge-the-host.mdthat report-only mode requires a manual apply/reload for every merge, and explains why a merge can look “landed” while nothing reloaded. - Updates
deploy-stack.mdto correct the “Normally you do not [deploy]” guidance when the host is in report-only mode.
File summaries
| File | Description |
|---|---|
| docs/runbooks/deploy-stack.md | Adds an explicit exception to “normally you do not deploy” when the host is in report-only mode. |
| docs/runbooks/converge-the-host.md | Adds a worked example, reload guidance, and a verification command to detect Prometheus rule drift while in report-only mode. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| > curl -s localhost:9090/api/v1/rules | jq '[.data.groups[].rules[] | select(.type=="alerting")] | length' | ||
| > ``` | ||
| > | ||
| > Compare it against `grep -c '^ - alert:' stacks/observability/prometheus/rules/*.rules.yaml`. |
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
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.
Follow-up to #273, from a gap that change hit on its first day.
What was missing
converge-the-host.mdexplained what report-only mode does and how to leave it, and never said what it costs while it is on: nothing is applied, so a merge lands on disk and reaches no running process.That is easy to miss because a merge in this mode looks like it landed —
git pullmoves the checkout, the files are on disk, and the containers bind-mount the directories those files came from. Nothing has re-read them.The worked example
The merge that installed the agent also added
deploy.rules.yaml. Prometheus ran with 48 alerting rules where the repository had 53, and the five missing ones were the rules that watch convergence —DeployApplyDisabledamong them.The mode had silently disabled the alerting that reports the mode.
make reloadfixed it in one hot reload, no container restart.The trap underneath
Prometheus re-reads rule files at startup and on
POST /-/reload, and at nothing else. Loki polls its rule directory and updates on its own — so Loki being current is actively misleading evidence about Prometheus. On the day in question #98's Loki rules did arrive by themselves while Prometheus's did not.Also fixed
deploy-stack.md§Updating opens "Normally you do not. A merged pull request reaches this host within the hour on its own" — false while report-only is set, and that is exactly the document someone reads to find out whether they still have to deploy.On the verification command
It is
jq, notgrep -c, and that is not stylistic: the rules API returns the whole document on one line, sogrep -ccounts that line and answers1however many rules are loaded. Both forms were run against the live stack — the first draft of this note shipped the broken one, and testing it is why this saysjq.Docs only;
make validatepasses.🤖 Generated with Claude Code