From 6a7c364307480093c9307678d99f5727465e2597 Mon Sep 17 00:00:00 2001 From: Peter Sprygada Date: Thu, 8 Oct 2026 17:46:19 -0400 Subject: [PATCH] fix(router): Sweep vip_xlat_table rows no ServiceVIPBinding claims galactic-router now sweeps the table on every GC tick after the first pass. controller.VIPXlatSweeper takes the table's generation, lists every ServiceVIPBinding targeting the node, resolves each one's two rows through the same parse and VRF-context lookup the reconciler uses, and calls Reconcile with that live set. Rows registered after the snapshot survive, and skipping the initial pass gives the reconciler a full interval to re-register every live binding first. A binding whose VRF context does not resolve right now (VRF not yet on the node, or briefly gone) has no known Argument, so every row shaped like one of its rows (direction, slot, protocol, address, port) is kept under any Block and Argument, rather than deleted and written back once it resolves. A binding whose spec does not parse never wrote a row, and any error listing bindings or building the index skips the pass. resolveVIPBindingRows is split into parseVIPBindingRows and a VRF placement step so the sweep reuses it, and runCmd's GC ticker moves into runGCTicker to stay under the gocyclo limit. usid.c has no slot-0 fallback, so galactic-gateway and galactic-router must be upgraded together across #799. The docs now say so. Co-Authored-By: Claude Opus 5.5 --- cmd/galactic-router/root.go | 65 +++-- docs/agents/ARCHITECTURE-ROUTER.md | 9 +- docs/gateway/configuration.md | 6 + .../servicevipbinding_controller.go | 30 ++- internal/controller/vipxlat_sweep.go | 176 +++++++++++++ internal/controller/vipxlat_sweep_test.go | 231 ++++++++++++++++++ internal/gc/gc.go | 6 +- 7 files changed, 489 insertions(+), 34 deletions(-) create mode 100644 internal/controller/vipxlat_sweep.go create mode 100644 internal/controller/vipxlat_sweep_test.go diff --git a/cmd/galactic-router/root.go b/cmd/galactic-router/root.go index 67e215fc..412b6536 100644 --- a/cmd/galactic-router/root.go +++ b/cmd/galactic-router/root.go @@ -167,13 +167,20 @@ func runCmd(cfg *config.RouterConfig) error { // matters only once a tap-kind binding is reconciled here, at which point // the reconciler reports a clear error rather than silently doing nothing. // Veth bindings work regardless. - var vipTranslationTable controller.VIPTranslationTable + // + // The same handle backs the GC ticker's vip_xlat_table sweep, which is + // skipped while the pin is missing. + var ( + vipTranslationTable controller.VIPTranslationTable + vipXlatSweeper *controller.VIPXlatSweeper + ) vipXlatTable, vipXlatCloser, vipXlatErr := vipxlatmap.OpenPinnedVipXlatTable(attach.PinDir) if vipXlatErr != nil { ctrl.Log.Error(vipXlatErr, "vip_xlat_table not available on this node; "+ "tap-kind ServiceVIPBinding objects will fail to bind until the eBPF uSID datapath is loaded") } else { vipTranslationTable = vipXlatTable + vipXlatSweeper = &controller.VIPXlatSweeper{Client: mgr.GetClient(), NodeName: nodeName, Table: vipXlatTable} defer vipXlatCloser.Close() //nolint:errcheck // best-effort close of our own fd at shutdown } @@ -294,28 +301,7 @@ func runCmd(cfg *config.RouterConfig) error { return fmt.Errorf("setup GC controller: %w", err) } - // The GC ticker runs until the manager's context is cancelled. The - // first pass waits for informer caches to sync, so it does not see an - // empty advertisement list and delete live VRFs. - go func() { - ticker := time.NewTicker(cfg.GCInterval) - defer ticker.Stop() - - if !mgr.GetCache().WaitForCacheSync(ctx) { - log.Printf("GC: cache sync failed, skipping initial pass") - return - } - gcRec.RunGC(ctx) - - for { - select { - case <-ctx.Done(): - return - case <-ticker.C: - gcRec.RunGC(ctx) - } - } - }() + go runGCTicker(ctx, mgr, cfg.GCInterval, gcRec, vipXlatSweeper) if err := mgr.Start(ctx); err != nil { return fmt.Errorf("manager exited: %w", err) @@ -346,6 +332,39 @@ func runCmd(cfg *config.RouterConfig) error { return nil } +// runGCTicker runs the GC controller's passes every interval until ctx is +// cancelled. The first pass waits for informer caches to sync, so it does not +// see an empty advertisement list and delete live VRFs. +// +// vipXlatSweeper, nil when vip_xlat_table is not open, runs only on ticks, +// never in that first pass, so the ServiceVIPBinding reconciler re-registers +// every live binding before the first sweep. +func runGCTicker( + ctx context.Context, mgr ctrl.Manager, interval time.Duration, + gcRec *controller.GCReconciler, vipXlatSweeper *controller.VIPXlatSweeper, +) { + ticker := time.NewTicker(interval) + defer ticker.Stop() + + if !mgr.GetCache().WaitForCacheSync(ctx) { + log.Printf("GC: cache sync failed, skipping initial pass") + return + } + gcRec.RunGC(ctx) + + for { + select { + case <-ctx.Done(): + return + case <-ticker.C: + gcRec.RunGC(ctx) + if vipXlatSweeper != nil { + vipXlatSweeper.Sweep(ctx) + } + } + } +} + // setupVIPBindingControllers registers the two halves of this node's DSR // backend side: NetworkRuleBindingReconciler, which writes a ServiceVIPBinding // for every NetworkRule backend whose VPCAttachment is on this node, and diff --git a/docs/agents/ARCHITECTURE-ROUTER.md b/docs/agents/ARCHITECTURE-ROUTER.md index 21bb65d8..27f472f0 100644 --- a/docs/agents/ARCHITECTURE-ROUTER.md +++ b/docs/agents/ARCHITECTURE-ROUTER.md @@ -82,7 +82,8 @@ galactic/ │ ├── metadata/ # Build-time version info (Version, GitCommit, etc.) │ ├── gc/ # Orphaned BGPAdvertisement/BGPVRFInstance CRD, stale │ │ # kernel VRF, and eBPF vrf_table entry cleanup, -│ │ # driven by the GC controller +│ │ # driven by the GC controller (the vip_xlat_table +│ │ # sweep is internal/controller/vipxlat_sweep.go) │ └── plumbing/ # Low-level kernel and network primitives (shared with │ ├── intf/ # the CNI chain — see ARCHITECTURE-CNI.md's own copy │ ├── srv6/ # of this tree for the CNI-side entries) @@ -180,7 +181,8 @@ lives in `root.go`'s `runCmd`: NetworkRuleBinding, NodeDeparture, ServiceRoutePolicy, and GC (the GC controller also starts a ticker goroutine that waits for cache sync, then runs on `--gc-interval`, default 5m; NodeDeparture sweeps on the - same interval). + same interval, and when `vip_xlat_table` opened in step 7, each GC tick + after the first pass also sweeps it). 11. `mgr.Start(ctx)` — blocks until the signal-handler context is cancelled. This is identical whether the pod is running in the plain role @@ -280,7 +282,8 @@ co-located `galactic-gateway` container's own health port on the same - **A tenant VRF route that fails to install is retried, not dropped.** `processEVPNPath` writes a VRF-scoped path's route into `egress_route_table` through `internal/runtime/gobgp/vrfroutes.go`. `Register` resolves the SID's uplink and neighbor MAC at write time, which fails while an uplink is still coming up after a restart, and BGP never redelivers an unchanged best path, so before #676 such a route stayed missing until the path changed upstream. `vrfRoutes` keeps each failed route in a pending set and retries it whenever the main table changes (1s debounce) and every 30s, until it installs or its path is withdrawn; removing a VRF drops its pending routes. One mutex serializes every write to the map from this runtime, taken per route during a retry so an unresolvable neighbor (up to 2s) does not hold up the RIB watcher for the whole pass. `EgressRouteTable.Refresh` cannot cover this: it walks existing entries only. The pending count, and the plain-route reconciler's failing count, are exported as `galactic_router_route_install_failing{kind="vrf"|"plain"}`, and `GalacticRouterRoutesNotInstalled` fires when either stays above 0 for 10 minutes. - **`ServiceVIPBinding`s are generated, one per node per backend.** `NetworkRuleBindingReconciler` (`internal/controller/networkrule_binding_controller.go`) runs on every node and writes a binding for each IPv6 VIP of each accepted `NetworkRule` and each backend the rule's `backendSelector` places on this node, through the same `selectRuleBackends` the gateway uses, so the gateway never sends a backend's flows to a node with no binding planned for it. Each node writes and deletes only its own bindings (labelled `galactic.datum.net/binding-node` and `app.kubernetes.io/managed-by: galactic-router`), so there is one writer per object and no leader election. A binding is owned by its rule, records the rule's `vpcRef` (which names the VRF its rows go in) and takes `egressKind` from the attachment's interface mode. The node summarizes its bindings on the rule as `/BackendsBound`; for an invalid rule only the nodes that served it (held bindings going in, or already report `InvalidRule`) write one, since the gateways already report the error and a condition from every node would be a status write per node per edit. Several backends of one VIP can share a node: the gateway writes each backend's slot into the uSID and `usid.c` keys the ingress row on it (#799). - **Surviving routers clean up after a departed node.** Only a node's own `NetworkRuleBindingReconciler` clears its `/BackendsBound` conditions and deletes its generated bindings, and only its own `ServiceVIPBindingReconciler` removes the teardown finalizer from bindings targeting it. So once a node's `Node` object is deleted, `NodeDepartureReconciler` (`internal/controller/node_departure_controller.go`) on every other router removes that node's `BackendsBound` condition from every `NetworkRule`, deletes its generated bindings and removes the finalizer from every binding whose `targetRef.name` names it, hand-written ones included; the kernel rows went with the node. Without it the conditions stayed forever and a deleted rule's bindings stayed `Terminating`. Every Node create and delete maps to one sweep key, so the sweep runs on the initial Node list (startup), on every Node delete and every `--gc-interval`, and a router down during the delete still converges. A node missing from the cached Node list counts as departed only once an uncached `Get` returns NotFound. Every router sweeps, with no leader election: status writes and finalizer patches retry on conflict (the patch carries the resourceVersion) and NotFound counts as done. -- **Deleting a `ServiceVIPBinding` does not need its VRF on the node.** A binding's two `vip_xlat_table` rows are keyed on its VRF's uSID Argument. `ServiceVIPBindingReconciler` reads it from the `BGPVRFInstance` its `vpcRef` names on this node, and that is gone once the VPC leaves the node. Before #750, deletion re-derived it, failed, held the finalizer forever and left both rows in the map. Teardown now finds the rows by value with `vipxlatmap.UnregisterBinding`: since #764 each key carries the VIP or backend address, and since #799 the ingress key also carries the backend's slot, so the binding's ingress row (its VIP, port and slot, rewriting to its backend) and egress row (its backend and port, rewriting to its VIP) are found exactly, under any Argument. Each row is removed only while it still holds the binding's values, so a row another binding has since written stays. When the VRF context still resolves, `UnregisterBindingAt` applies the same checks at that location too. Neither pass deletes a row without first reading it, and a process-wide lock keeps a bind from landing between the read and the delete. A value check cannot tell two bindings with identical values apart, so while such a twin is live on the node, deleting one removes nothing. Rows exist only for IPv6 addresses, so an IPv4 or invalid address skips both passes instead of blocking deletion. Only a failed kernel delete keeps the finalizer. Nothing sweeps `vip_xlat_table` periodically, so rows left behind by a finalizer cleared by hand stay until removed with `galactic-router vip xlat list` and `vip xlat remove --protocol --vip --port --backend --backend-port` (`--slot` defaults to the backend's own), run in that node's `galactic-router` pod (`kubectl exec -- /galactic-router vip xlat list`). +- **Deleting a `ServiceVIPBinding` does not need its VRF on the node.** A binding's two `vip_xlat_table` rows are keyed on its VRF's uSID Argument. `ServiceVIPBindingReconciler` reads it from the `BGPVRFInstance` its `vpcRef` names on this node, and that is gone once the VPC leaves the node. Before #750, deletion re-derived it, failed, held the finalizer forever and left both rows in the map. Teardown now finds the rows by value with `vipxlatmap.UnregisterBinding`: since #764 each key carries the VIP or backend address, and since #799 the ingress key also carries the backend's slot, so the binding's ingress row (its VIP, port and slot, rewriting to its backend) and egress row (its backend and port, rewriting to its VIP) are found exactly, under any Argument. Each row is removed only while it still holds the binding's values, so a row another binding has since written stays. When the VRF context still resolves, `UnregisterBindingAt` applies the same checks at that location too. Neither pass deletes a row without first reading it, and a process-wide lock keeps a bind from landing between the read and the delete. A value check cannot tell two bindings with identical values apart, so while such a twin is live on the node, deleting one removes nothing. Rows exist only for IPv6 addresses, so an IPv4 or invalid address skips both passes instead of blocking deletion. Only a failed kernel delete keeps the finalizer. Rows left behind some other way, by a finalizer cleared by hand, a crash or a manual write, are removed by the periodic sweep below; to remove one sooner, use `galactic-router vip xlat list` and `vip xlat remove --protocol --vip --port --backend --backend-port` (`--slot` defaults to the backend's own), run in that node's `galactic-router` pod (`kubectl exec -- /galactic-router vip xlat list`). +- **`vip_xlat_table` is swept on the GC ticker.** `controller.VIPXlatSweeper` (`internal/controller/vipxlat_sweep.go`) runs on every GC tick after the first, never in the initial pass at startup, so the `ServiceVIPBinding` reconciler has re-registered every live binding first. It takes the table's generation, lists every `ServiceVIPBinding` targeting this node (including ones being deleted, whose finalizer owns their rows), resolves each one's two rows the way the reconciler does, and calls `VipXlatTable.Reconcile`, which removes every row not in that set unless it was registered after the snapshot. The live set is by key, so a row is kept whatever value it holds. A binding whose VRF context does not resolve right now has no known Argument, so every row shaped like one of its rows (direction, slot, protocol, address and port, under any Block and Argument) is kept instead. A binding whose spec does not parse never wrote a row and protects nothing, and any error listing bindings or building the index skips the whole pass. This is what removes the slot-0 ingress rows a router built before #799 wrote: the current key carries the backend's slot, so nothing else ever matches them. `usid.c` has no slot-0 fallback, so `galactic-gateway` and `galactic-router` have to be upgraded together across #799; a node whose router has not yet rewritten its rows with slots drops the flows a new gateway sends it. --- diff --git a/docs/gateway/configuration.md b/docs/gateway/configuration.md index 003b702c..61a6f063 100644 --- a/docs/gateway/configuration.md +++ b/docs/gateway/configuration.md @@ -584,6 +584,12 @@ and protocol, in the same VPC. The older binding keeps serving. The newer one is not programmed and reports `Bound=False` with reason `Conflict`, naming the binding that holds the row. +`usid.c` matches only the slot the gateway sent, with no fallback to the +slot-0 ingress rows a `galactic-router` built before #799 wrote, so upgrade +`galactic-gateway` and `galactic-router` together. Each router rewrites its +bindings' rows with slots when it starts, and its periodic `vip_xlat_table` +sweep removes the old slot-0 rows. + Each node summarizes its bindings on the rule as `/BackendsBound`: `True` with reason `Bound` once every one reports `Bound`, `False` with reason `BindingsNotBound` naming each that diff --git a/internal/controller/servicevipbinding_controller.go b/internal/controller/servicevipbinding_controller.go index e5d0ccfd..9d15be6e 100644 --- a/internal/controller/servicevipbinding_controller.go +++ b/internal/controller/servicevipbinding_controller.go @@ -497,6 +497,20 @@ type vipBindingRows struct { func resolveVIPBindingRows( idx *backendSIDIndex, nodeName string, binding *bgpv1alpha1.ServiceVIPBinding, ) (vipBindingRows, error) { + rows, err := parseVIPBindingRows(binding) + if err != nil { + return vipBindingRows{}, err + } + block, argument, err := resolveVIPBindingContextFromIndex(idx, nodeName, binding.Spec.VPCRef) + if err != nil { + return vipBindingRows{}, fmt.Errorf("resolve VRF context for VIP binding: %w", err) + } + return rows.at(block, argument), nil +} + +// parseVIPBindingRows derives the two rows binding claims from its spec alone, +// with their block and argument left zero for the caller to fill in with at. +func parseVIPBindingRows(binding *bgpv1alpha1.ServiceVIPBinding) (vipBindingRows, error) { vipIP, err := netip.ParseAddr(binding.Spec.VIPAddress) if err != nil { return vipBindingRows{}, fmt.Errorf("invalid vipAddress %q: %w", binding.Spec.VIPAddress, err) @@ -513,19 +527,14 @@ func resolveVIPBindingRows( return vipBindingRows{}, err } - block, argument, err := resolveVIPBindingContextFromIndex(idx, nodeName, binding.Spec.VPCRef) - if err != nil { - return vipBindingRows{}, fmt.Errorf("resolve VRF context for VIP binding: %w", err) - } - vipPort := uint16(binding.Spec.Port) //nolint:gosec // kubebuilder-validated 1-65535 backendPort := uint16(binding.Spec.BackendPort) //nolint:gosec // kubebuilder-validated 1-65535 slot := srv6.BackendSlot(backendIP, backendPort) return vipBindingRows{ - ingress: vipRow{block: block, argument: argument, slot: slot, proto: proto, addr: vipIP, port: vipPort}, - egress: vipRow{egress: true, block: block, argument: argument, proto: proto, addr: backendIP, port: backendPort}, + ingress: vipRow{slot: slot, proto: proto, addr: vipIP, port: vipPort}, + egress: vipRow{egress: true, proto: proto, addr: backendIP, port: backendPort}, vipAddr: net.IP(vipIP.AsSlice()), backendAddr: net.IP(backendIP.AsSlice()), vipPort: vipPort, @@ -533,6 +542,13 @@ func resolveVIPBindingRows( }, nil } +// at returns rows placed in the VRF identified by (block, argument). +func (rows vipBindingRows) at(block uint64, argument uint16) vipBindingRows { + rows.ingress.block, rows.ingress.argument = block, argument + rows.egress.block, rows.egress.argument = block, argument + return rows +} + // vipPeer is another live binding on this node and the rows it claims. type vipPeer struct { binding *bgpv1alpha1.ServiceVIPBinding diff --git a/internal/controller/vipxlat_sweep.go b/internal/controller/vipxlat_sweep.go new file mode 100644 index 00000000..60e328f2 --- /dev/null +++ b/internal/controller/vipxlat_sweep.go @@ -0,0 +1,176 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package controller + +import ( + "context" + "log/slog" + + "sigs.k8s.io/controller-runtime/pkg/client" + + "go.datum.net/galactic/internal/gc" + "go.datum.net/galactic/internal/plumbing/ebpf/vipxlatmap" + bgpv1alpha1 "go.datum.net/network/api/v1alpha1" +) + +// VIPXlatSweepTable is the part of *vipxlatmap.VipXlatTable VIPXlatSweeper +// drives, so tests can run it against a fake. +type VIPXlatSweepTable interface { + Generation() uint64 + List() ([]vipxlatmap.Entry, error) + Reconcile(live map[vipxlatmap.Key]struct{}, cutoff uint64) ([]vipxlatmap.Entry, error) +} + +// VIPXlatSweeper removes vip_xlat_table rows that no ServiceVIPBinding on this +// node claims. ServiceVIPBindingReconciler removes a binding's rows when the +// binding is deleted, but nothing else ever does, so without this sweep a row +// stays for good once its owner is gone without that teardown: a finalizer +// cleared by hand, a crash between writing a row and recording the binding, a +// manual write, or a row whose key the current code no longer writes, such as +// the slot-0 ingress rows a router built before #799 left behind. +// +// The live set is every row the node's bindings claim, resolved exactly as the +// reconciler resolves them, so a row is kept whatever value it holds and +// whichever of two conflicting bindings owns it. A binding being deleted still +// counts: its finalizer removes its rows, and the sweep does not race it. +// +// A binding whose VRF context does not resolve right now, such as one whose +// VRF has not reached the node yet or has briefly left it, has no known block +// or argument, so its rows cannot be put in the live set by key. Every row +// shaped like one of its two rows (direction, slot, protocol, address and +// port) is kept instead, under any block and argument. Deleting them would +// only have the reconciler write them back once the VRF resolves, and teardown +// already finds them by value. A binding whose spec does not parse never +// registered a row, so it protects nothing. +// +// Any error building the live set skips the whole pass rather than sweep +// against a partial view. +type VIPXlatSweeper struct { + Client client.Client + NodeName string + Table VIPXlatSweepTable +} + +// Sweep runs one pass and returns what it removed. +// +// It trusts the API's binding list, not this process's own registrations, so +// it is safe on a fresh process once the informer caches have synced. +// galactic-router still runs it only from the GC ticker, never in the initial +// pass at startup, so the reconciler has had a full interval to re-register +// every live binding before the first sweep, as VipXlatTable.Reconcile asks. +func (s *VIPXlatSweeper) Sweep(ctx context.Context) gc.CleanupResult { + var result gc.CleanupResult + + // Captured before listing bindings, so a row the reconciler registers + // for a binding created after the list below survives this pass. + cutoff := s.Table.Generation() + + live, unresolved, err := s.liveRows(ctx) + if err != nil { + slog.Error("GC: failed to build live set for eBPF vip_xlat_table sweep, skipping this tick", "err", err) + result.Errors++ + return result + } + + if len(unresolved) > 0 { + entries, err := s.Table.List() + if err != nil { + slog.Error("GC: failed to list eBPF vip_xlat_table for sweep, skipping this tick", "err", err) + result.Errors++ + return result + } + for _, e := range entries { + for _, rows := range unresolved { + if rows.shapes(e) { + live[e.Key] = struct{}{} + break + } + } + } + } + + removed, err := s.Table.Reconcile(live, cutoff) + for _, e := range removed { + slog.Info("GC: removed stale eBPF vip_xlat_table entry", + "direction", e.Direction, "block", e.Block, "argument", e.Argument, "slot", e.Slot, + "proto", e.Proto, "addr", e.Addr, "port", e.Port) + } + result.EBPFVIPXlatEntriesRemoved = len(removed) + if err != nil { + slog.Error("GC: errors while reconciling eBPF vip_xlat_table", "err", err) + result.Errors++ + } + if result.EBPFVIPXlatEntriesRemoved > 0 || result.Errors > 0 { + slog.Info("eBPF vip_xlat_table GC sweep complete", + "removed", result.EBPFVIPXlatEntriesRemoved, "errors", result.Errors) + } + return result +} + +// liveRows returns the keys of every row this node's bindings claim, and the +// rows, without a block or argument, of every binding whose VRF context does +// not resolve. +func (s *VIPXlatSweeper) liveRows( + ctx context.Context, +) (map[vipxlatmap.Key]struct{}, []vipBindingRows, error) { + list := &bgpv1alpha1.ServiceVIPBindingList{} + if err := s.Client.List(ctx, list); err != nil { + return nil, nil, err + } + + live := make(map[vipxlatmap.Key]struct{}) + var unresolved []vipBindingRows + indexes := make(map[string]*backendSIDIndex) + for i := range list.Items { + b := &list.Items[i] + if b.Spec.TargetRef.Name != s.NodeName { + continue + } + rows, err := parseVIPBindingRows(b) + if err != nil { + continue // never registered a row + } + idx, ok := indexes[b.Namespace] + if !ok { + if idx, err = buildBackendSIDIndex(ctx, s.Client, b.Namespace); err != nil { + return nil, nil, err + } + indexes[b.Namespace] = idx + } + block, argument, err := resolveVIPBindingContextFromIndex(idx, s.NodeName, b.Spec.VPCRef) + if err != nil { + slog.Debug("GC: ServiceVIPBinding does not resolve; keeping every vip_xlat_table row shaped like its own", + "binding", client.ObjectKeyFromObject(b), "reason", err.Error()) + unresolved = append(unresolved, rows) + continue + } + rows = rows.at(block, argument) + live[rows.ingress.key()] = struct{}{} + live[rows.egress.key()] = struct{}{} + } + return live, unresolved, nil +} + +// key returns the vip_xlat_table key r is written under. +func (r vipRow) key() vipxlatmap.Key { + return vipxlatmap.Key{ + Block: r.block, Argument: r.argument, Slot: r.slot, Proto: r.proto, Addr: r.addr, Port: r.port, + } +} + +// shapes reports whether e has the direction, slot, protocol, address and port +// of one of rows' two rows, under any block and argument. +func (rows vipBindingRows) shapes(e vipxlatmap.Entry) bool { + var row vipRow + switch e.Direction { + case vipxlatmap.DirectionIngress: + row = rows.ingress + case vipxlatmap.DirectionEgress: + row = rows.egress + default: + return false + } + return e.Slot == row.slot && e.Proto == row.proto && e.Addr == row.addr && e.Port == row.port +} diff --git a/internal/controller/vipxlat_sweep_test.go b/internal/controller/vipxlat_sweep_test.go new file mode 100644 index 00000000..b30fb221 --- /dev/null +++ b/internal/controller/vipxlat_sweep_test.go @@ -0,0 +1,231 @@ +// Copyright 2026 Datum Cloud, Inc. +// +// SPDX-License-Identifier: AGPL-3.0-or-later + +package controller + +import ( + "context" + "errors" + "net" + "net/netip" + "testing" + + "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/client/fake" + "sigs.k8s.io/controller-runtime/pkg/client/interceptor" + + "go.datum.net/galactic/internal/plumbing/ebpf/vipxlatmap" + "go.datum.net/galactic/internal/plumbing/srv6" + bgpv1alpha1 "go.datum.net/network/api/v1alpha1" +) + +const ( + testSweepOrphanVIP = "2001:db8:5:5::200" + testSweepOrphanBackend = "fd20:60::9:9" + // testSweepUnresolvedArgument is the Argument the unresolvable binding's + // rows were written under, before its VRF left the node. + testSweepUnresolvedArgument = uint16(77) +) + +// sweepRow is one row a previous galactic-router wrote, before the sweep runs. +type sweepRow struct { + name string + egress bool + argument uint16 // 0 means the test binding's resolved Argument + slot uint16 + vip string + backend string +} + +// writeSweepRows writes rows through a VipXlatTable of their own, as a +// previous process would have, so the sweeper's fresh table over the same map +// reports every one at generation 0. +func writeSweepRows(t *testing.T, mem *memTable, block uint64, argument uint16, rows []sweepRow) { + t.Helper() + prev := vipxlatmap.NewVipXlatTable(mem) + for _, r := range rows { + arg := argument + if r.argument != 0 { + arg = r.argument + } + vip, backend := net.ParseIP(r.vip), net.ParseIP(r.backend) + var err error + if r.egress { + err = prev.RegisterEgress(block, arg, ipProtoTCP, backend, 30080, vip, 8080) + } else { + err = prev.RegisterIngress(block, arg, r.slot, ipProtoTCP, vip, 8080, backend, 30080) + } + if err != nil { + t.Fatalf("write row %s: %v", r.name, err) + } + } +} + +// liveSlot is the slot the test binding's ingress row is keyed on. +func liveSlot() uint16 { + return srv6.BackendSlot(netip.MustParseAddr(testVIPBindingBackendAddr), 30080) +} + +func TestVIPXlatSweeper_Sweep(t *testing.T) { + liveIngress := sweepRow{name: "live ingress", slot: liveSlot(), + vip: testVIPBindingVIPAddr, backend: testVIPBindingBackendAddr} + liveEgress := sweepRow{name: "live egress", egress: true, + vip: testVIPBindingVIPAddr, backend: testVIPBindingBackendAddr} + legacyIngress := sweepRow{name: "slot-0 ingress", slot: 0, + vip: testVIPBindingVIPAddr, backend: testVIPBindingBackendAddr} + orphanIngress := sweepRow{name: "orphan ingress", slot: 0x1234, + vip: testSweepOrphanVIP, backend: testSweepOrphanBackend} + orphanEgress := sweepRow{name: "orphan egress", egress: true, + vip: testSweepOrphanVIP, backend: testSweepOrphanBackend} + unresolvedIngress := sweepRow{name: "unresolved ingress", argument: testSweepUnresolvedArgument, + slot: liveSlot(), vip: testVIPBindingVIPAddr, backend: testVIPBindingBackendAddr} + unresolvedEgress := sweepRow{name: "unresolved egress", egress: true, argument: testSweepUnresolvedArgument, + vip: testVIPBindingVIPAddr, backend: testVIPBindingBackendAddr} + + tests := []struct { + name string + // vpcRef is the test binding's vpcRef; empty means no binding. + vpcRef string + otherNode bool + rows []sweepRow + wantKept []sweepRow + }{ + { + name: "slot-0 ingress row of a live binding is removed, its live rows kept", + vpcRef: testVPCRef, + rows: []sweepRow{liveIngress, liveEgress, legacyIngress}, + wantKept: []sweepRow{liveIngress, liveEgress}, + }, + { + name: "rows no binding claims are removed", + vpcRef: testVPCRef, + rows: []sweepRow{liveIngress, liveEgress, orphanIngress, orphanEgress}, + wantKept: []sweepRow{liveIngress, liveEgress}, + }, + { + name: "an unresolvable binding keeps rows shaped like its own under any VRF", + vpcRef: "vpc-not-on-node", + rows: []sweepRow{unresolvedIngress, unresolvedEgress, orphanIngress}, + wantKept: []sweepRow{unresolvedIngress, unresolvedEgress}, + }, + { + name: "an unresolvable binding does not keep its slot-0 ingress row", + vpcRef: "vpc-not-on-node", + rows: []sweepRow{ + unresolvedIngress, unresolvedEgress, + {name: "unresolved slot-0 ingress", argument: testSweepUnresolvedArgument, slot: 0, + vip: testVIPBindingVIPAddr, backend: testVIPBindingBackendAddr}, + }, + wantKept: []sweepRow{unresolvedIngress, unresolvedEgress}, + }, + { + name: "a binding on another node keeps nothing here", + vpcRef: testVPCRef, + otherNode: true, + rows: []sweepRow{liveIngress, liveEgress}, + }, + { + name: "no bindings removes every row", + rows: []sweepRow{liveIngress, orphanEgress}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + router, _, vrf := newBackendFixtures(testVPCRef) + objs := []client.Object{router, vrf} + if tt.vpcRef != "" { + node := testComputeNodeName + if tt.otherNode { + node = "some-other-node" + } + b := newTestServiceVIPBinding(bgpv1alpha1.ServiceVIPBindingEgressKindTap, node) + b.Spec.VPCRef = tt.vpcRef + objs = append(objs, b) + } + c := fake.NewClientBuilder().WithScheme(newRuleTestScheme(t)).WithObjects(objs...).Build() + + block, argument, err := resolveVIPBindingContext(context.Background(), c, testNamespace, + testComputeNodeName, testVPCRef) + if err != nil { + t.Fatalf("resolveVIPBindingContext: %v", err) + } + mem := newMemTable() + writeSweepRows(t, mem, block, argument, tt.rows) + + table := vipxlatmap.NewVipXlatTable(mem) + s := &VIPXlatSweeper{Client: c, NodeName: testComputeNodeName, Table: table} + result := s.Sweep(context.Background()) + if result.Errors != 0 { + t.Fatalf("Sweep: %d errors", result.Errors) + } + + want := map[vipxlatmap.Key]string{} + for _, r := range tt.wantKept { + want[sweepRowKey(block, argument, r)] = r.name + } + entries, err := table.List() + if err != nil { + t.Fatalf("List: %v", err) + } + for _, e := range entries { + if _, ok := want[e.Key]; !ok { + t.Errorf("row %+v survived the sweep", e.Key) + } + delete(want, e.Key) + } + for _, name := range want { + t.Errorf("row %q was removed, want kept", name) + } + if got, wantRemoved := result.EBPFVIPXlatEntriesRemoved, len(tt.rows)-len(tt.wantKept); got != wantRemoved { + t.Errorf("EBPFVIPXlatEntriesRemoved = %d, want %d", got, wantRemoved) + } + }) + } +} + +// sweepRowKey is the key writeSweepRows wrote r under. +func sweepRowKey(block uint64, argument uint16, r sweepRow) vipxlatmap.Key { + if r.argument != 0 { + argument = r.argument + } + if r.egress { + return vipxlatmap.Key{Block: block, Argument: argument, Proto: ipProtoTCP, + Addr: netip.MustParseAddr(r.backend), Port: 30080} + } + return vipxlatmap.Key{Block: block, Argument: argument, Slot: r.slot, Proto: ipProtoTCP, + Addr: netip.MustParseAddr(r.vip), Port: 8080} +} + +// TestVIPXlatSweeper_ListErrorSkipsPass checks that a failure to list the +// node's bindings removes nothing rather than sweeping against an empty set. +func TestVIPXlatSweeper_ListErrorSkipsPass(t *testing.T) { + router, _, vrf := newBackendFixtures(testVPCRef) + c := fake.NewClientBuilder().WithScheme(newRuleTestScheme(t)).WithObjects(router, vrf). + WithInterceptorFuncs(interceptor.Funcs{ + List: func(ctx context.Context, cl client.WithWatch, list client.ObjectList, opts ...client.ListOption) error { + if _, ok := list.(*bgpv1alpha1.ServiceVIPBindingList); ok { + return errors.New("injected list failure") + } + return cl.List(ctx, list, opts...) + }, + }).Build() + + mem := newMemTable() + writeSweepRows(t, mem, 0x20010db8ff01, 70, []sweepRow{{name: "orphan", slot: 0x1234, + vip: testSweepOrphanVIP, backend: testSweepOrphanBackend}}) + table := vipxlatmap.NewVipXlatTable(mem) + + result := (&VIPXlatSweeper{Client: c, NodeName: testComputeNodeName, Table: table}).Sweep(context.Background()) + if result.Errors != 1 || result.EBPFVIPXlatEntriesRemoved != 0 { + t.Errorf("Sweep = %+v, want 1 error and nothing removed", result) + } + entries, err := table.List() + if err != nil { + t.Fatalf("List: %v", err) + } + if len(entries) != 1 { + t.Errorf("table holds %d rows, want the 1 row untouched", len(entries)) + } +} diff --git a/internal/gc/gc.go b/internal/gc/gc.go index c4840e40..4f9f7875 100644 --- a/internal/gc/gc.go +++ b/internal/gc/gc.go @@ -61,7 +61,11 @@ type CleanupResult struct { // reconciled against the same live set as EBPFVRFEntriesRemoved since // the two tables share the same (Block, Argument) key and lifecycle. EBPFVPCAttributionEntriesRemoved int - Errors int + // EBPFVIPXlatEntriesRemoved counts stale eBPF vip_xlat_table entries + // removed by galactic-router's controller.VIPXlatSweeper, rows no live + // ServiceVIPBinding on this node claims. + EBPFVIPXlatEntriesRemoved int + Errors int } // vrfNameRegex matches the deterministic VRF interface name Galactic