feat: Add ingress sidecar for VPC backend connectivity by privateip · Pull Request #424 · datum-cloud/galactic

privateip · 2026-08-18T20:52:54Z

Summary

HTTP Ingress for VPC Networks (#796) needs a way to attach the shared Envoy Gateway fleet's node-local Linux VRF/SRv6 route state to each VPC backend pod, so Envoy's own upstream sockets can reach a tenant's pods over the SRv6 underlay. This adds that piece: galactic-vrf, a new sidecar binary that runs as a second container in the Envoy Gateway pod and watches the per-pod EndpointSlices galactic-cni publishes (#854).

Two kernel resources are reconciled at two different granularities, matching how the underlying VRF/SID primitives actually work rather than the tenant label they're discovered through: one Linux VRF device per VPC (shared across every attachment of that VPC on the node), and one SRv6 egress route per pod (since pods of the same tenant on different nodes carry different SIDs). Teardown of either is delayed by a configurable grace period after the corresponding EndpointSlice disappears, and a VPC's own grace period never starts while any of its routes are still within their own — so a slow config-push on the extension-server side (#856/#857) can't get raced into a blackhole, and a live sibling attachment is never torn down out from under.

Startup inventories existing kernel state before the first reconcile so a restart doesn't either treat live pods as stale or leak truly orphaned VRFs forever.

Full design in docs/plans/855-ingress-sidecar-vpc-backend-connectivity.md.

Test plan

Unit tests for the reconciler's desired-state diffing at both granularities (VRF appears/disappears with first/last pod; a route's own lifecycle is independent of its siblings; a second attachment of the same VPC doesn't disturb the first)
Full-repo build/vet/test clean
Required before merge, not yet done: real-kernel verification of RouteEgressAdd's netlink.RouteGet assumption and the VRF lock's flock-path writability, run from an actual Envoy Gateway pod's netns
Required before merge, not yet done: end-to-end verification that the existing TC-BPF uSID decap datapath handles this sidecar's traffic the same as EVPN-originated traffic

Opening as draft until both required kernel/e2e checks above are done — see §7 of the plan.

Related to datum-cloud/enhancements#855
Related to datum-cloud/enhancements#796

🤖 Generated with Claude Code

Bare "apt-get update"/"apt-get install" have no per-request timeout, so a stalled connection to GitHub's default regional mirror (azure.archive.ubuntu.com has been observed hanging outright) blocks until the job's own timeout-minutes kills it, burning the whole 15-25 minute budget instead of failing fast or retrying. This hit both #424 (install-ebpf-deps, inside the E2E Tests job) and #425 (the vrf kernel module step, inside Unit Tests (root)) in the same CI run window, on two otherwise-unrelated branches -- both jobs got cancelled at their timeout with apt still stuck on the initial 'update'. Add -o Acquire::Retries=3 -o Acquire::http::Timeout=10 -o Acquire::https::Timeout=10 to every apt-get invocation that installs CI-only packages (install-ebpf-deps' clang/llvm/ linux-libc-dev, and the two copies -- ci.yaml and scripts/ci.sh -- of the linux-modules-extra vrf-module install) so a bad mirror fails each attempt in ~10s and retries up to 3 times instead of hanging for the full job timeout. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

0xmc

There may be a startup race. Inventory relies on WaitForCacheSync, which only guarantees the informer's initial list, not that the reconcile queue has drained.

On a busy node at boot this can seed a duplicate boot// route entry, and once its grace period expires, Sweep can delete a route a live pod still needs.

Not covered by existing tests (which only test the intended ordering).

Either drain the initial reconcile batch or base Inventory's dedup on live API state.

0xmc's review on PR #424 flagged a real race in the startup sequence: Store.Inventory was gated on mgr.GetCache().WaitForCacheSync(ctx) alone, on the assumption that a synced informer cache implies every pre-existing EndpointSlice has already gone through the controller's own Reconcile (and therefore Store.SetDesired). That assumption is false — WaitForCacheSync only guarantees the informer's initial List landed in the cache, not that the workqueue it fed has been drained by Reconcile. On a busy node at boot the two can race: Store.Inventory could see a still-live pod's kernel route as orphaned (routeKnownLocked found no tracked state for it yet) and seed it under a synthetic "boot/<vpc>/<prefix>" key with its own independent grace period. The real Reconcile, once it caught up, would create/update the real "namespace/name" key and never touch the stale boot entry. Once that entry's grace period elapsed, Sweep would call backend.RemoveRoute — deleting the actual kernel route (routes are addressed by prefix+table, not by Store's map key) out from under the still-live pod, with nothing left to notice or recover. Fix: internal/ingresssidecar.SeedFromAPI lists every tenant-labeled EndpointSlice directly via the manager's uncached reader (mgr.GetAPIReader(), safe to use before mgr.Start) and applies each one's desired route through the same Store.SetDesired path Reconciler itself uses — before Store.Inventory ever runs. This closes the race independently of cache/workqueue timing rather than trying to wait it out; SetDesired is idempotent, so the controller's own later, redundant Reconcile of the same objects is harmless. cmd/galactic-vrf/root.go's startup goroutine now calls SeedFromAPI before Inventory and drops the WaitForCacheSync gate, which was never sufficient on its own. Updated Store.Inventory/RunSweeper's doc comments and docs/plans/855's §9 item 2 decision to describe the corrected mechanism instead of the broken assumption. Added internal/ingresssidecar/seed_test.go, including TestSeedFromAPIThenInventoryDoesNotOrphanLiveRoute, which runs the fixed startup order directly (SeedFromAPI claiming a route from live API state with no completed Reconcile standing in for it) and asserts a long sweep afterward never removes it — the exact gap 0xmc called out as uncovered by existing tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

privateip · 2026-08-21T01:21:54Z

Thanks for catching this @0xmc — confirmed the race and pushed a fix in b383d58.

Root cause: Store.Inventory was gated on mgr.GetCache().WaitForCacheSync(ctx) alone, which only guarantees the informer's initial List landed in the cache — not that the resulting workqueue had been drained by Reconcile. On a busy node at boot, Inventory could see a still-live pod's kernel route as orphaned before its Reconcile caught up, seed it under a synthetic boot/<vpc>/<prefix> key with its own grace period, and once that period elapsed, Sweep would delete the underlying kernel route (routes are addressed by prefix+table, not by Store's map key) out from under the still-live pod.

Fix: added ingresssidecar.SeedFromAPI, which lists every tenant-labeled EndpointSlice directly via the manager's uncached reader (mgr.GetAPIReader(), safe to use before mgr.Start) and applies each one through the same Store.SetDesired path Reconciler itself uses — before Inventory ever runs. This closes the race independently of cache/workqueue timing (your "base Inventory's dedup on live API state" option) rather than trying to drain/wait out the workqueue. SetDesired is idempotent, so the controller's own later, now-redundant Reconcile of the same objects is harmless.

Also added internal/ingresssidecar/seed_test.go, including TestSeedFromAPIThenInventoryDoesNotOrphanLiveRoute, which runs the fixed startup order directly and asserts a long sweep afterward never removes a route claimed only through SeedFromAPI (no completed Reconcile standing in for it) — the gap you noted wasn't covered by the existing tests. Updated docs/plans/855-...md §9 item 2 to describe the corrected mechanism.

🤖 Generated with Claude Code

0xmc's review on PR #424 flagged a real race in the startup sequence: Store.Inventory was gated on mgr.GetCache().WaitForCacheSync(ctx) alone, on the assumption that a synced informer cache implies every pre-existing EndpointSlice has already gone through the controller's own Reconcile (and therefore Store.SetDesired). That assumption is false — WaitForCacheSync only guarantees the informer's initial List landed in the cache, not that the workqueue it fed has been drained by Reconcile. On a busy node at boot the two can race: Store.Inventory could see a still-live pod's kernel route as orphaned (routeKnownLocked found no tracked state for it yet) and seed it under a synthetic "boot/<vpc>/<prefix>" key with its own independent grace period. The real Reconcile, once it caught up, would create/update the real "namespace/name" key and never touch the stale boot entry. Once that entry's grace period elapsed, Sweep would call backend.RemoveRoute — deleting the actual kernel route (routes are addressed by prefix+table, not by Store's map key) out from under the still-live pod, with nothing left to notice or recover. Fix: internal/ingresssidecar.SeedFromAPI lists every tenant-labeled EndpointSlice directly via the manager's uncached reader (mgr.GetAPIReader(), safe to use before mgr.Start) and applies each one's desired route through the same Store.SetDesired path Reconciler itself uses — before Store.Inventory ever runs. This closes the race independently of cache/workqueue timing rather than trying to wait it out; SetDesired is idempotent, so the controller's own later, redundant Reconcile of the same objects is harmless. cmd/galactic-vrf/root.go's startup goroutine now calls SeedFromAPI before Inventory and drops the WaitForCacheSync gate, which was never sufficient on its own. Updated Store.Inventory/RunSweeper's doc comments and docs/plans/855's §9 item 2 decision to describe the corrected mechanism instead of the broken assumption. Added internal/ingresssidecar/seed_test.go, including TestSeedFromAPIThenInventoryDoesNotOrphanLiveRoute, which runs the fixed startup order directly (SeedFromAPI claiming a route from live API state with no completed Reconcile standing in for it) and asserts a long sweep afterward never removes it — the exact gap 0xmc called out as uncovered by existing tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Implements docs/plans/855-ingress-sidecar-vpc-backend-connectivity.md (as revised by PR #377) against #854's already-resolved EndpointSlice schema: - internal/ingresssidecar (new): the core mechanism. - desired.go: BuildDesiredRoute translates one EndpointSlice into a DesiredRoute, gated on the tenant label (discovery) and the SID annotation (not always present yet). - backend.go: Backend interface + kernelBackend, wired directly to internal/plumbing/vrf and internal/plumbing/srv6 -- the same primitives galactic-cni's own pod-attachment path uses. Also lists existing kernel VRFs/routes for startup inventory. - store.go: Store, the two-granularity (VRF per-VPC, route per-pod) grace-period-aware reconciler at this package's core. SetDesired applies "up" transitions immediately; Sweep tears down "down" transitions only once their independent grace periods elapse, with a VPC's own grace period never starting while any of its routes are still within their own -- per §9 item 1 of the plan. Inventory seeds truly-orphaned kernel state at boot with a fresh grace period rather than ignoring or immediately deleting it -- §9 item 2. - controller.go: thin controller-runtime Reconciler translating EndpointSlice watch events into Store.SetDesired calls, plus RunSweeper for the periodic teardown pass. - metrics.go: Prometheus surface per §6 (active/pending VRF and route gauges, reconcile errors/latency). - internal/crdnames: adds ParseTenantIdentifier, splitting TenantIdentifier(vpc, vpcAttachment) back into its halves. Needed because the plan's §2 assumed `vpc` arrives on the EndpointSlice verbatim, but #854's actual contract only publishes the combined tenant identifier -- see that function's doc comment and the plan's own now-corrected §2 text. - internal/config: adds VRFConfig (GALACTIC_VRF_* env vars / flags), following RouterConfig/GatewayConfig's three-tier precedence convention. No NodeName field -- this sidecar has no CRD identity keyed by node, unlike its siblings. - cmd/galactic-vrf (new binary): manager wiring, RBAC pre-flight check for the EndpointSlice watch (§9 item 8), startup inventory gated on cache sync, and the periodic sweep goroutine. Deliberately has no gRPC health server -- §5 of the plan notes neither existing binary has an established convention to copy. - containers/galactic-vrf/Dockerfile: minimal distroless build, per §6. Not done here, both flagged in the plan's own §7 as required pre-merge and blocking before this sidecar is trusted with real traffic, neither possible from a sandboxed dev environment: - Real-kernel verification of RouteEgressAdd's netlink.RouteGet assumption and the vrf.Add/Delete flock path's writability, run from an actual Envoy Gateway pod's netns. - End-to-end eBPF decap verification against the existing TC-BPF uSID datapath. Also out of scope, per the plan's own dependency order (§8): #856's deployment/injection manifests (config/, sidecar patch). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…rams golangci-lint's goconst flags exact string literals repeated three or more times within a package when used as composite-literal field values or in comparisons (positional call arguments are exempt): - internal/config: gateway.go/router.go/vrf.go each bind their own metrics-port flag to a metrics_port Viper key and share an identical out-of-range Validate() message -- extract flagMetricsPort, keyMetricsPort, and errMetricsPortRange into config.go, and the matching test name/wantErr strings into config_test.go. - internal/crdnames/crdnames_test.go: the formatted (vpc, attachment) identifiers abc-def and 0000000jU-00G recur across BGPAdvertisementName/TenantIdentifier/ParseTenantIdentifier test tables -- join them from the existing raw-part constants instead of repeating the formatted string, and add testAttachmentBase62 for the 00G part that was also repeated. - internal/ingresssidecar: vpc1 and pod-a recur across controller_test.go/desired_test.go/store_test.go -- add testVPC1/ testPodName constants shared via store_test.go. Fixing pod-a's repetition also let unparam catch readySlice's namespace parameter (every caller passed "ns") and, once that value became the constant testPodName, its name parameter too (every caller passed testPodName) -- both are now hardcoded in the function body instead of threaded through as parameters. No behavior change; verified with go build/vet/test and a full golangci-lint run across the repo. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

…erface testChainedGalacticBGP invokes galactic-bgp's cmdAdd with an ipv6_subnet carried through from the tap step's prevResult, so ipamResult.IPv6Subnet is non-nil and ADD takes the EndpointSlice-publish branch added in internal/cnibgp/ops_add.go. That branch requires nadpatch.ParsePodName(args.Args) to resolve a real K8S_POD_NAME, but the test never set CNI_ARGS on this step, so args.Args was empty and ADD failed with "publish EndpointSlice: no K8S_POD_NAME in CNI_ARGS \"\"". Set CNI_ARGS="IgnoreUnknown=1;K8S_POD_NAMESPACE=default;K8S_POD_NAME=<pod>" on the galactic-bgp step only, mirroring what Multus actually sets on a real invocation. The tap step above deliberately keeps CNI_ARGS unset (see its own comment) so nadpatch.VerifyChainComplete/AnnotateNAD skip NAD lookups the test doesn't set up a NAD for; galactic-bgp's own cmdAdd never calls either of those, so setting CNI_ARGS only here is safe. Verified locally: task test:e2e passes (TestCNITapInterface, including the chained galactic-bgp ADD/CHECK), plus task lint and task test:unit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Rebasing this branch onto main brought gateway_test.go and nat66_test.go's own pre-existing "valid config" test-case name together with this branch's new vrf_test.go, pushing golangci-lint's goconst threshold from 2 (safe) to 3 (flagged) occurrences across the config package. Add testCaseValidConfig alongside this file's other shared test literals (config_test.go's existing dedupe pattern for exactly this situation) and use it in all three. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

0xmc's review on PR #424 flagged a real race in the startup sequence: Store.Inventory was gated on mgr.GetCache().WaitForCacheSync(ctx) alone, on the assumption that a synced informer cache implies every pre-existing EndpointSlice has already gone through the controller's own Reconcile (and therefore Store.SetDesired). That assumption is false — WaitForCacheSync only guarantees the informer's initial List landed in the cache, not that the workqueue it fed has been drained by Reconcile. On a busy node at boot the two can race: Store.Inventory could see a still-live pod's kernel route as orphaned (routeKnownLocked found no tracked state for it yet) and seed it under a synthetic "boot/<vpc>/<prefix>" key with its own independent grace period. The real Reconcile, once it caught up, would create/update the real "namespace/name" key and never touch the stale boot entry. Once that entry's grace period elapsed, Sweep would call backend.RemoveRoute — deleting the actual kernel route (routes are addressed by prefix+table, not by Store's map key) out from under the still-live pod, with nothing left to notice or recover. Fix: internal/ingresssidecar.SeedFromAPI lists every tenant-labeled EndpointSlice directly via the manager's uncached reader (mgr.GetAPIReader(), safe to use before mgr.Start) and applies each one's desired route through the same Store.SetDesired path Reconciler itself uses — before Store.Inventory ever runs. This closes the race independently of cache/workqueue timing rather than trying to wait it out; SetDesired is idempotent, so the controller's own later, redundant Reconcile of the same objects is harmless. cmd/galactic-vrf/root.go's startup goroutine now calls SeedFromAPI before Inventory and drops the WaitForCacheSync gate, which was never sufficient on its own. Updated Store.Inventory/RunSweeper's doc comments and docs/plans/855's §9 item 2 decision to describe the corrected mechanism instead of the broken assumption. Added internal/ingresssidecar/seed_test.go, including TestSeedFromAPIThenInventoryDoesNotOrphanLiveRoute, which runs the fixed startup order directly (SeedFromAPI claiming a route from live API state with no completed Reconcile standing in for it) and asserts a long sweep afterward never removes it — the exact gap 0xmc called out as uncovered by existing tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

Updates added to the code per comments

privateip mentioned this pull request Aug 19, 2026

fix(ci): harden apt-get against mirror hangs in eBPF/vrf-module steps #435

Closed

3 tasks

privateip marked this pull request as ready for review August 20, 2026 00:18

privateip requested a review from a team as a code owner August 20, 2026 00:18

privateip requested review from JoseSzycho and scotwells August 20, 2026 00:18

privateip force-pushed the feat/855-ingress-sidecar branch from 8c12e99 to a3a114a Compare August 20, 2026 18:16

privateip requested a review from 0xmc August 20, 2026 18:17

0xmc previously requested changes Aug 20, 2026

View reviewed changes

privateip requested a review from 0xmc August 21, 2026 01:22

privateip enabled auto-merge August 21, 2026 01:26

privateip force-pushed the feat/855-ingress-sidecar branch from b383d58 to 342a6fa Compare August 21, 2026 01:33

scotwells previously approved these changes Aug 21, 2026

View reviewed changes

privateip and others added 5 commits August 20, 2026 22:22

privateip dismissed scotwells’s stale review via 1eac52d August 21, 2026 02:25

privateip force-pushed the feat/855-ingress-sidecar branch from 342a6fa to 1eac52d Compare August 21, 2026 02:25

privateip requested a review from scotwells August 21, 2026 02:28

scotwells approved these changes Aug 21, 2026

View reviewed changes

privateip disabled auto-merge August 21, 2026 02:36

privateip enabled auto-merge August 21, 2026 02:36

privateip merged commit c0a947f into main Aug 21, 2026
11 checks passed

privateip deleted the feat/855-ingress-sidecar branch August 21, 2026 02:37

Navigation Menu

Uh oh!

feat: Add ingress sidecar for VPC backend connectivity - #424

feat: Add ingress sidecar for VPC backend connectivity#424
privateip merged 5 commits into
mainfrom
feat/855-ingress-sidecar

privateip commented Aug 18, 2026

Uh oh!

0xmc left a comment

Uh oh!

privateip commented Aug 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

privateip commented Aug 18, 2026

Summary

Test plan

Uh oh!

0xmc left a comment

Choose a reason for hiding this comment

Uh oh!

privateip commented Aug 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants