[GH-ISSUE #411] Health check config collides between two targets sharing the same backend host:port on reconnect/reload #8718

New Issue

GiteaMirror · 2026-07-14T20:57:50-05:00

GiteaMirror commented

2026-07-14 20:57:50 -05:00

Originally created by @gravis on GitHub (Jul 14, 2026).
Original GitHub issue: https://github.com/fosrl/newt/issues/411

Describe the Bug

When two targets point at the same backend host:port but have different Expected Response Codes, a websocket reconnect / config reload cross-contaminates their health-check configuration: one target inherits the other's expected status code and is then reported unhealthy forever even though it responds correctly.

This is distinct from #240 (which reverts custom codes to the 2xx default). Here the code does not revert to 2xx — it is overwritten with the other target's custom code.

Setup that triggers it:

Target A — expects 200 (e.g. GET /manifest.json) → backend 10.0.0.10:8123
Target B — expects 401 (e.g. GET /api/mcp) → same backend 10.0.0.10:8123

On a clean start both load correctly and are healthy. After a websocket drop + reconnect, targets are re-registered (Replacing existing target with ID ...) and Target A ends up expecting 401 (Target B's code). Target A then logs unexpected status code: 200 (expected: 401) every health-check interval and shows Unhealthy, even though 200 is its correct/expected response.

A clean newt restart fixes it (both targets healthy again) — but the next reconnect can re-trigger it.

Environment

OS Type & Version: NixOS (newt in a systemd-nspawn container)
Pangolin Version: 1.20.0 (also observed under 1.19.4)
Newt Version: 1.14.0
Gerbil / Traefik / Olm: n/a

To Reproduce

Create two resources whose health-check targets share the same backend host:port, with different Expected Response Codes (e.g. Target A → 200, Target B → 401).
Start newt cleanly. Both log Target X initial status: healthy.
Cause a websocket reconnect (server restart, DNS blip, or brief network drop — enough for newt to log not connected then Websocket connected).
On reconnect newt logs Replacing existing target with ID A / ID B, then flips one target to initial status: unhealthy.
That target now logs Target A: unexpected status code: 200 (expected: 401) on every interval and stays Unhealthy until newt is restarted.

Actual log (reconnect race, IDs/IPs genericized)

14:42:01 ERROR Failed to send registration message: not connected
   ... (~35s of "not connected") ...
14:42:34 ERROR Failed to send message: not connected
14:42:37 INFO  Websocket connected
14:42:37 INFO  Target B initial status: healthy
14:42:37 INFO  Target A initial status: healthy      <- both healthy right after connect
14:42:37 INFO  Replacing existing target with ID A
14:42:37 INFO  Replacing existing target with ID B
14:42:37 INFO  Target B initial status: healthy
14:42:37 INFO  Target A initial status: unhealthy     <- flips after the "Replacing" step
14:42:37 WARN  Target A: unexpected status code: 200 (expected: 401)   <- inherited B's 401
14:43:07 WARN  Target A: unexpected status code: 200 (expected: 401)
   ... repeats every 30s until newt is restarted ...

The initial status: healthy → Replacing existing target → initial status: unhealthy sequence for Target A within the same second strongly suggests the re-registration keys health-check config by backend address, so the second target (B) sharing 10.0.0.10:8123 overwrites Target A's expected code.

Expected Behavior

On reconnect/reload, each target should retain its own configured Expected Response Code regardless of whether another target shares the same backend host:port.

Workaround

Restart newt for a clean reload (both targets load correct codes), or avoid two targets sharing an identical backend host:port for health checks (disable the health check on one, or differentiate backends). Both are temporary — the collision returns on the next reconnect.

Originally created by @gravis on GitHub (Jul 14, 2026). Original GitHub issue: https://github.com/fosrl/newt/issues/411 ### Describe the Bug When **two targets point at the same backend `host:port`** but have **different Expected Response Codes**, a websocket **reconnect / config reload** cross-contaminates their health-check configuration: one target inherits the other's expected status code and is then reported **unhealthy forever** even though it responds correctly. This is distinct from #240 (which reverts custom codes to the 2xx default). Here the code does *not* revert to 2xx — it is overwritten with the *other* target's custom code. **Setup that triggers it:** - Target A — expects **200** (e.g. `GET /manifest.json`) → backend `10.0.0.10:8123` - Target B — expects **401** (e.g. `GET /api/mcp`) → **same backend** `10.0.0.10:8123` On a **clean start** both load correctly and are healthy. After a websocket drop + reconnect, targets are re-registered (`Replacing existing target with ID ...`) and Target A ends up expecting **401** (Target B's code). Target A then logs `unexpected status code: 200 (expected: 401)` every health-check interval and shows Unhealthy, even though 200 is its correct/expected response. A clean `newt` restart fixes it (both targets healthy again) — but the next reconnect can re-trigger it. ### Environment - OS Type & Version: NixOS (newt in a systemd-nspawn container) - Pangolin Version: 1.20.0 (also observed under 1.19.4) - Newt Version: 1.14.0 - Gerbil / Traefik / Olm: n/a ### To Reproduce 1. Create two resources whose health-check targets share the **same backend `host:port`**, with **different** Expected Response Codes (e.g. Target A → 200, Target B → 401). 2. Start newt cleanly. Both log `Target X initial status: healthy`. 3. Cause a websocket reconnect (server restart, DNS blip, or brief network drop — enough for newt to log `not connected` then `Websocket connected`). 4. On reconnect newt logs `Replacing existing target with ID A` / `ID B`, then flips one target to `initial status: unhealthy`. 5. That target now logs `Target A: unexpected status code: 200 (expected: 401)` on every interval and stays Unhealthy until newt is restarted. ### Actual log (reconnect race, IDs/IPs genericized) ``` 14:42:01 ERROR Failed to send registration message: not connected ... (~35s of "not connected") ... 14:42:34 ERROR Failed to send message: not connected 14:42:37 INFO Websocket connected 14:42:37 INFO Target B initial status: healthy 14:42:37 INFO Target A initial status: healthy <- both healthy right after connect 14:42:37 INFO Replacing existing target with ID A 14:42:37 INFO Replacing existing target with ID B 14:42:37 INFO Target B initial status: healthy 14:42:37 INFO Target A initial status: unhealthy <- flips after the "Replacing" step 14:42:37 WARN Target A: unexpected status code: 200 (expected: 401) <- inherited B's 401 14:43:07 WARN Target A: unexpected status code: 200 (expected: 401) ... repeats every 30s until newt is restarted ... ``` The `initial status: healthy` → `Replacing existing target` → `initial status: unhealthy` sequence for Target A within the same second strongly suggests the re-registration keys health-check config by backend address, so the second target (B) sharing `10.0.0.10:8123` overwrites Target A's expected code. ### Expected Behavior On reconnect/reload, each target should retain its own configured Expected Response Code regardless of whether another target shares the same backend `host:port`. ### Workaround Restart newt for a clean reload (both targets load correct codes), or avoid two targets sharing an identical backend `host:port` for health checks (disable the health check on one, or differentiate backends). Both are temporary — the collision returns on the next reconnect.

GiteaMirror commented

2026-07-14 20:57:51 -05:00

@gravis commented on GitHub (Jul 14, 2026):

Update after tracing this end-to-end in the Newt source: this is not a Newt bug — moving it to the server. Filed as fosrl/pangolin#3444.

Newt stores each health-check target's expected code by target ID (healthcheck.Config.Status) and applies whatever the server sends; it never derives, mutates, or keys the expected code by backend address. I confirmed the reconnect "Replacing existing target" path preserves each target's own expected code (reproduced with a unit test).

The real trigger is a server payload disagreement on reconnect: the newt/sync message carries the target's correct expected code (200), but the newt/wg/connect registration message that arrives right after carries the other co-located resource's code (401), and it overwrites the good value. Details + logs in pangolin#3444.

Separately, while investigating I did find a genuine (unrelated) data race in healthcheck.go: monitorTarget/performHealthCheck mutate a target's status fields with no lock while getAllTargetsUnsafe snapshots the whole struct — confirmed with -race. Happy to open a small PR for that if useful. Closing this issue in favor of pangolin#3444.

@gravis commented on GitHub (Jul 14, 2026): Update after tracing this end-to-end in the Newt source: **this is not a Newt bug** — moving it to the server. Filed as fosrl/pangolin#3444. Newt stores each health-check target's expected code by target **ID** (`healthcheck.Config.Status`) and applies whatever the server sends; it never derives, mutates, or keys the expected code by backend address. I confirmed the reconnect "Replacing existing target" path preserves each target's own expected code (reproduced with a unit test). The real trigger is a **server payload disagreement on reconnect**: the `newt/sync` message carries the target's correct expected code (200), but the `newt/wg/connect` registration message that arrives right after carries the *other* co-located resource's code (401), and it overwrites the good value. Details + logs in pangolin#3444. Separately, while investigating I did find a genuine (unrelated) data race in `healthcheck.go`: `monitorTarget`/`performHealthCheck` mutate a target's status fields with no lock while `getAllTargetsUnsafe` snapshots the whole struct — confirmed with `-race`. Happy to open a small PR for that if useful. Closing this issue in favor of pangolin#3444.

GiteaMirror commented

2026-07-14 20:57:51 -05:00

@gravis commented on GitHub (Jul 14, 2026):

Reopening with the correct root cause — I was wrong twice above (it is neither a server-side payload bug nor a host:port keying bug). It is a client bug in handleConnect, and I've reproduced it with a unit test.

handleConnect decodes the newt/wg/connect payload into the persistent n.wgData:

json.Unmarshal(jsonData, &n.wgData)

Go's encoding/json merges a JSON array into an existing slice by position, and a JSON null is a no-op for a non-pointer int (healthcheck.Config.Status). On reconnect the health-check rows come back in a different order (the server's config query has no ORDER BY), so a target with hcStatus: null (no custom expected code) lands on the slice slot previously held by another target and inherits its stale expected code — e.g. a /manifest.json target that should accept any 2xx inherits a sibling's 401 and is then reported Unhealthy forever until restart.

handleSync is immune because it decodes into a fresh local SyncData; clean boot is immune because wgData starts empty. Fix + regression test in #413 (decode into a fresh WgData, assign on success).

@gravis commented on GitHub (Jul 14, 2026): Reopening with the **correct** root cause — I was wrong twice above (it is neither a server-side payload bug nor a host:port keying bug). It is a client bug in `handleConnect`, and I've reproduced it with a unit test. `handleConnect` decodes the `newt/wg/connect` payload into the **persistent** `n.wgData`: ```go json.Unmarshal(jsonData, &n.wgData) ``` Go's `encoding/json` merges a JSON array into an existing slice **by position**, and a JSON `null` is a **no-op** for a non-pointer `int` (`healthcheck.Config.Status`). On reconnect the health-check rows come back in a different order (the server's config query has no `ORDER BY`), so a target with `hcStatus: null` (no custom expected code) lands on the slice slot previously held by another target and **inherits its stale expected code** — e.g. a `/manifest.json` target that should accept any 2xx inherits a sibling's `401` and is then reported Unhealthy forever until restart. `handleSync` is immune because it decodes into a fresh local `SyncData`; clean boot is immune because `wgData` starts empty. Fix + regression test in #413 (decode into a fresh `WgData`, assign on success).

GiteaMirror commented

2026-07-14 20:57:51 -05:00

@AstralDestiny commented on GitHub (Jul 14, 2026):

Are the repro steps different or just the final root cause different?

@AstralDestiny commented on GitHub (Jul 14, 2026): Are the repro steps different or just the final root cause different?

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: github-starred/newt#8718