Repository navigation
feat: Count interrupted netlink listings - #1135
Conversation
A netlink listing of links, addresses or routes that another process interrupts is retried up to 50 times before it fails, and nothing recorded the retries. Sustained churn on a node stayed invisible until it exhausted them and a component failed. nldump now runs every attempt itself instead of leaving the first ten of each round to the netlink library's hidden retry, so it sees each interruption. The bound is unchanged: five rounds of ten back-to-back attempts, with 20, 40, 60 and 80 ms pauses between rounds. Two counters, labelled by listing kind, record what happens: - galactic_netlink_dump_interrupted_total counts every attempt the kernel interrupted. - galactic_netlink_dump_failures_total counts listings still interrupted on their last attempt. Every long-running binary that reaches nldump registers them on its metrics port: galactic-cni's daemon, galactic-router, galactic-vrf, galactic-gateway, galactic-nat and galactic-edge. The CNI plugins run once per attach and expose no metrics. Retries log nothing, so normal churn adds no log lines. docs/observability/netlink.md documents the counters, the port and scrape job of each component that exposes them, and example queries. A listing that exhausts its retries now reports the real attempt count in its error, since netlink's own message counts only the handle's single try. Resolves #1134
|
Both reviewers agreed on this head. pr-adversary: "VERDICT: merge. Timing is unchanged, counters register once per process, and the build and the unit tests I ran pass; hold the merge itself until CI finishes." pr-conventions-reviewer: "VERDICT: merge. The change is Go code and docs with no deploy path of its own. The title, body, commit and draft state meet the conventions." The adversary built the change and ran the unit tests. The conventions reviewer checked the title, body, commit and draft state against the conventions. No findings were applied. The only findings were two nits, left as they are: a note that galactic-vrf and galactic-nat share a default port, and a possible trivial rebase against open PR #598. CI is green on the head: every check passed, and the service API publish job was skipped. Auto-merge is enabled with a merge commit, the only method the ruleset allows. The ruleset requires one code owner approval after the last push, so the PR waits for a human approval. |
ecv
left a comment
ecv
left a comment
There was a problem hiding this comment.
Approve.
The metric is bounded (two counters, three fixed kind values, pre-seeded at init), the retry cost is unchanged, and every long-running process that reaches nldump registers it.
- Cardinality: labels are
kindonly (link,address,route), so six series per process. Node and job come from the scrape. - Retry path: the handle drops
RetryInterruptedandretrynow loops 5 rounds of 10 attempts. That is the same 50 attempts as before (5 outer times 10 inner), with pauses of 20, 40, 60 and 80 ms after each round and none after the last. Each attempt re-issues the dump either way, so the only added work is a counter increment. The docs figure of "5 rounds of 10" and the pauses match the code and the test. - Failure path: the final error now reads "still interrupted after 50 attempts" and still wraps
ErrDumpInterrupted, soerrors.Iscallers are unaffected. Non-interrupt errors return at once and move no counter. - Registration: by
go list -deps, the nldump importers are galactic-edge, gateway, nat, router (and router-rr), vrf, and the galactic-cni installer, and each one registers it on the registry its metrics port serves. The remaining importers (galactic-bgp, route, tap, veth) are one-shot CNI plugins with no metrics server, which the doc states.
Follow-ups:
- galactic-vrf and galactic-gateway have no scrape object shipped, so their counters are collected but not scraped. The doc says so. A ServiceMonitor or PodMonitor would close it.
- Several packages still call
netlink.AddrList,RouteListFiltered,LinkListandNeighListdirectly (for exampleinternal/hostgw,internal/plumbing/srv6,internal/plumbing/vip). They get neither the retry nor the count, so the metric undercounts churn on those paths.
Summary
When another process changes links or routes on a node, Galactic's netlink listings retry up to 50 times before failing, and until now nothing recorded those retries, so sustained churn stayed invisible until a component failed.
Every long-running Galactic component now counts each interrupted attempt and each listing that failed after its last retry, labelled by listing kind, on its existing metrics port.
The retry bound and timing are unchanged, retries log nothing, and the one-shot CNI plugins are not counted because they expose no metrics.
Test plan
Fixes #1134