Repository navigation
feat: Pick load balancer backends by label and bind them automatically - #809
Conversation
updateAcceptedCondition returned early whenever Accepted was already True, so the condition kept the observedGeneration of the spec it first saw. After any edit to a rule's spec, kubectl wait --for=condition=Accepted treats the condition as stale and times out, which stalled the lab's deploy:galactic-gateway once its rules changed form. It now returns early only when Accepted is True for the rule's current generation, and otherwise rewrites the condition with the new one. Related to #799 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Two backends of one VIP on one node could not both take traffic. The gateway encapsulated a flow to the node's End.DT46 uSID, which names the node and the VRF but not the backend, and the node kept one ingress vip_xlat_table row per VIP and port, so every flow went to the oldest binding's backend and the next one reported Conflict. A backend now has a slot, srv6.BackendSlot: a 16-bit FNV-1a hash of its address and port that is never 0. uformat carries it in bytes 10-11 of the uSID, and ComputeBackendSID builds a node uSID with it. usid_ingress reads the slot from the outer destination before decapsulating and keys the ingress vip_xlat lookup on it. It takes the key's former always-zero pad field, so vip_xlat_table keeps its 32-byte key and the pinned map is not recreated on upgrade (#768). Egress rows keep slot 0, since a reply's source address and port already name its backend. vipxlatmap's ingress row operations, ServiceVIPBindingReconciler's rows and conflict checks, and the vip xlat CLI (a --slot flag and a SLOT column) carry the slot, so two bindings of one VIP with different backends both bind. A conflict now needs the same backend address and port, or a slot collision. Related to #799 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A NetworkRule listed fixed backend addresses, and each backend's node needed a hand-written ServiceVIPBinding before it answered on the VIP. When an instance moved node or changed address, the gateway followed it but the binding did not, so the rule reported Accepted and Programmed while that backend's connections dropped. datum-cloud/network#29 replaced spec.backends with backendSelector and backendPort and added a required vpcRef to ServiceVIPBinding. go.mod pins network main at the #30 merge, which carries both #29 and the private service types main already uses. Gateway: buildDesiredRule expands the selector into the IPv6 interface addresses of the VPCAttachments it picks in the rule's own VPC, resolves each to the uSID of the node its attachment reports, and writes the backend's slot into that uSID. The reconciler watches VPCAttachments cluster-wide, so a backend that appears, moves or changes address reconverges every gateway. Selected attachments with no node or no IPv6 address show up under BackendsUnresolved. galactic-router: NetworkRuleBindingReconciler writes one binding per selected backend on its own node for the rule's IPv6 VIP, owned by the rule, labelled with its node and managed-by, with egressKind from the attachment's interface mode, and deletes the ones that no longer apply. Each node writes only its own bindings, so there is one writer per object. The rule gains a per-node <node>/BackendsBound condition. ServiceVIPBindingReconciler resolves the VRF from the binding's vpcRef, which replaces the address-containment search and its TODO(dsr-maglev). Both sides select backends through the same code, which also drops, in the same order everywhere, a backend whose address another attachment already claims or whose slot collides with another backend on its node, and reports it as unresolved rather than sending it flows it could not answer. A rule may carry at most one IPv6 VIP, because a backend node rewrites a reply's source to a single VIP; a rule with two fails to load and reports InvalidRule. A rule that loses Accepted, as it does whenever its namespace briefly has no NetworkGateway, keeps its bindings, so the backend nodes' vip_xlat_table rows survive until the gateways return. RBAC: galactic-router can create and delete servicevipbindings, read networkrules and write their status. galactic-gateway can read vpcattachments. Both RBAC preflights check the new watches, and the gateway's also checks bgpvrfinstances, which it already watched. Fixes #799 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The lab's NetworkRules listed one fixed backend per site, and hand-written ServiceVIPBindings bound it, because a node could bind a VIP's port to only one backend. Each site now runs two ns60 backends on its compute node, each with its own NAD, its own /96 and its own VPCAttachment, and the rules select them by label. Two attachments sharing one subnet would leave the VRF a single route to one backend's veth. The lab has no attachment controller, so publish-ns60-attachments.sh, run by deploy:ns60, writes each attachment's status from where its pod runs. The hand-written bindings are deleted: the compute node's galactic-router generates them, and deploy-galactic-gateway.sh and verify:gateway wait for and count them. verify-gateway-ingress.sh now fails unless every backend in a site answers at least one flow, which only passes while the gateway's per-backend slot reaches the node. The lab's cloud CRD pin moves to go.mod's cloud commit, which carries spec.interface.mode. Related to #799 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
4ab7d93 to
f1942cd
Compare
The drop tests called dropConflictingBackends directly, so removing its call from selectRuleBackends left every test passing while the gateway and the binding writer went back to planning flows and bindings for a repeated address or a colliding slot. A test now selects through selectRuleBackends and fails without that call. Also re-wraps a docs paragraph an earlier edit left with one over-long line. Related to #799 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Second-pass verdicts, both run on head f1942cd. pr-rereviewer: "VERDICT: hold. The commits are unsigned, and the wiring of pr-conventions-reviewer: "VERDICT: hold. Two open PRs (#738, #643) overlap this one, and #738 may be obsolete, so the human has to order them before this lands. The breaking CRD and rollout points above also need a human call." What they ran. The re-reviewer read the signature log and the diff between the first-pass and second-pass heads, ran go build ./..., ran go test on the controller and srv6 packages, and ran three tamper runs of the controller tests in a scratch worktree. The conventions reviewer read the PR metadata, the diff stat, the open PR list and the repository commit rules, ran kustomize build on the gateway overlays, tenant overlays, router base and gateway base, and diffed go.mod and config between base and head. The lab and root eBPF tests were not run by either reviewer; the author ran them (lab gateway and gateway-ingress checks on all three sites, root eBPF prog tests, the race-enabled unit suite, golangci-lint). Findings applied in the first pass: Accepted=False no longer deletes bindings, slot and duplicate-address conflicts are dropped in shared selection, a rule with more than one IPv6 VIP is refused, doc and RBAC-comment nits, and the history was split into four commits. After the second pass, c5b67ad adds the selectRuleBackends wiring test (removing the dropConflictingBackends call makes it fail) and re-wraps the flagged docs line. That commit sits on top of the head both reviewers read. Human decision: land this PR before #643, #580 and #738, and leave #738 as it is for now. No backwards compatibility is needed for the mixed-version rollout, since the load balancer is not deployed in production. A network release containing datum-cloud/network#29 and #30 is needed before the next galactic release. Commit signing is not required in this repository. The stale BackendsBound condition while a rule is unaccepted is accepted as minor, and commit subjects are under the 72-character limit. CI on c5b67ad: every check passed, including lint, unit tests, root unit tests and e2e. The root unit tests job first stalled on a package download until its timeout and passed on re-run. Merge method: merge, the only one the ruleset allows. The ruleset requires one approving review with code-owner review and last-push approval. The PR was already marked ready and had already merged on an approval of this head when auto-merge was requested, so auto-merge was not needed. |
2 participants
Summary
A load balancer rule named its backends as fixed addresses, every backend's node needed a hand-written binding, and two backends of one VIP on one node could not both take traffic.
datum-cloud/network#29 replaced the address list with a label selector, and staging and the lab already run its CRDs, so rules on galactic main stop loading until this lands.
The gateway now picks backends from the VPCAttachments the selector matches, each node writes the bindings for its own backends, and the gateway carries Maglev's choice to the node so two backends on one node each get their own flows.
Breaking changes
Rules must use the selector and a backend port, bindings are generated, and a rule may carry at most one IPv6 VIP. Delete any hand-written bindings before rolling this out.
Test plan
Fixes #799
🤖 Generated with Claude Code