fix(iptables): render the ruleset from a canonically ordered peer list - #15
Merged
Conversation
The unchanged-ruleset cache added in #14 never hit in production: 54 syncs across 5 tenants produced 0 "iptables rules unchanged, skipping restore". Every state push still ran a full restore, so the ~1-3s wg0 blackhole -- and the fleet-wide LibreNMS false-down bursts it causes -- continued unchanged after that fix was deployed. The cache itself is wired correctly; its key is order-sensitive and the peer order is not stable. The manager builds the peer list by iterating peers.Items from a controller-runtime cached r.List, which returns informer-map iteration order, so it reshuffles on every reconcile even when no peer changed. GenerateIptableRulesFromPeers walks the slice in order, so the rendered ruleset came out byte-different but semantically identical and the comparison missed every time. Measured on managed-superopti: 8 consecutive state pushes carried 1 distinct peer set in 8 distinct orders, every state.json exactly 113480 bytes. Sorting collapses all 8 to a single rendering. Sort a copy of the peers on (Address, AddressV6, Name) before rendering. Semantically inert: each peer gets its own chain and peer addresses are disjoint, so the order the rules are emitted in cannot change what matches. The key is a total order across both families because IPv6-only tenants leave Address empty on every peer. Copying leaves the caller's slice untouched -- cmd/agent retains it as latestState and re-applies wg.Sync on it. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
mxhob1
added a commit
that referenced
this pull request
Jul 30, 2026
The reconciler rewrites the Wireguard Secret whenever the marshalled state differs from what is stored, and every rewrite patches wgConfigLastUpdated on EVERY tenant's WG pod. filteredPeers is built by iterating peers.Items from a controller-runtime CACHED List, which returns informer-map iteration order, so it reshuffles between reconciles even when no peer changed -- and that reshuffle alone makes the marshalled bytes differ. Measured on managed-superopti: 340 "Updating secret with new config" in 37h (~9/h) against 40 peers whose set never changed, and 8 consecutive pushes carried ONE peer set in EIGHT distinct orders. Normalising only the order collapsed all 8 to a single hash -- no other field, and specifically not resourceVersion/generation/managedFields, differed between them. The churn is entirely ordering, so nothing else is touched here. Extract stateForAgent() and sort a copy of the peers by name: names are unique within a namespace and always set, so name alone is a total order. Copy rather than sort in place -- the caller built the slice and may still be using it. #15 already made the agent skip the iptables-restore when the rendered ruleset is unchanged, so these pushes are no longer harmful. This removes the pointless secret rewrites and the fleet-wide pod pokes behind them. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The unchanged-ruleset cache from #14 is deployed fleet-wide (
b8cfbdb, containers#461) but never hits: measured live 2026-07-30, 54 syncs across 5 tenants produced 0iptables rules unchanged, skipping restore(superopti 21, wavesquared 14, pivit 9, optimised 8, legion 2). So every state push still runs a fulliptables-restore, the ~1-3swg0blackhole continues, and the fleet-wide LibreNMS false-down bursts never stopped — superopti logged 402 outages by 10:55 today vs 419-884 on full pre-fix days.Root cause
The cache is wired correctly (constructed once in
cmd/agent/main.go, closed over by the state-change callback). Its key is the problem:GenerateIptableRulesFromPeerswalksstate.Peersin slice order, and that order is not stable. The manager buildsfilteredPeersby iteratingpeers.Itemsfrom a controller-runtime cachedr.List, which returns informer-map iteration order — it reshuffles on every reconcile even when no peer changed. Same rules, different bytes, cache miss, gratuitous restore.Measured on
managed-superopti(40 peers, all with zero egress policies, so the ruleset is a pure function of the ordered address list):33948e69×8Every
state.jsonwas exactly 113480 bytes. 1 distinct set, 8 distinct orders — and sorting collapses all 8 to one rendering.Change
Sort a copy of the peers on
(Address, AddressV6, Name)before rendering.Addressempty on every peer, soAddressalone would tie and leave those deployments unstable.cmd/agentretains the slice aslatestStateand re-applieswg.Syncon it, andagent.UpdatePeerNameMappingreads it.Tests
Three new tests, each watched failing first:
TestSyncSkipsRestoreWhenPeerOrderChanges— reordering the same peer set must not re-apply (failed:want 1 invocation, got 2)TestSyncSkipsRestoreWhenIPv6OnlyPeerOrderChanges— same for IPv6-only peers with emptyAddress(failed:want 1, got 2)TestSyncDoesNotReorderCallerPeers—Syncmust not mutate the caller's slice (failed:index 0 is 10.8.0.2, want 10.8.0.4)They drive the real
iptables-restoreexec path via the existing stub-on-PATH helper and count invocations. All 8 tests in the package pass.Deploy note
This is an agent-layer change, so bumping it recreates every tenant's WG pod (brief reconnect on all tenants). LibreNMS email alerting is being disabled fleet-wide for the roll and restored after handshakes are confirmed re-established.
🤖 Generated with Claude Code