[cnuss]

What

Adds opt-in router readiness that requires local TCP listeners. ModelRouter.spec.proxy.readinessRequiredListeners (for example [15008], Istio ambient's HBONE inbound port) moves the router-proxy's readiness probe to a new GET /readyz. It answers 503 while any listed port has no LISTEN socket in the pod's network namespace. Liveness stays on /health.

Why

A router-proxy replica can be running while the service mesh that captures its traffic has not set it up. /health keeps that replica in the Service endpoints, and requests sent to it fail. Gating readiness on the mesh's listener takes such a replica out of rotation without restarting it.

Fixes #2442

How

• router-proxy: a repeatable --readiness-required-listener=<port> flag, and GET /readyz on the existing listener next to /health and /healthz, which are unchanged. Detection is pure Go and reads /proc/net/tcp and /proc/net/tcp6. A port counts only when an entry's local port matches and its state is 0A (LISTEN). /readyz returns 503 with {"status":"not ready","missingListeners":[...]}, otherwise 200. With no ports required it always returns 200.
• Fail open: if a table can't be read, or its header isn't the expected format, ports not found in the readable tables are not reported missing, and the replica stays Ready. Each check ends in one of three states, logged once per transition: required listeners present; reporting ready (every port seen), required listeners missing; reporting not ready, and required listeners not enforced; reporting ready (a table couldn't be read, with the ports and the unreadable tables). Each table also logs cannot read TCP socket table once, and once more when it can be read again. A missing /proc/net/tcp6, as on a kernel booted with IPv6 disabled, counts as an empty table rather than a read failure. Fail-open covers unreadable tables only: a port that never listens leaves every replica with the field unready.
• Bounded cost: the check result is cached for about 1s, and callers that arrive while a check runs wait for it instead of starting another, so /proc is read at most about once per second however many callers there are. The scans run outside the mutex, which only guards the cached result and the logging state.
• Controller: when the list is non-empty, the readiness probe path becomes /readyz (same port and timings), and the args get one --readiness-required-listener <port> per entry. Unset or empty renders the same Deployment as before, so upgrading the operator does not roll existing routers. The existing routerproxy-*.golden files are unchanged and still pass.
• API: readinessRequiredListeners []int32 with item bounds 1 to 65535, MaxItems=16 and listType=set. CRDs and chart CRDs regenerated.
• Docs: docs/site/concepts/model-router.md covers the field, liveness staying on /health, the fail-open behavior and the log lines. It also says fail-open covers unreadable tables only, so a port that never listens takes every replica out and stalls a rollout. It says not to combine the field with a runtime class that has its own network stack (gVisor), and covers the image requirement when spec.proxy.image is pinned, the ~1s cache, and the posture of /readyz.
• Posture: /readyz is unauthenticated on the inference port, like /health. It reveals only which configured ports have no listener.
• Tests:
  • Detector tests run against /proc/net/tcp and tcp6 fixtures in the kernel's format: LISTEN in tcp only, LISTEN in tcp6 only, ESTABLISHED on the port, a different port listening, unreadable, missing or malformed tables, and a port found while the other table is unreadable. Each case asserts which of the three state lines is logged, and that it is logged once.
  • Cache tests use an injected clock and a counted scan: two calls within the TTL scan once; a call after the TTL rescans and sees the change; 32 concurrent callers share one scan and get the same answer (run with -race).
  • /readyz is tested through the mux.
  • Flag parsing is tested on the real flag registration.
  • A router-proxy binary test checks the flag wiring. On Linux it reserves the required port without listening, so nothing else can take it, expects 503, then listens on that same socket and expects 200. Elsewhere it expects fail-open with both warnings logged.
  • Controller builder tests and an envtest reconcile test cover set and clear on the live Deployment.
  • An envtest test checks the CRD bounds: 0, 65536 and duplicates are rejected; 1 and 65535 are accepted.
• Small cleanup: waitForReady in the router-proxy integration tests lost its timeout parameter, because every caller passed 10s and unparam flagged it.

Checklist

• Tests added/updated
• make test passes locally (the full suite ran through make gate, which also runs fmt, vet, lint-all, regeneration drift and copyright checks)
• make lint passes locally (make lint-all, darwin and linux)
• Commit messages follow conventional commits
• All commits are signed off (git commit -s) per DCO
• AI assistance (if any) is disclosed above, per CONTRIBUTING.md
• Documentation updated (if user-facing change)

Assisted-by: Claude Code (Claude Opus 5.5) generated the code, tests and this description; I reviewed the diff and the description and ran the checks above.

[cnuss]

One question for you: when spec.proxy.image pins a router-proxy older than this change and readinessRequiredListeners is set, the proxy exits on the unknown flag and crash-loops. Right now the docs call that out. Would you rather the controller warn (an event or status condition) or refuse when both are set? Happy to add either here or in a follow-up.

[codecov]

Codecov Report

❌ Patch coverage is 92.54658% with 12 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
cmd/router-proxy/main.go	72.72%	6 Missing ⚠️
api/v1alpha1/zz_generated.deepcopy.go	25.00%	3 Missing ⚠️
internal/router/readiness.go	97.16%	3 Missing ⚠️

📢 Thoughts on this report? Let us know!

[Defilan]

Reviewed against what #2442 settled, not just the PR body. All four items hold:

• Unset is byte-identical. routerproxy-deployment.golden and routerproxy-deployment-proxy-options.golden pin readinessProbe.httpGet.path: /health and are untouched, and the empty-list case is pinned against the nil-proxy case in the builder test. So an operator upgrade does not roll a router that did not opt in.
• Both tables, 0A only. The fixtures cover LISTEN in tcp only, LISTEN in tcp6 only, and the established row on 15008 not counting. The established fixture carries the port on both sides, local and remote, which is the version I'd want.
• Fail open on an unreadable table, logged once per transition, with recovery logged.
• CRD bounds 1 to 65535, 16 unique entries, both the kustomize base and the chart copy, deepcopy regenerated.

Verified rather than taken on trust: the header and column parsing is right for both tables, local_address at field 1 and st at field 3 on tcp and tcp6, port as the hex suffix after the last colon, so the 32 hex char tcp6 address parses. Absent tcp6 as an empty table is the correct call for ipv6.disable=1. Set and clear both reach the live Deployment because the update path copies the desired Template.Spec. Router-proxy pods are never hostNetwork in this repo, only multinode.go sets it and that is InferenceService, so the read really is the pod's own netns.

Three things I'd like, none of them blocking:

1. readiness.go logs the read failure and then, in the same call, required listeners present; reporting ready, because the missing set is forced empty before the transition key is computed. Say "not enforced" rather than "present" for that case.
2. /readyz rescans both tables per request under a mutex on the unauthenticated inference port. Cost scales with the socket count, and this proxy holds a lot of them to model pods. A short TTL cache, a second or so, bounds what a caller can make us do.
3. Docs: fail-open covers unreadable tables only. A port that never listens in this container's netns is not fail-open, it is every replica unready at once. One clause saying that is worth it, since the whole page leads with a single port number a reader will copy.

One question, and it decides how useful the Istio example is for other users. In your environment, does the LISTEN on 15008 land inside the workload container's netns? The port itself is right, ztunnel binds [::]:15008 as its inbound, but istio makes the DaemonSet's hostNetwork conditional on ambient.shareHostNetworkNamespace, so I cannot establish from upstream where the socket lives by default. Your #2442 note about the ss -Hltn 'sport = :15008' probe in the model pods reads like yes, and you have the environment to confirm it on 1.30.7. If it holds only for some topologies then the docs need the netns stated as a requirement rather than implied by the example.

Minor: the binary test's 503-before-listen assertion assumes the free port it picked stayed free on the runner. Fine to leave, worth knowing it can flake.

[Defilan]

Two follow-ups to my review.

The 15008 question: I had that wrong, so there's nothing for you to check. The shareHostNetworkNamespace setting I pointed at is on the istio-cni agent's DaemonSet, not ztunnel's, and it says nothing about where this socket lives. ztunnel's architecture doc lists 15008 as bound within the pod's network namespace, and Istio's traffic redirection doc says the same. So the example doesn't depend on topology, and the docs only need the clause I asked for: a port that never listens leaves every replica unready.

I also checked it on the deployment behind #2442, on 1.30.7. The model pods' ss probes pass, and ztunnel's active proxy count matches the enrolled pods on each node, with none pending.

Your pinned image question: warn, don't refuse. The controller can't tell what a pinned tag or digest contains, so refusing would block pins that work. The failure is also contained: the proxy exits with flag provided but not defined: -readiness-required-listener, and the rollout stalls with an old replica still serving. Keep the docs note here, and add a Warning event when both are set, the way warnPrefixAffinityIgnored does. A follow-up PR is fine for that.

Timing: as I said on #2442, this lands after 0.10.17, so please hold the merge until that's cut. I'll let you know when it is.

[cnuss]

Thanks for the review. All three are addressed in new commits on the branch:

1. Log message: a fail-open check now logs required listeners not enforced; reporting ready with the ports and the unreadable tables. present is only logged when the ports were actually seen. Present, missing and not enforced each log once per transition, and tests assert that an unreadable table never logs present.
2. /readyz cost: /readyz stays on the inference port. The result is cached for about a second, and concurrent callers wait for the check in flight rather than starting their own, so /proc is read at most about once per second. The scans now run outside the mutex, and the set of wanted ports is built once. Tests use an injected clock and count scans, and the concurrency test runs under -race.
3. Docs: they now say fail-open covers unreadable tables only. A port that never listens leaves every replica unready at once, and the rollout stalls. They also say not to combine the field with a runtime class that has its own network stack, such as gVisor.

On the test flake: the binary test now binds the required port without listening on it. Nothing else can take the port during the 503 check, and the test then listens on that same socket for the 200 check.

Thanks for retracting the netns question. It matches what we saw on Istio 1.30.7: the listener shows up as [::]:15008 in /proc/net/tcp6 of an enrolled pod. The Warning event for spec.proxy.image combined with readinessRequiredListeners will come in a separate PR, following warnPrefixAffinityIgnored.

[Defilan]

I read the code and docs in the commits since my approval (1026fe771 through b55c07d82). The three-state logging, the cache and the shared in-flight check look right, and the docs now state the limits clearly. My approval stands, and thanks for addressing all of that so quickly.

One optional hardening, not blocking: if a scan ever panicked, inflight would never be cleared, and every later /readyz would block on it. Clearing and closing it in a defer would rule that out.

v0.10.17 is cut, so the hold from my last comment is over. Tell me if you want to add the defer first; otherwise this is good to merge.