Repository navigation
feat: Report whether load balancer routes are valid - #713
Conversation
|
|
Connect each load balancer listener to its backend service and report in status whether the route is accepted, and why not when it isn't, so customers can see and fix a misconfigured route. Key changes: - Resolve named service ports and bound IPv6 interfaces for each listener - Let the older route win when two routes claim one listener - Report unsupported options (TLS, SNI, shared addresses) instead of partly applying them - Keep routes from sharing a backend port until the data plane supports it
ff35564 to
c128a94
Compare
kevwilliams
left a comment
kevwilliams
left a comment
There was a problem hiding this comment.
Hold: the backend socket conflict check grows exponentially with the number of routes in a namespace. For each older peer it recursively re-runs the same check on that peer with no memoization, so reconciling the newest of N non-conflicting routes on one network does about 2^N backend resolutions. Measured with the fake client: 6 routes 7ms, 10 routes 95ms, 12 routes 390ms, 14 routes 1.7s, roughly 4x per two routes added. Every NetworkInterface or route event in the namespace requeues every route, so a project with around 20 routes would tie up the controller for minutes per event. Computing socket ownership once per namespace in age order (or memoizing the blocked result per route and parent) keeps the transitive rule and makes it linear. A test that reconciles 30 or more routes would lock that in. Smaller gaps that need not block: no test fails if the same-timestamp name tie-breaks are inverted in either the listener owner or the socket owner, or if the same-network scope on socket claims is removed.
Deciding whether an older route owns a backend socket re-ran the same check for every older peer without remembering the answer, so reconciling one route cost about 2^N backend resolutions for N routes in a namespace. Key changes: - Remember each route's socket ownership and resolved backends per reconcile - Test 32 routes resolve each route's backends a bounded number of times - Test same-timestamp tie-breaks for listener and socket owners - Test socket claims stay scoped to one network
|
Fixed. The socket check now remembers each route's answer within a reconcile, so the work grows linearly with the number of routes. New tests cover that growth with 32 routes, name tie-breaks for listener and socket owners, and that socket claims never cross networks. Generated by Claude Code |
kevwilliams
left a comment
kevwilliams
left a comment
There was a problem hiding this comment.
Verified a 30-route reconcile now takes 20ms, and removing the memo, flipping either tie-break, or dropping network scope each fails a test.
ecv
left a comment
ecv
left a comment
There was a problem hiding this comment.
Approve.
Ran at c1ca2cf: the NLB controller tests pass with envtest 1.31.0 and the locations CRDs rendered.
CI at this head is green.
The new commit memoizes socket ownership and resolved backends per reconcile, which fixes the quadratic and recursive cost I flagged, at internal/controller/networkloadbalancerroute_controller.go:363 and :375.
The cache key is route UID, parent and listener, and each reconcile builds its own cache, so no result outlives a reconcile.
Nit: 19 comment lines in this file against the repo CLAUDE.md zero-comment rule.
Stack: based on #712, merge after it.
Summary
This connects each load balancer listener to its backend service and reports in status whether the route is accepted, and if not, why. When two routes claim the same listener, the older one wins. Unsupported options such as TLS or shared addresses are reported instead of being partly applied. Traffic is spread evenly across healthy backends in a location, and a route only shows as ready after a serving location confirms it.
Test plan
Depends on #712. Replaces #625. Related to #611 and datum-cloud/enhancements#884