Skip to content

fix: a kubelet read is bounded, and one node's silence costs one node (ADR 0045) - #78

Merged
kfeofantov merged 1 commit into
mainfrom
fix/kubelet-read-bounds
Aug 27, 2026
Merged

kfeofantov merged 1 commit into
mainfrom
fix/kubelet-read-bounds

Conversation

@kfeofantov

Copy link
Copy Markdown
Contributor

The two kubelet reads (/stats/summary, /metrics/cadvisor) had no deadline
and no size limit, and the nodes were read one after another in the goroutine
that also runs the snapshot flush. A kubelet that accepts the connection and
then goes quiet held the entire poll: client-go bounds the dial and the TLS
handshake, not the response, and the API server proxies silence as readily as
data.

  • Deadline of 10 s per read, per path — the two fail independently and are
    already counted independently.
  • Size limits, and passing one is a failed read rather than a short one:
    8 MiB for the summary (unmarshalled from a buffer, up to 8 in flight) and
    64 MiB for the exposition (decoded off the wire). A truncated response is a
    valid smaller response — a node that appears to run fewer containers.
  • Eight nodes read at once, reading only. Ingestion stays in the polling
    goroutine, so the accumulator and the counter baselines keep one owner and no
    mutex.
  • The client's rate limit is stated (50 QPS / burst 100). client-go's
    default of 5 QPS is already less than one poll of a 75-node cluster needs, and
    the limiter waits inside the request's own context — the deadline above would
    have fired on our own throttling.

Measured on kind 1.36.1, one node, by adding 40 pause pods to a 9-pod node:

pods /stats/summary /metrics/cadvisor
9 44,367 B 687,903 B
49 167,654 B 3,076,138 B

~3.1 KB and ~60 KB per pod; at the kubelet's 110-pod ceiling that is ~355 KB and
~6.7 MB. The limits are ~23× and ~10× those.

Each guard was checked by making it fail:

--- FAIL: TestASilentKubeletDoesNotHoldThePollForever (10.00s)
    pollOnce is still waiting on a silent kubelet; the read has no deadline
--- FAIL: TestOneUnresponsiveNodeCostsOneNodeNotTheCluster (2.41s)
    the poll took 2.412288208s, over 1.2s: the hung nodes were read one after another
    io.LimitReader: err=<nil> samples=1 (a plain limit is a clean end of input)

Nothing collected, stored or transmitted changes, so docs/security.md is
untouched; the reasoning is in ADR 0045, which amends ADR 0006 §1.

go build ./..., go test -race -count=1 ./..., make lint (0 issues),
go vet -tags e2e ./..., gofmt — all green.

🤖 Generated with Claude Code

… (ADR 0045)

The two kubelet reads had no deadline and no size limit, and the nodes were
read one after another, so a kubelet that accepted the connection and then went
quiet held the whole poll — and the snapshot flush behind it.

- 10 s deadline per read; 8 MiB for /stats/summary (buffered) and 64 MiB for
  /metrics/cadvisor (streamed). Passing a limit is a failed read, never a short
  one: a truncated exposition decodes as a node running fewer containers.
- Nodes are read eight at a time. Only the reading — ingestion stays in the
  polling goroutine, so the accumulator keeps one owner and no mutex.
- The API client's rate limit is stated (50 QPS / burst 100) instead of taking
  client-go's 5, which one poll of a 75-node cluster already exceeds and which
  would have consumed the deadline above.

Measured on kind 1.36.1, one node, 9 → 49 pods: /stats/summary 44,367 →
167,654 B, /metrics/cadvisor 687,903 → 3,076,138 B.

Negative checks, each failing with its own message: without the deadline the
poll still hangs at 10 s; read serially the six-node poll takes 2.41 s against a
1.2 s bound; an io.LimitReader on a cut exposition returns 1 sample and nil.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@kfeofantov
kfeofantov merged commit e458300 into main Aug 27, 2026
3 checks passed
@kfeofantov
kfeofantov deleted the fix/kubelet-read-bounds branch August 27, 2026 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant