Repository navigation
feat: Tell serving locations what load balancers need - #716
Conversation
|
|
f801336 to
eb6c9df
Compare
kevwilliams
left a comment
kevwilliams
left a comment
There was a problem hiding this comment.
Approving. The new cell intent is published only in a Validate or Withdrawn state, nothing in this repo consumes it, and no path sets Programmed or Announced, so the cell-side identity checks from the cell contract are still a precondition for any later PR that programs traffic. Non-blocking: this controller reuses the namespace-wide sibling requeue and the uncached olderRouteClaimingBackend from #713, so that fix covers both callers; withdrawn intents are never deleted and will accumulate until the cell acknowledgment exists; and the tests still pass with the Allocated observedGeneration check or the Bound phase filter removed, so both lack a regression test.
Each location that will serve a load balancer now receives what it needs to serve: the address, port, backends and health check settings. Locations validate this request but do not program traffic yet. Key changes: - Publish scoped cell intent per listener and per-location IPv6 address - Hold back setups the data plane cannot serve yet, such as several addresses on one listener or more than 64 backends - Withdraw old intent when a route is removed or an address changes
eb6c9df to
8c847c4
Compare
Guard two cell intent rules that had no regression test. Key changes: - withhold intent while an address allocation reports an older generation - leave interfaces that are not bound out of the published backends
|
Added the two regression tests: one for an address allocation from an older generation and one for a backend that isn't bound. The first fails if the generation check is removed. The bound check in the route resolver repeats the service membership filter, so the second test fails only when both are removed. I left the duplicate check in as a second guard. Generated by Claude Code |
kevwilliams
left a comment
kevwilliams
left a comment
There was a problem hiding this comment.
Verified intents stay Validate or Withdrawn with no consumer, and the new tests kill the stale-generation mutant; the Bound check duplicates matchingInterfaces.
ecv
left a comment
ecv
left a comment
There was a problem hiding this comment.
Approve.
Ran at c352b72: the cell intent tests pass with envtest 1.31.0.
CI at this head is green.
The rebase adds a per-call claim cache at internal/controller/networkloadbalancercellintent_controller.go:114 that reuses the memoization from #713, with no other change to the intent.
The new tests cover a stale allocation generation and an unbound interface left out of backends.
The open gaps are design work tracked in #720: the cell consumer, the backend health producer, a route condition for the 64-backend limit, and deleting withdrawn intents from the hub.
Reword the body line "The location validates this" to say the intent is published and propagated, with the consumer in #720.
Nit: the PR title says "what each load balancer needs" and the commit says "what load balancers need".
Stack: based on #715, merge last.
scotwells
changed the title
The merge-base changed after approval.
Summary
Each location that will serve a load balancer now receives what it needs to serve: the address, port, backends and health check settings. This request is published to the location, but nothing there acts on it or programs traffic yet; that work is tracked in #720. Setups the data plane can't handle today, such as several addresses on one listener or more than 64 backends, are held back instead of being partly applied. Removing a route or changing an address withdraws the old request.
Test plan
Depends on #715. Replaces #628. Related to #611 and datum-cloud/enhancements#884