refactor(cni): extract galactic-ipam as a real delegated CNI IPAM plugin

[privateip]

Stack (merge bottom to top):

• refactor(cni): split galactic-cni into veth/tap master plugins #303 refactor(cni): split galactic-cni into veth/tap master plugins
• refactor(cni): extract galactic-ipam as a real delegated CNI IPAM plugin #304 refactor(cni): extract galactic-ipam as a real delegated CNI IPAM plugin (this PR)
• refactor(cni): extract galactic-bgp as its own CNI chain plugin #305 refactor(cni): extract galactic-bgp as its own CNI chain plugin
• refactor(cni): extract galactic-route as its own CNI chain plugin #306 refactor(cni): extract galactic-route as its own CNI chain plugin
• docs+fixes(cni): finish the CNI plugin-chain split — docs, containerlab, dead code #307 docs+fixes(cni): finish the CNI plugin-chain split - docs, containerlab, dead code

---

Summary

Second branch in the CNI plugin-chain split stack (based on #303). galactic-ipam is now a real CNI IPAM delegation target (github.com/containernetworking/plugins/pkg/ipam.ExecAdd/ExecDel), execed by galactic-cni/galactic-tap-cni — not the in-process library call step 0 left it as.

Explicit IPAM contract

• Whether IPAM runs at all is decided solely by "ipam" block presence in a master plugin's own config. No environment variable can trigger or suppress that anymore — the old GALACTIC_CNI_ENABLE_LOCAL_IPAM "enforce ipam block" check is gone.
• ipv6_subnet/ipv4_subnet/address_families move inside the "ipam" block. Their CIDR validation moved with them, into galactic-ipam's own parseConf.
• ipam.type now names the delegated binary, not a pool-vs-static mode selector. Mode is decided internally: static_ip presence selects static, otherwise ipv6_subnet/ipv4_subnet select pool mode.
• enableLocalIPAM → GALACTIC_IPAM_ENABLE_LOCAL_IPAM (internal/config/ipam.go), read only by galactic-ipam itself, and only fills a default IPv6 pool CIDR when "ipam" is present but under-specified — it can't manufacture the block anymore.

The unlock: IPv6 pool persistence

PoolAllocator tracked allocations in memory only — fine for ADD (each pool belongs to exactly one VPCAttachment), useless for DEL (fresh process, nothing to look up). Deallocation used to close that gap by reading the allocated value back out of the BGPAdvertisement CRD annotation galactic-bgp wrote — the one thing keeping galactic-ipam coupled to galactic-bgp's CRD naming and a Kubernetes client it has no other use for.

Fixed: PoolAllocator gets the same on-disk marker file IPv4PoolAllocator already had (keyed by containerID, flock-guarded, one shared lock dir for both families — ipam.DefaultLockDir). Both allocators gained DeallocateContainer(containerID) and a peek-only LookupContainer(containerID). galactic-ipam now looks its own allocations up locally — no k8s client, no crdnames, no bgpv1alpha1 import at all.

Verification

• task lint ✅ task build ✅ (builds + stages galactic-ipam too) task test:unit ✅
• internal/cniipam at 91% coverage, including a real ADD → CHECK → DEL → CHECK round trip proving the self-contained-by-containerID design holds end to end.
• task test:e2e — not run, same caveat as refactor(cni): split galactic-cni into veth/tap master plugins #303.

🤖 Generated with Claude Code

[mattdjenkinson]

Went through this one too. The delegation wiring itself is fine and the build/unit tests are clean, but moving IPAM state from in-memory/CRD-backed to durable on-disk marker files changes some failure-mode assumptions that the rest of the ADD/DEL/CHECK lifecycle wasn't updated to match. A few of these look like they should block merge, roughly in order of how bad I think they are.

Rollback on a failed ADD never releases the IPAM allocation. internal/cni/resource.go:74 (and the same gap in internal/cnitap/resource.go:68) tears down VRF, veth, BGP CRDs, and eBPF state on a failed ADD, but never calls ipam.ExecDel. Under the old in-memory scheme this didn't matter, the allocation just died with the process. Now it's a marker file on disk, so any ADD that gets past IPAM and then fails on a later step permanently burns a subnet or address out of the pool.

The checked-in NAD configs weren't migrated to the new shape. All ten nad.yaml files under deploy/containerlab/resources/tenants/** still set ipv6_subnet/address_families at the top level, but PluginConf.IPAM is now only populated from a nested ipam block. Those fields go unread, pluginConf.IPAM stays nil, and ADD succeeds while handing the workload an interface with no IP address at all. Nothing errors.

There's no migration path from the old CRD-annotation-based allocation record to the new on-disk markers. A pod allocated before this ships has no marker file. After the upgrade, a new pod's ADD scans on-disk state, doesn't see that old subnet as used, and can allocate it to someone else, so you end up with two live pods sharing an address.

Allocate(containerID) in internal/cni/ipam/ipam.go:142 (and the IPv4 equivalent in ipv4.go:107) doesn't check whether the container already holds an allocation before making a new one. The CNI spec explicitly permits a runtime to retry ADD for the same container after a transient failure, and each retry here writes a fresh marker file, leaking the earlier one. Since findContainerMarker returns on the first match, only one of the leaked markers is ever recoverable via DEL.

CHECK never validates IPAM state. internal/cni/ops_check.go:35's cmdCheck never calls ipam.ExecCheck, even though galactic-ipam implements it specifically for this. A lost or corrupted marker file goes completely undetected by CHECK.

GALACTIC_IPAM_ENABLE_LOCAL_IPAM gets re-read independently on every invocation in internal/cniipam/config.go:66. If that env var flips between a pod's ADD and its later DEL, deallocate() sees an empty subnet and skips cleanup silently, leaking the allocation with no error.

There's also a locking bug: DeallocateContainer in internal/cni/ipam/ipam.go:265 removes the marker file after findContainerMarker has already released the flock, so a cross-process caller can interleave between the scan and the removal. Allocate/Deallocate hold the lock across the whole read-modify-write; this path doesn't. ipv4.go has the same structure.

And a performance concern on top of that: the site-wide IPv4 pool can hold on the order of 4000 entries, and findContainerMarker in ipv4.go:200 reads every marker file's contents while holding the pool-wide flock. A single pod's DEL or CHECK can now block every other pod's ADD/DEL/CHECK against that pool for the duration, which is new lock-hold-time cost this PR introduces.

Two smaller things: DeallocateContainer/LookupContainer/findContainerMarker are duplicated almost verbatim between the IPv6 and IPv4 allocators, which live in the same package, so a shared helper would save having to patch both for any future fix to the scan/lock logic. And the new test names in internal/cniipam/config_test.go (like "missing ipam block rejected") use lowercase phrases rather than the UpperCamelCase the repo's conventions call for.

The allocation leak on failed ADD, the un-migrated NAD configs, and the missing upgrade path for existing allocations feel like the ones worth resolving before this merges, since they're silent correctness and availability problems rather than edge cases.