Skip to content

Commit 0ee2273

Browse files
fix(agent): adoption cannot hide a node; Config Import on fresh installs (v1.11.1)
Six fixes to the agent's discovery path and one long-standing parity gap, found by auditing it in loops against a real fleet. DISCOVERY REPORTS THAT NEVER REACHED THE SERVER The agent posts an unmanaged keepalived.conf for adoption and caches the hash of what it sent, so the file - which carries the VRRP password - is re-posted only when it changes. Delivery was judged by curl's exit code, and curl without -f exits 0 on 5xx too, so a report the server REJECTED was recorded as delivered. Since a hand-maintained config does not change on its own, that node dropped out of "Unmanaged keepalived detected" permanently; the only cure was deleting a cache file on the node by hand. - the report is cached only on a 2xx; - GET /agents/{name}/keepalived-config now reports whether the server actually holds a discovery for that agent, and the cache may only suppress while it says yes - which is what lets nodes stuck from earlier releases recover on their own, with nobody touching them; - the flag is parsed with has() + tostring, not `// empty`: jq's alternative operator returns the alternative for **false** as well as null, so the naive form could not tell "no record" from "older backend" and the recovery would have been completely inert; - a 400/413/422 records the refusal so identical bytes are not re-posted forever - 4xx and 5xx agent calls are never sampled out of the request log, so an unattended loop would write a row carrying the whole config every cycle - while 401 and 404 keep retrying, because here they mean a token rotation or an agent row briefly absent, not a bad payload; - the CLEAR path had the same exit-code defect, where it left a stale row offering a managed node for adoption with nothing to ever retry it. CONFIG IMPORT WAS A NO-OP ON FRESHLY INSTALLED AGENTS check_config_requests uploads a node's live haproxy.cfg on request. It was defined in the installer body and in the self-upgrade daemon, but not in the heredoc a fresh install writes, and its call site is guarded by `type` - so on such a node the operator asked for a config and nothing arrived, with no error anywhere. Any agent that had self-upgraded at least once already had it, which is why it went unnoticed. The self-upgrade definition is copied verbatim (verified line-for-line). A freshly installed agent now polls that endpoint once per cycle exactly as every upgraded agent already does; no node running today changes behaviour. DETERMINISTIC CONFIG PATH A pool may hold several clusters and the join that resolves keepalived_config_path was unordered, so the path handed to an agent could differ between polls whenever two clusters disagreed - the agent would inspect a file that is not there and the node would never appear, intermittently. A customised path now wins over the shipped default, then the lowest cluster id. Verified against a real PostgreSQL over seven arrangements: with one cluster per pool, or when every cluster carries the default, the value is byte-identical to before. Verified end to end on a production fleet and, for each decision, against the real _kp_discover block rather than a paraphrase. Backend suite: 1674 passed, 152 skipped. bash -n passes on the whole file and on the fresh-install body in isolation. The keepalived path is logic-identical across both daemon copies, now pinned by a test.
1 parent 9d7a142 commit 0ee2273

7 files changed

Lines changed: 483 additions & 26 deletions

File tree

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2475,6 +2475,7 @@ Developed with ❤️ for the HAProxy community
24752475

24762476
## Release Notes
24772477

2478+
- **v1.11.1** (2026-08-15) — **A node can no longer be hidden from adoption for good, and Config Import reaches a freshly installed agent**: the agent posts an unmanaged `keepalived.conf` for adoption and caches the hash of what it sent, so the file (which carries the VRRP password) is re-posted only when it changes. Delivery was judged by `curl`'s exit code, which is **0 for 5xx as well**, so a report the server *rejected* was recorded as delivered — and because a hand-maintained config does not change on its own, that node dropped out of *Unmanaged keepalived detected* permanently, curable only by deleting a cache file on the node by hand. Now: the report is cached only on a 2xx; `GET /agents/{name}/keepalived-config` tells the agent whether the server actually holds a discovery for it, so nodes stuck from earlier releases recover by themselves on the next poll; a **400/413/422** records the refusal so identical bytes are not re-posted forever (4xx and 5xx are never sampled out of the request log, so an unattended loop would write a row carrying the whole config every cycle), while **401 and 404 keep retrying** because in this system they mean a token rotation or an agent row briefly absent, not a bad payload. The same exit-code mistake in the *clear* path is fixed too, where it left a managed node still being offered for adoption. Separately, **Config Import silently did nothing on any agent that had never self-upgraded**: `check_config_requests` was defined in the installer and in the self-upgrade daemon but not in the body a fresh install writes, and its call site is guarded by `type`, so the operator asked a node for its `haproxy.cfg` and nothing arrived, with no error anywhere. And the cluster whose `keepalived_config_path` is handed to an agent is now resolved deterministically — a pool may hold several clusters, and the unordered join could return a different one between polls, pointing the agent at a file that does not exist. Agent-script change: sync the script from Agent Management and let the agents upgrade. No schema change.
24782479
- **v1.11.0** (2026-08-14) — **Unified request/response log with configurable retention**: until now the only record of what happened was `user_activity_logs`, which stores non-GET **2xx** operations with no bodies — so when something failed you could see *that* the count went up, never *what was sent or what came back*. This release adds one queryable timeline covering **both directions**: every inbound API call (**including GETs and including 4xx/5xx**) with the user, client IP, status, duration and — redacted and size-capped — the request and response bodies; and every **outbound** HTTP call the backend makes, tagged with who it went to (ACME/Let's Encrypt, Cloudflare, GoDaddy, HAProxy stats, agents, the ACME diagnostics probe). Outbound rows **inherit the inbound request's id**, so one operator action and the CA/DNS calls it triggered read as a single trace — opening a failed *Request Certificate* shows the exact `POST /acme/new-order` and the CA's `429` body underneath it. Capture is a **pure-ASGI middleware that tees** the request and response streams rather than draining them, so no downstream handler is affected (notably the raw-body agent heartbeat), and rows are written by a **batching background writer** with a bounded queue, so the request path never waits on the database and a saturated logger drops rows visibly instead of blocking. Secrets never land: headers are an allowlist (`Authorization`/`Cookie` reduced to a presence marker), body keys and value shapes are redacted (passwords, tokens, API keys, private-key PEMs, JWTs), the **ACME JWS request body is never stored** (a stored `protected`+`signature` pair is a replayable credential — a summary is logged instead), DNS-provider errors record only the exception **type**, and the ACME HTTP-01 challenge endpoint is excluded so `key_authorization` is never captured. **Retention is operator-configurable** in *Settings → Request Log*: separate day counts for successful and failed rows (defaults 7 and 30) plus a hard row cap (500 000), whichever is reached first, pruned in **batches** under a Postgres advisory lock so a multi-million-row table cannot time out the delete or have every replica scan it at once. New **Request Log** page (`requestlog.read`) and retention/purge permission (`requestlog.manage`); `super_admin` and `security_admin` get both, `operator` gets read, `viewer` gets neither. **Successful agent polls are not logged** (`capture_agent_success`, default off; failures always are), which is what keeps the table's size a function of operator activity rather than of node count: measured at 2 424 bytes/row, a 200-node fleet would otherwise write 2.0M rows/day and reach the row cap in six hours, silently reducing the configured 7-day/30-day retention to a few hours for everything in the table. Cost is measured, not estimated: 27.7 µs per request on the hot path, 18.8 µs per row on the writer task, **0.096 % of one core at 500 nodes**. Adds one new table (`request_logs`) and its settings seed — SCHEMA_VERSION 11 → 12 (not 11: that number was taken by v1.10.4 while this was in review, and the version gate would have skipped the migration entirely on every existing install), auto-migrated, no existing table altered, no agent or rendered-config change. Kill switches: `REQUEST_LOG_ENABLED=false` (environment — the middleware is then never registered and costs nothing) or the `enabled` toggle in Settings (no restart).
24792480
- **v1.10.14** (2026-08-14) — **A converged node keeps acknowledging**: the deploy report is the server's only evidence that a member node applied its `keepalived.conf`, and it was sent on the write path alone. Once the rendered config was on disk the agent took the idempotency early return on every cycle and never reported again, so a **single lost report** — a backend restart, a 5xx, a network blip — left the VIP reading `SYNCING (0/n)` with an empty *Last ack* forever, while the node was demonstrably running the right config. Nothing would ever reconcile the two: the node was correct, the page was not, and the only way out was to change the rendered config so the agent wrote it again. The agent now re-asserts its state on the idempotent path too, which costs one request per node per ~2.5 minutes and touches nothing on the node — keepalived is not reloaded and the file is not rewritten. This is a long-standing gap from the original HA/VIP work, surfaced when acknowledgements were dropped for an unrelated reason in v1.10.12. Agent-script change: sync the script from Agent Management and let the agents upgrade. No schema or API change.
24802481
- **v1.10.13** (2026-08-14) — **Agent deploy acknowledgements were silently dropped** (regression in v1.10.12, fix it before or with that release): the takeover-retirement clause added to `POST /agents/{name}/keepalived-status` in v1.10.12 reused one query placeholder for both the assignment `last_deploy_hash=$n` and the comparison inside its `CASE`. PostgreSQL deduces a type per **use**, so the same placeholder came out as `text` in one and `character varying` in the other, and asyncpg rejected the statement with `AmbiguousParameterError`. The failure was not partial: the whole UPDATE never ran, so **no member ever recorded an acknowledgement**. Every VIP sat at `SYNCING (0/n)` with an empty *Last ack*, even after the nodes had deployed the config successfully, and teardown acknowledgements were lost the same way. The hash is now bound to its own placeholder, which is only ever compared against the column and therefore unambiguous. Verified against a real PostgreSQL: both statements execute, a matching hash retires the takeover authorisation, a non-matching hash and a NULL `applied_config_hash` both leave it in place, and every case records the acknowledgement. A test now asserts every `$n` in these statements is bound exactly once and that the count matches the arguments passed. Backend only: no schema, agent or API-shape change.

UPGRADE_GUIDE.md

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,46 @@
1+
# Upgrade Notes — v1.11.1 (adoption cannot hide a node; Config Import on fresh installs)
2+
3+
**Agent-script change, no schema change.** No `SCHEMA_VERSION` bump, so the built-in roles are
4+
**not** re-seeded. After deploying, sync the Linux agent script from **Agent Management** and let
5+
the agents upgrade, or none of this reaches the nodes.
6+
7+
- **A node that never appeared under *Unmanaged keepalived detected* now recovers by itself.**
8+
The agent caches the hash of its last discovery report and skips re-posting while it matches.
9+
Delivery was judged by `curl`'s exit code, which is 0 for 5xx too, so a rejected report was
10+
cached as delivered and a hand-maintained config — which never changes on its own — kept that
11+
node hidden. The report is now cached only on a 2xx, and the config endpoint reports whether
12+
the server actually holds a discovery for that agent, so the cache can only suppress while the
13+
server agrees. **No access to the nodes is needed**; affected nodes reappear within one poll
14+
cycle (~2.5 min) after the agents pick up the new script.
15+
- **Permanent refusals do not loop.** A 400, 413 or 422 means the payload itself is unacceptable,
16+
so the refusal is recorded and the same bytes are not re-posted; fixing the file releases the
17+
brake, because it is keyed to the content hash. **401 and 404 keep retrying** — in this system
18+
they mean a token rotation or an agent row briefly absent while it re-registers, and braking on
19+
them would have re-created the very failure above. This matters beyond noise: 4xx and 5xx agent
20+
calls are never sampled out of the request log, so a loop would write a row carrying the whole
21+
`keepalived.conf` every cycle on every affected node.
22+
- **The clear path had the same defect.** When a node becomes managed the agent tells the server
23+
to drop the discovery; that too was judged by the exit code, so a rejected clear left a stale
24+
row offering a **managed** node for adoption, with nothing to ever retry it.
25+
- **Config Import now works on a freshly installed agent.** `check_config_requests` uploads a
26+
node's live `haproxy.cfg` when you ask for it. It was defined in the installer and in the
27+
self-upgrade daemon but not in the body a fresh install writes, and its call site is guarded by
28+
`type`, so on such a node the feature was a silent no-op: the request was made and nothing ever
29+
arrived. Any agent that had self-upgraded at least once already had it, which is why it went
30+
unnoticed. A freshly installed agent now polls the pending-requests endpoint once per cycle,
31+
exactly as every upgraded agent already does — **no node running today changes behaviour**.
32+
- **The keepalived.conf path is resolved deterministically.** A pool may hold more than one
33+
cluster and the join was unordered, so the path handed to an agent could differ between polls
34+
whenever two clusters disagreed on it — the agent would inspect a file that is not there and the
35+
node would never appear, intermittently. A customised path now wins over the shipped default,
36+
then the lowest cluster id. With one cluster per pool, or when every cluster carries the
37+
default, the value is byte-identical to before.
38+
39+
**Rollback:** safe. No schema or data change; reverting restores the previous behaviour, in which
40+
a rejected discovery report is never retried and Config Import is absent on fresh installs.
41+
42+
---
43+
144
# Upgrade Notes — v1.11.0 (Unified request/response log)
245

346
**Adds one new table and bumps `SCHEMA_VERSION` 11 → 12. The migration runs automatically on the

backend/routers/agent.py

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2325,16 +2325,41 @@ async def get_agent_keepalived_config(agent_name: str, x_api_key: Optional[str]
23252325
# to write/own-marker-check even on not_configured/teardown.
23262326
agent = await conn.fetchrow("""
23272327
SELECT a.id, a.name, COALESCE(a.enabled, TRUE) AS enabled,
2328-
hc.keepalived_config_path
2328+
hc.keepalived_config_path,
2329+
-- v1.11.1: does the server already hold a discovery for this node? The agent
2330+
-- caches the hash of its last discovery report next to the config and skips
2331+
-- re-posting while it matches. That cache used to be written even when the
2332+
-- POST was REJECTED, so a node could be hidden from the adoption panel for
2333+
-- good: the file never changes, so the agent never speaks again. Telling it
2334+
-- what we actually hold lets it recover on its own, with no extra request and
2335+
-- no one having to touch the node.
2336+
EXISTS (SELECT 1 FROM vip_discoveries vd WHERE vd.agent_id = a.id)
2337+
AS discovery_known
23292338
FROM agents a
23302339
LEFT JOIN haproxy_clusters hc ON hc.pool_id = a.pool_id
23312340
WHERE a.name = $1
2341+
-- A pool may hold more than one cluster, and the join then multiplies this row. With
2342+
-- no ordering the fetch took an arbitrary one, so the keepalived.conf PATH handed to
2343+
-- the agent was non-deterministic whenever two clusters in a pool disagreed on it:
2344+
-- the agent would look at the wrong file, find nothing there, and the node would
2345+
-- never appear for adoption — intermittently, which is the worst way to fail.
2346+
--
2347+
-- A CUSTOMISED path wins over the shipped default, then the lowest cluster id. The
2348+
-- column defaults to '/etc/keepalived/keepalived.conf' rather than NULL, so ordering
2349+
-- by id alone could have picked a default-valued row over one the operator had
2350+
-- deliberately set — turning "undefined" into "reliably wrong" for that install.
2351+
-- When every cluster in the pool carries the default the string is identical, so the
2352+
-- ordering cannot change what any working deployment already receives.
2353+
ORDER BY (hc.keepalived_config_path IS NULL
2354+
OR hc.keepalived_config_path = '/etc/keepalived/keepalived.conf'),
2355+
hc.id
2356+
LIMIT 1
23322357
""", agent_name)
23332358
if not agent:
23342359
raise HTTPException(status_code=404, detail=f"Agent '{agent_name}' not found")
23352360
config_path = agent['keepalived_config_path'] or '/etc/keepalived/keepalived.conf'
23362361
if not agent['enabled']:
2337-
return {"agent_name": agent_name, "status": "not_configured", "config_path": config_path, "keepalived": None}
2362+
return {"agent_name": agent_name, "status": "not_configured", "config_path": config_path, "discovery_known": bool(agent["discovery_known"]), "keepalived": None}
23382363

23392364
row = await conn.fetchrow("""
23402365
SELECT v.id AS vip_id, v.name AS vip_name, v.is_active, v.track_haproxy,
@@ -2350,22 +2375,22 @@ async def get_agent_keepalived_config(agent_name: str, x_api_key: Optional[str]
23502375
""", agent['id'])
23512376

23522377
if not row:
2353-
return {"agent_name": agent_name, "status": "not_configured", "config_path": config_path, "keepalived": None}
2378+
return {"agent_name": agent_name, "status": "not_configured", "config_path": config_path, "discovery_known": bool(agent["discovery_known"]), "keepalived": None}
23542379
if not row['is_active']:
23552380
# Soft-deleted VIP → teardown. purge carries the operator's opt-in package removal;
23562381
# the agent still only purges on nodes where IT installed keepalived (install marker).
23572382
return {"agent_name": agent_name, "status": "teardown", "vip_id": row['vip_id'],
2358-
"config_path": config_path, "keepalived": None,
2383+
"config_path": config_path, "discovery_known": bool(agent["discovery_known"]), "keepalived": None,
23592384
"purge": bool(row['purge_on_teardown'])}
23602385
if not row['applied_config_content']:
2361-
return {"agent_name": agent_name, "status": "not_configured", "config_path": config_path, "keepalived": None}
2386+
return {"agent_name": agent_name, "status": "not_configured", "config_path": config_path, "discovery_known": bool(agent["discovery_known"]), "keepalived": None}
23622387

23632388
from services.keepalived_config import build_haproxy_check_script
23642389
check_script = build_haproxy_check_script() if row['track_haproxy'] else ""
23652390
return {
23662391
"agent_name": agent_name,
23672392
"status": "available",
2368-
"config_path": config_path,
2393+
"config_path": config_path, "discovery_known": bool(agent["discovery_known"]),
23692394
"keepalived": {
23702395
"desired_state": "enabled",
23712396
"install_if_missing": True,

0 commit comments

Comments
 (0)