fix: Remove a deleted ingress sidecar pod's map rows and advertisements

docs/agents/ARCHITECTURE-CNI.md

@@ -96,6 +96,29 @@ delivery reads the per-interface `ifindex_egress_kind_table` row, which is
 exact. Once no egress shard resolves, a pass skips the remaining VRFs' shard
 routes rather than wait out each one's neighbor solicitation.
 
+The ingress sidecar (`galactic-vrf`) writes rows into the same maps from its
+pod's network namespace, each in a key range no host writer reaches: `vrf_table`
+under `uformat.BlockIngressSidecar`, `ifindex_vrf_table` at or above
+`ifindexvrfmap.SidecarIfindexBase`, and `egress_route_table` for tables at or
+above `vrf.SidecarTableIDBase`. It leaves them in place when it exits, so a
+restarted container keeps serving, and the host's GC and repair passes never
+touch them. Two things remove a deleted pod's rows
+(`internal/plumbing/ebpf/sidecarmap`, #773). A new sidecar removes, at startup,
+every sidecar row its own VRFs do not account for. The installer's sidecar
+return sweep (`internal/installer/sidecarreap.go`) treats a sidecar gateway
+advertisement whose recorded host-side interface no longer exists, or is no
+longer a veth, as the advertisement of a deleted pod, and deletes it. When every
+sidecar advertisement on the node is in that state on two sweeps in a row, it
+first removes every sidecar row from the maps. The second sweep covers a
+replacement pod that has written its rows but not yet republished its
+advertisements. A node whose sidecar publishes no advertisements gives the
+installer no evidence, so it keeps that node's rows. Either path can still take
+a live sidecar's rows for a deleted pod's: the startup prune when two sidecar
+pods overlap on a node, and the installer when a replacement pod fails to
+publish for two sweeps. So every sidecar reads its own `vrf_table` and
+`ifindex_vrf_table` rows back on each sweep and reapplies its VRFs and routes
+when any are missing, the same reapply a datapath reload triggers.
+
 `galactic-router`'s own reconciler independently derives the *same* SID value
 from the same inputs (`srv6.ComputeSID`, `internal/plumbing/srv6/usid.go`,
 called from `internal/reconcile/reconcile.go`) for the BGP control-plane

internal/gc/gc.go

@@ -586,7 +586,10 @@ func SweepEBPFVRFTable(ctx context.Context, k8s client.Client, namespace, nodeNa
 	// on the next tick, leaving vrf_table empty for those entries while
 	// ifindex_vrf_table, which this sweep never touches, still holds them, and
 	// silently blackholing the sidecar's outbound connections. Preserve every
-	// entry in that block unconditionally.
+	// entry in that block here. Removing a deleted sidecar pod's rows is the
+	// sidecar's own job at startup and the installer's task once every sidecar
+	// advertisement on the node has looked gone for two sweeps, since only they
+	// can tell a live sidecar from a gone one.
 	existing, err := reg.VRF.List()
 	if err != nil {
 		slog.Error("GC: failed to list eBPF vrf_table for sweep", "err", err)

internal/ingresssidecar/backend.go

@@ -45,6 +45,16 @@ type Backend interface {
 	// it. Store compares successive values to know when every VRF and route
 	// must be reapplied.
 	DatapathGeneration() (string, error)
+	// PruneDatapath removes every row in the node's shared eBPF maps that an
+	// ingress sidecar wrote for a VRF whose routing table is not in
+	// keepTableIDs, and reports how many it removed. Those rows belong to an
+	// earlier sidecar pod on this node, which never removes its own on exit.
+	PruneDatapath(keepTableIDs map[uint32]struct{}) (removed int, err error)
+	// DatapathRowsPresent reports whether the shared eBPF maps still hold the
+	// rows EnsureVRF registered for every VRF in tableIDs. Another writer can
+	// remove them without reloading the datapath, which DatapathGeneration
+	// does not see.
+	DatapathRowsPresent(tableIDs []uint32) (bool, error)
 }
 
 // VRFInfo describes one kernel VRF device discovered by Backend.ListVRFs.
@@ -131,6 +141,14 @@ func (kernelBackend) DatapathGeneration() (string, error) {
 	return datapathGeneration()
 }
 
+func (kernelBackend) DatapathRowsPresent(tableIDs []uint32) (bool, error) {
+	return datapathRowsPresent(tableIDs)
+}
+
+func (kernelBackend) PruneDatapath(keepTableIDs map[uint32]struct{}) (int, error) {
+	return pruneDatapath(keepTableIDs)
+}
+
 // vrfNameRegex matches the interface name generated for a VPC: a leading
 // letter, nine zero-padded base62 characters, and a trailing letter.
 //

internal/ingresssidecar/controller.go

@@ -81,7 +81,11 @@ var (
 )
 
 // RunStartup blocks, seeding store from reader, taking its inventory of host
-// state, then sweeping it every interval until ctx is done.
+// state, removing the eBPF map rows an earlier sidecar pod left behind, then
+// sweeping it every interval until ctx is done.
+//
+// The prune runs only after a successful inventory. A failed one can leave a
+// VRF in this pod untracked, and pruning then would remove its live rows.
 //
 // Only a failed EndpointSlice list holds the sweep back, retried with backoff:
 // running Inventory without the seed would reopen the race SeedFromAPI closes.
@@ -104,6 +108,11 @@ func RunStartup(ctx context.Context, reader client.Reader, store *Store, interva
 	}
 	if err := store.Inventory(ctx, time.Now()); err != nil {
 		slog.Error("ingresssidecar: startup inventory", "error", err)
+	} else if err := store.PruneForeignDatapath(); err != nil {
+		// Not retried: the rows it leaves only hold map slots until the next
+		// sidecar pod's startup prune, or until the host's installer judges
+		// every sidecar on this node gone.
+		slog.Error("ingresssidecar: startup prune", "error", err)
 	}
 	RunSweeper(ctx, store, interval)
 }

internal/ingresssidecar/controller_test.go

@@ -154,6 +154,38 @@ func TestRunStartupSweepsAfterFailedSeed(t *testing.T) {
 	})
 }
 
+// TestRunStartupPrunesAfterInventory verifies the startup prune runs once,
+// after Inventory, so the VRFs the pod already holds are in its keep set.
+func TestRunStartupPrunesAfterInventory(t *testing.T) {
+	c := fake.NewClientBuilder().WithScheme(newTestScheme(t)).Build()
+
+	backend := newFakeBackend()
+	backend.seedRoute("existing", 9, mustPrefix(t, "fd00::9"), net.ParseIP("fd00:99::9"))
+	store := NewStore(backend, time.Hour, nil)
+
+	ctx, cancel := context.WithCancel(context.Background())
+	done := make(chan struct{})
+	go func() {
+		defer close(done)
+		RunStartup(ctx, c, store, time.Hour)
+	}()
+	t.Cleanup(func() {
+		cancel()
+		<-done
+	})
+
+	waitFor(t, "startup prune", func() bool {
+		backend.mu.Lock()
+		defer backend.mu.Unlock()
+		return len(backend.pruned) == 1
+	})
+	backend.mu.Lock()
+	defer backend.mu.Unlock()
+	if _, ok := backend.pruned[0][9]; !ok {
+		t.Errorf("keep = %v, want it to hold the inventoried VRF's table 9", backend.pruned[0])
+	}
+}
+
 func waitFor(t *testing.T, what string, cond func() bool) {
 	t.Helper()
 	deadline := time.Now().Add(5 * time.Second)

internal/ingresssidecar/ebpfdatapath.go

@@ -20,6 +20,7 @@ import (
 	"go.datum.net/galactic/internal/plumbing/ebpf/egressroutemap"
 	"go.datum.net/galactic/internal/plumbing/ebpf/ifindexvrfmap"
 	"go.datum.net/galactic/internal/plumbing/ebpf/prog"
+	"go.datum.net/galactic/internal/plumbing/ebpf/sidecarmap"
 	"go.datum.net/galactic/internal/plumbing/ebpf/uformat"
 	"go.datum.net/galactic/internal/plumbing/ebpf/usidmap"
 	"go.datum.net/galactic/internal/plumbing/vrf"
@@ -441,6 +442,69 @@ func removeEgressDatapath(tableID uint32) error {
 	return errors.Join(errs...)
 }
 
+// pruneDatapath removes every row in the shared maps that an ingress sidecar
+// registered for a VRF whose routing table is not in keepTableIDs.
+//
+// This sidecar leaves its rows in place when it exits, so that a restarted
+// container in the same pod finds them still serving. A deleted pod takes its
+// VRFs and veths with it but leaves the rows, and its replacement only
+// overwrites the rows of the VPCs it serves again. This sidecar is the only
+// writer of the sidecar's share of these maps on its node, so whatever its own
+// VRFs do not account for is a predecessor's.
+func pruneDatapath(keepTableIDs map[uint32]struct{}) (int, error) {
+	keep := make(map[uint16]struct{}, len(keepTableIDs))
+	for tableID := range keepTableIDs {
+		argument, err := argumentForTableID(tableID)
+		if err != nil {
+			continue // not a table this sidecar registers rows for
+		}
+		keep[argument] = struct{}{}
+	}
+
+	maps, closer, err := sidecarmap.OpenPinned(ebpfPinDir)
+	if err != nil {
+		return 0, err
+	}
+	defer func() { _ = closer.Close() }()
+
+	result, err := sidecarmap.Prune(maps, keep)
+	return result.Total(), err
+}
+
+// datapathRowsPresent reports whether vrf_table and ifindex_vrf_table still hold
+// the rows ensureEgressDatapath registered for each VRF in tableIDs.
+//
+// Those two are written together, before any egress_route_table entry, and
+// removed together by every path that removes a sidecar's rows: this
+// sidecar's own startup prune in a later pod, and the host installer's reaper
+// once it judges every sidecar on the node gone. Either can be wrong about a
+// live sidecar, in a pod overlapping its predecessor or one slow to publish
+// its advertisements, and reading the rows back is what lets this one notice.
+func datapathRowsPresent(tableIDs []uint32) (bool, error) {
+	if len(tableIDs) == 0 {
+		return true, nil
+	}
+	maps, closer, err := sidecarmap.OpenPinned(ebpfPinDir)
+	if err != nil {
+		return false, err
+	}
+	defer func() { _ = closer.Close() }()
+
+	for _, tableID := range tableIDs {
+		argument, err := argumentForTableID(tableID)
+		if err != nil {
+			continue // not a table this sidecar registers rows for
+		}
+		if _, ok, err := maps.VRF.Get(ingressSidecarBlock, argument); err != nil || !ok {
+			return false, err
+		}
+		if _, ok, err := maps.Ifindex.Get(ifindexvrfmap.SidecarIfindex(argument)); err != nil || !ok {
+			return false, err
+		}
+	}
+	return true, nil
+}
+
 // ensureRedirectRoute installs a plain host route for prefix into this pod's
 // main routing table, out the VRF interface for tableID as a bare nexthop
 // device. A VRF master device needs no gateway address: naming it as the link

internal/ingresssidecar/fakebackend_test.go

@@ -37,6 +37,19 @@ type fakeBackend struct {
 	failEnsureRoute error
 	failRemoveVRF   error
 	failRemoveRoute error
+
+	// pruned records the keep set of every PruneDatapath call, and
+	// failPrune, if set, is the error it returns.
+	pruned    []map[uint32]struct{}
+	failPrune error
+
+	// rowsMissing, if set, is the set of VRF tables whose rows
+	// DatapathRowsPresent reports absent, standing in for another writer
+	// removing them. EnsureVRF on that table restores them. failRowsRead,
+	// if set, is the error DatapathRowsPresent returns. Reads are not
+	// logged in calls, since Store makes them on every Sweep.
+	rowsMissing  map[uint32]bool
+	failRowsRead error
 }
 
 type routeRecord struct {
@@ -60,6 +73,7 @@ func (f *fakeBackend) EnsureVRF(vpc string) (uint32, error) {
 		return 0, f.failEnsureVRF
 	}
 	if id, ok := f.vrfs[vpc]; ok {
+		delete(f.rowsMissing, id)
 		return id, nil
 	}
 	id := f.nextTableID
@@ -132,6 +146,28 @@ func (f *fakeBackend) DatapathGeneration() (string, error) {
 	return f.generation, f.failGeneration
 }
 
+func (f *fakeBackend) PruneDatapath(keepTableIDs map[uint32]struct{}) (int, error) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	f.calls = append(f.calls, "PruneDatapath")
+	f.pruned = append(f.pruned, keepTableIDs)
+	return 0, f.failPrune
+}
+
+func (f *fakeBackend) DatapathRowsPresent(tableIDs []uint32) (bool, error) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	if f.failRowsRead != nil {
+		return false, f.failRowsRead
+	}
+	for _, id := range tableIDs {
+		if f.rowsMissing[id] {
+			return false, nil
+		}
+	}
+	return true, nil
+}
+
 // reloadDatapath simulates the CNI control daemon recreating the shared eBPF
 // maps empty: every route entry is lost and the generation moves to
 // testGenReloaded.

internal/ingresssidecar/store.go

@@ -395,10 +395,45 @@ func (s *Store) Inventory(ctx context.Context, now time.Time) error {
 	return nil
 }
 
+// PruneForeignDatapath removes the rows an earlier sidecar pod on this node left
+// in the shared eBPF maps, keeping those of every VRF this one tracks.
+//
+// Call it once, after Inventory: by then every VRF in this pod's namespace is
+// tracked, so a row it does not account for belongs to a pod that is gone. It
+// holds the store's lock throughout, so a VRF SetDesired creates concurrently
+// is either already tracked or registers its rows after the prune.
+func (s *Store) PruneForeignDatapath() error {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	keep := make(map[uint32]struct{}, len(s.vrfs))
+	for _, v := range s.vrfs {
+		if v.tableID != 0 {
+			keep[v.tableID] = struct{}{}
+		}
+	}
+	removed, err := s.backend.PruneDatapath(keep)
+	if removed > 0 {
+		slog.Info("ingresssidecar: removed rows an earlier sidecar pod left in the shared eBPF maps",
+			"removed", removed)
+	}
+	if err != nil {
+		s.countError("prune_datapath")
+		return fmt.Errorf("prune an earlier sidecar pod's eBPF map rows: %w", err)
+	}
+	return nil
+}
+
 // checkDatapathLocked reapplies every live VRF and route when the backend's
-// DatapathGeneration has changed since they were written, or when an earlier
+// DatapathGeneration has changed since they were written, when another writer
+// has removed this sidecar's rows from the shared maps, or when an earlier
 // reapply pass left something unapplied. Callers must hold s.mu.
 //
+// Rows go missing without a reload when a sidecar pod starting on the same
+// node, or the host installer's reaper, takes this live sidecar's rows for a
+// deleted pod's (see PruneForeignDatapath and internal/installer/sidecarreap.go).
+// Reading them back each sweep bounds that mistake to one sweep interval.
+//
 // Nothing else would notice. The CNI control daemon reloads the shared eBPF
 // datapath independently of this sidecar, recreating its maps empty on a
 // schema change and re-pinning usid_egress on every load, while this sidecar
@@ -417,7 +452,11 @@ func (s *Store) checkDatapathLocked() {
 		return
 	}
 	if gen == s.generation && !s.reapplyPending {
-		return
+		if s.rowsPresentLocked() {
+			return
+		}
+		slog.Warn("ingresssidecar: this sidecar's rows are missing from the shared eBPF maps, reapplying")
+		s.reapplyPending = true
 	}
 	if s.generation == "" && !s.anyInstalledLocked() {
 		s.generation = gen // nothing written yet, so nothing to reapply
@@ -433,6 +472,25 @@ func (s *Store) checkDatapathLocked() {
 	s.reapplyPending = !s.reapplyLocked()
 }
 
+// rowsPresentLocked reports whether the shared eBPF maps still hold the rows of
+// every VRF reapplyLocked would reapply. A failed read counts as present, for
+// the same reason a failed generation read is not acted on. Callers must hold
+// s.mu.
+func (s *Store) rowsPresentLocked() bool {
+	var tableIDs []uint32
+	for _, v := range s.vrfs {
+		if v.installed && v.absentSince.IsZero() && v.tableID != 0 {
+			tableIDs = append(tableIDs, v.tableID)
+		}
+	}
+	present, err := s.backend.DatapathRowsPresent(tableIDs)
+	if err != nil {
+		slog.Debug("ingresssidecar: read back eBPF map rows", "err", err)
+		return true
+	}
+	return present
+}
+
 // reapplyLocked re-runs EnsureVRF for every installed VRF with a live route,
 // then EnsureRoute for every installed route still desired, reporting whether
 // all of them succeeded. State within its teardown grace period is left alone: