diff --git a/Makefile b/Makefile index 676f59b..6596270 100644 --- a/Makefile +++ b/Makefile @@ -20,7 +20,23 @@ help: ## Show this help .PHONY: up up: render ## Render config and start the stack $(COMPOSE) up -d --remove-orphans - @printf '\n\033[0;32mup\033[0m — Grafana: http://localhost:$${GRAFANA_PORT:-3000}\n' + @# `up -d` recreates a container only when its *service definition* changes, + @# so a freshly rendered snmp.yaml or an edited prometheus.yaml is invisible + @# to it and the container keeps serving what it parsed at startup. Without + @# this line `make up` reports success over a stale config — see + @# scripts/reload-config.sh for the incident that produced it. + ./scripts/reload-config.sh $(STACK) + @# The port is read back out of the rendered .env rather than expanded here. + @# GRAFANA_PORT lives in $(STACK_DIR)/.env, which docker compose reads and make + @# does not, so a bare $${GRAFANA_PORT:-3000} in a recipe yields 3000 whatever + @# the operator actually set — a wrong URL that looks right. This line was + @# previously inside a single-quoted printf format, so it did not expand at + @# all and printed the literal text `$${GRAFANA_PORT:-3000}`: the same bug, + @# just honest about it. Passed as a %s argument, not interpolated into the + @# format, so a stray % in the value cannot be read as a format spec. + @# tail -1 because compose takes the last of duplicate keys. + @port="$$(grep -E '^GRAFANA_PORT=' $(STACK_DIR)/.env 2>/dev/null | tail -1 | cut -d= -f2-)"; \ + printf '\n\033[0;32mup\033[0m — Grafana: http://localhost:%s\n' "$${port:-3000}" .PHONY: down down: ## Stop the stack (volumes are preserved) @@ -43,20 +59,10 @@ logs: ## Tail logs (SERVICE=grafana to narrow) .PHONY: reload reload: ## Hot-reload Prometheus, Alertmanager and snmp-exporter (no restart) - $(COMPOSE) exec prometheus wget -q -O- --post-data='' http://localhost:9090/-/reload - $(COMPOSE) exec alertmanager wget -q -O- --post-data='' http://localhost:9093/-/reload - @# snmp-exporter reads its config once at startup, but v0.30.1 also serves an - @# unconditional POST /-/reload — no --web.enable-lifecycle gate, unlike - @# Prometheus. That is what lets an SNMP community rotation be - @# `make render && make reload` rather than a container recreate. - @# - @# It works only because render-config.sh writes the rendered file with `>`, - @# truncating in place and keeping the inode. The container bind-mounts the - @# file, not the directory, so switching to write-temp-then-mv would leave - @# the mount pointing at the old inode and this reload would cheerfully - @# re-read the old community. - $(COMPOSE) exec snmp-exporter wget -q -O- --post-data='' http://localhost:9116/-/reload - @printf '\033[0;32mreloaded\033[0m\n' + @# `make up` runs this too. It stays a separate target because a config-only + @# change — an SNMP community rotation, an Alertmanager route edit — needs + @# only `make render && make reload`, with no compose round trip. + ./scripts/reload-config.sh $(STACK) .PHONY: nuke nuke: ## Stop the stack AND delete its volumes (destroys all metrics and logs) diff --git a/docs/observability.md b/docs/observability.md index 5bbdd97..98efcf1 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -157,7 +157,7 @@ after being converted to query parameters, so they never become metric labels. make up # render secrets, start everything make ps # container status make logs SERVICE=grafana # tail one service -make reload # hot-reload Prometheus + Alertmanager config +make reload # hot-reload Prometheus, Alertmanager, snmp-exporter make validate # everything CI runs make backup # tar the data volumes into ./backups/ make down # stop, keep data @@ -166,4 +166,10 @@ make nuke # stop, destroy data (prompts) Prometheus and Alertmanager are started with lifecycle endpoints enabled, so rule and route changes apply via `make reload` without dropping the TSDB head -block. +block. snmp-exporter serves `POST /-/reload` unconditionally, with no lifecycle +flag to enable. + +`make up` runs that same reload as its last step. It has to: `docker compose up +-d` keys off the service definition, not the contents of the files it mounts, +so without the reload a re-rendered config would sit on disk while the +container served the copy it parsed at startup. diff --git a/docs/runbooks/deploy-stack.md b/docs/runbooks/deploy-stack.md index ab05311..55ca61b 100644 --- a/docs/runbooks/deploy-stack.md +++ b/docs/runbooks/deploy-stack.md @@ -61,14 +61,19 @@ Then in the UI: ```bash git pull make validate -make up # recreates only what changed +make up # recreates changed services, then reloads config on the rest ``` -Config-only changes to Prometheus rules, Alertmanager routing or the rendered -snmp-exporter config do not need a restart: +`make up` recreates a container only when its *service definition* changes — a +changed bind-mounted config file is invisible to `docker compose up -d`. So +`make up` finishes by reloading Prometheus, Alertmanager and snmp-exporter from +disk, and fails if any of them will not take the new config. + +A config-only change — Prometheus rules, Alertmanager routing, the rendered +snmp-exporter config — needs no compose round trip at all: ```bash -make reload +make render && make reload ``` ## Rolling back @@ -93,6 +98,7 @@ and it prompts. | Grafana panels empty, no error | Datasource UID mismatch | `make check-dashboards` | | Loki `ready` returns 503 for a while | Normal on first start | Wait ~45s | | Every log line labelled `info` | The level regex is not matching | See `docs/observability.md` | +| `make up` fails: `did not accept a reload within 60s` | A service started but never bound its listener, so it may be serving a stale config | `make logs SERVICE=`; raise `RELOAD_TIMEOUT` only if the box is genuinely that slow | ## Backups diff --git a/docs/runbooks/rotate-snmp-community.md b/docs/runbooks/rotate-snmp-community.md index 1c28fef..a83e324 100644 --- a/docs/runbooks/rotate-snmp-community.md +++ b/docs/runbooks/rotate-snmp-community.md @@ -154,8 +154,13 @@ make reload `make render` fails loudly if any placeholder is left unsubstituted, so a typo in a key name is caught before anything restarts. `make reload` is enough: snmp-exporter reads its config once at startup but serves an unconditional -`POST /-/reload`, so no container needs recreating. `make up` also works and is -not wrong, it just does more than is needed. +`POST /-/reload`, so no container needs recreating. + +`make up` also works — it runs the same reload after `docker compose up -d`. +Until PR #19 it did **not**, and that was a trap rather than a longer road: +`compose up -d` recreates a container only when its *service definition* +changes, so a freshly rendered `snmp.yaml` was invisible to it. `make up` +reported success and snmp-exporter went on polling with the old community. ### 2.4 Verify @@ -269,7 +274,8 @@ care how many times you write it. | `error: malformed line ... expected 'KEY: value'` | A key was written `KEY:value`, with no space after the colon | `make secrets-edit` | | Target still `DOWN` a minute after `make reload` | snmp-exporter reloaded from the *old* rendered file | You skipped `make render`. Run `make render && make reload` | | `unsubstituted placeholders remain` | A `SNMP_COMMUNITY_*` key is missing from the secrets file | `make secrets-edit` | -| `make reload` fails on the snmp-exporter line | The container is not running | `make ps`, then `make up` | +| `error: snmp-exporter is not running` | The container is stopped or crash-looping | `make ps`, then `make up` | +| `error: snmp-exporter refused the reload` | The rendered `snmp.yaml` does not parse; it is **still serving the old config** | `make snmp-generate` output is bad — inspect it, or `git checkout --` it | | Target `UP` but every metric missing | The community works; the module does not match the device | `curl -s 'localhost:9116/snmp?target=&module=&auth=auth_'` | | iLO unreachable after saving | The management processor reset | Wait 2 minutes, then `hponcfg` from the host OS, or the rack | | The UPS card stops responding on save | The NMC restarted its NIC | Wait 60s. The UPS is still supplying power | diff --git a/scripts/reload-config.sh b/scripts/reload-config.sh new file mode 100755 index 0000000..87cd153 --- /dev/null +++ b/scripts/reload-config.sh @@ -0,0 +1,146 @@ +#!/usr/bin/env bash +# +# Make every running service re-read its config from disk. +# +# This exists because `docker compose up -d` recreates a container only when its +# *service definition* changes. A changed bind-mounted file is invisible to it: +# compose compares the config hash of the service (image, command, mounts, +# environment), not the contents of what those mounts point at. So +# render-config.sh can write a brand-new snmp.yaml, `make up` reports success, +# and snmp-exporter keeps serving the config it parsed at startup. +# +# That is not hypothetical. The corrected pfSense walk in PR #18 was rendered to +# disk, `make up` ran clean, and the exporter kept timing out at 45s until +# someone restarted the container by hand. The failure mode is the bad one: the +# tool says done, and only the target tells you otherwise, minutes later. +# +# Reload rather than restart, for all three: +# +# prometheus POST /-/reload, gated behind --web.enable-lifecycle, which +# compose.yaml sets. A restart would drop the TSDB head. +# alertmanager POST /-/reload. Its .rendered/webhook_url is read at notify +# time so it does not need this, but alertmanager.yaml is a +# bind mount with exactly the staleness problem above. +# snmp-exporter POST /-/reload. v0.30.1 serves this unconditionally — there +# is no lifecycle flag to enable, unlike Prometheus. That is +# what lets an SNMP community rotation be `make render && make +# reload` rather than a container recreate. +# +# The snmp-exporter reload works only because render-config.sh writes the +# rendered file with `>`, truncating in place and keeping the inode. The +# container bind-mounts the file, not the directory, so switching to +# write-temp-then-mv would leave the mount pointing at the old inode and this +# reload would cheerfully re-read the old community. +# +# Failing is the point. An earlier sketch of this swallowed reload errors so +# `make up` would not break on a slow box — which rebuilds the exact defect it +# was written to close, because a swallowed reload leaves `make up` reporting +# success over a stale config. A reload that never lands is a failed deploy and +# says so. +# +# Usage: scripts/reload-config.sh [stack] (default: observability) +# +# RELOAD_TIMEOUT= bounds how long a running-but-not-yet-listening +# container is given to answer. Only reached on a cold `make up`. + +set -euo pipefail + +REPO_ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +STACK="${1:-observability}" +COMPOSE_FILE="${REPO_ROOT}/stacks/${STACK}/compose.yaml" +TIMEOUT="${RELOAD_TIMEOUT:-60}" + +die() { printf '\033[0;31merror:\033[0m %s\n' "$*" >&2; exit 1; } +ok() { printf '\033[0;32m reloaded\033[0m %s\n' "$*"; } + +[[ -f "${COMPOSE_FILE}" ]] || die "no such stack: ${COMPOSE_FILE}" + +COMPOSE=(docker compose -f "${COMPOSE_FILE}") + +# service:port. Ordered cheapest-to-diagnose first: if the whole stack is down, +# the operator sees prometheus fail rather than waiting out three timeouts. +SERVICES=( + prometheus:9090 + alertmanager:9093 + snmp-exporter:9116 +) + +# Whether compose considers the service up right now. Checked before the retry +# loop so a stopped container fails immediately with advice, instead of burning +# the full timeout re-discovering that it is stopped — `make reload` against a +# stopped stack is a documented, ordinary mistake. +is_running() { + local cid + # `compose ps -q` and `docker inspect -f`, not `compose ps --format + # '{{.State}}'`: custom Go templates only reached `compose ps` in a later + # Compose v2, and on an older one the template is taken as a literal format + # name, so every service reads as not-running and `make up` fails on a + # perfectly healthy stack. `-q` has meant the same thing since v1. + # + # `compose ps` without --all lists running containers only, so a stopped + # service yields an empty id. A crash-looping one is listed, and inspect + # reports `restarting` — which is correctly not `running`, since a container + # that is always restarting looks alive and reloads nothing. + cid="$("${COMPOSE[@]}" ps -q "$1" 2>/dev/null)" || return 1 + [[ -n "${cid}" ]] || return 1 + [[ "$(docker inspect -f '{{.State.Status}}' "${cid}" 2>/dev/null)" == "running" ]] +} + +reload_one() { + local svc="$1" port="$2" deadline out rc + deadline=$((SECONDS + TIMEOUT)) + + is_running "${svc}" || die \ + "${svc} is not running, so there is nothing to reload. Check 'make ps', then 'make up'." + + while :; do + # -T: no TTY allocation, so the response body is capturable and this + # behaves the same under make, CI and a bare shell. + # + # rc is captured with `|| rc=$?` rather than read from $? after an `if`. + # An `if` whose condition fails and which has no else branch exits 0, so + # `$?` afterwards is the status of the *if*, not of the command — which + # silently disarmed the rc == 8 branch below and turned every bad config + # into a full-timeout wait. + rc=0 + out="$("${COMPOSE[@]}" exec -T "${svc}" \ + wget -q -O- --post-data='' "http://localhost:${port}/-/reload" 2>&1)" || rc=$? + if ((rc == 0)); then + ok "${svc}" + return 0 + fi + + # "The service answered and refused" means a config on disk that does not + # parse, and no amount of waiting fixes it — so do not spend the timeout + # pretending otherwise. + # + # Detected by message, not by exit status, because all three images ship + # BusyBox wget and BusyBox exits 1 for everything: a refused connection and + # an HTTP 500 are indistinguishable by status. This was written against GNU + # wget's exit 8 first, which meant the branch could never fire and every + # unparseable config waited out the full timeout before reporting a + # misleading "did not accept a reload" — the wrong diagnosis for a service + # that had answered immediately and said exactly what was wrong. The exit 8 + # test is kept for an image that ever ships GNU wget; on these three it is + # the string that fires. + if ((rc == 8)) || [[ "${out}" == *"server returned error"* ]]; then + [[ -n "${out}" ]] && printf '%s\n' "${out}" >&2 + die "${svc} refused the reload: its config on disk does not parse. It is still serving the previous config." + fi + + # Anything else is almost always wget exit 4, connection refused, because + # the container is running but has not bound its listener yet. `compose up + # -d` returns when containers are started, not when they are ready. + if ((SECONDS >= deadline)); then + [[ -n "${out}" ]] && printf '%s\n' "${out}" >&2 + die "${svc} did not accept a reload within ${TIMEOUT}s. It may be serving a stale config — check 'make logs SERVICE=${svc}'." + fi + sleep 1 + done +} + +for entry in "${SERVICES[@]}"; do + reload_one "${entry%%:*}" "${entry##*:}" +done + +printf '\033[0;32mreloaded\033[0m — every service re-read its config from disk\n' diff --git a/stacks/observability/compose.yaml b/stacks/observability/compose.yaml index 06018ab..485e51e 100644 --- a/stacks/observability/compose.yaml +++ b/stacks/observability/compose.yaml @@ -140,7 +140,15 @@ services: expose: - "9116" healthcheck: - test: ["CMD", "wget", "--spider", "-q", "http://localhost:9116/health"] + # /-/healthy, not /health. snmp_exporter v0.30.1 serves no /health — it + # returns 404 — so this probe failed on every run and the container sat + # permanently unhealthy while working perfectly. It was invisible because + # nothing depends_on snmp-exporter and Docker does not restart on + # unhealthy, so the only symptom was a status in `make ps` that had always + # been there. A healthcheck that cannot pass is worse than none: it + # reports a permanent fault the operator learns to scroll past, and it + # deadlocks the first service that ever waits on service_healthy. + test: ["CMD", "wget", "--spider", "-q", "http://localhost:9116/-/healthy"] interval: 60s timeout: 10s retries: 3