refactor(cni): extract galactic-bgp as its own CNI chain plugin - #305
Open
privateip wants to merge 1 commit into
Open
refactor(cni): extract galactic-bgp as its own CNI chain plugin#305privateip wants to merge 1 commit into
privateip wants to merge 1 commit into
Conversation
privateip
force-pushed
the
refactor/cni-chain-2-galactic-bgp
branch
from
August 8, 2026 12:02
40ab587 to
d1a6f68
Compare
privateip
force-pushed
the
refactor/cni-chain-2-galactic-bgp
branch
from
August 8, 2026 12:15
d1a6f68 to
f67ee3f
Compare
8 tasks
privateip
force-pushed
the
refactor/cni-chain-2-galactic-bgp
branch
from
August 8, 2026 12:26
f67ee3f to
fd81bfd
Compare
Step 2 of the CNI plugin-chain split (galactic/plan-cni-plugin-chain):
pulls BGP/SRv6/eBPF publishing out of the veth and tap master plugins
into its own chained CNI binary, galactic-bgp, invoked last in the
conflist after galactic-cni/galactic-tap-cni and galactic-ipam.
internal/cnibgp is the new plugin package:
- cnibgp.go: RunPlugin() entrypoint (skel.PluginMainFuncs, ADD/DEL/
CHECK/STATUS/VERSION), mirroring the shape of cniipam/cnitap.
- types.go, config.go: PluginConf{VPC, VPCAttachment, Namespace},
parsed from stdin. Deliberately reuses internal/config.CNIConfig
(GALACTIC_CNI_* env vars) rather than inventing its own prefix like
galactic-ipam did — these are shared node-level settings (API server
address, namespace, etc.), not a BGP-specific concern, so there is
no reason to duplicate or rename them for this binary.
- prevresult.go: inferFromPrevResult() reconstructs everything
galactic-bgp needs (interface kind — veth vs tap, and the allocated
IPAM result) purely from the previous plugin's prevResult, since
galactic-bgp itself never touches the container's network namespace.
Interface kind is inferred from prevResult shape: two interfaces
means veth (host+container), one means tap.
- resource.go: a resourceTracker scoped to exactly what galactic-bgp's
own ADD creates — BGPVRFInstance, BGPAdvertisement, and the eBPF
vrf_table entry. This is intentionally smaller than the old single
process-wide tracker: the kernel-interface/VRF cleanup that used to
live alongside these now belongs to each master plugin's own
tracker (internal/cni, internal/cnitap), scoped to exactly what its
own ADD creates. Selective rollback stays correct because each
plugin only ever rolls back what it itself created.
- ops_add.go/ops_del.go/ops_check.go: cmdAdd parses config, infers
from prevResult, publishes BGP state, and passes prevResult through
unchanged (galactic-bgp is the last plugin in the chain). cmdDel is
a no-op everywhere in the chain, as decided in the plan — orphaned
VRF/CRD state is reconciled independently by galactic-router's GC
controller, not by any CNI DEL path, so cmdDel does not need to
distinguish "resources this plugin created" from anything else.
cmdCheck is new logic (the old bgp.go had no CHECK story of its own):
it verifies the BGPVRFInstance and BGPAdvertisement CRDs exist and
cross-checks the eBPF vrf_table entry via a new checkEBPFEntry
helper. cmdStatus probes the API server, matching the STATUS story
every other binary in the chain now implements (per plan decision).
internal/cnibgp/bgp.go keeps the actual BGP/SRv6/eBPF logic moved over
from internal/cni/bgp.go, with everything now unexported since it is
package-internal to cnibgp rather than shared across internal/cni:
publishConfig, publishResult, publishBGPState, egressKindForInterface-
Type, unregisterEBPFDatapath, plus the untouched allocation/collision/
CRD-building helpers (allocateArgument, checkArgumentCollision,
lookupBGPRouter, buildVRFInstanceSpec, buildAdvertisementSpec,
ipamAdvertisementPrefixes, allAdvertisedPrefixes, registerEBPFDatapath,
isTransientError/retryK8sOps).
Design refinement beyond the original plan: ConfigureHostGateway and
its helpers (installGatewayNeighbor, ipv4GatewayAddrParams,
installGatewayRoute, routeConflicts) do NOT move into galactic-bgp.
They configure the container's default route to the VRF gateway
address, which is kernel-interface/netns work — exactly the kind of
dependency the plan's own rationale for splitting BGP out says
galactic-bgp should have zero of. Moving them into galactic-bgp would
have reintroduced that dependency one step later in the chain instead
of removing it. They now live in a new internal/cni/hostgw package,
called directly by both master plugins (galactic-cni and
galactic-tap-cni) right after they configure the container interface,
before handing off down the chain.
Also discovered while wiring inferFromPrevResult: types.PluginConf's
PrevResult field (from containernetworking/cni/pkg/types) has a json
tag of "-" and is never populated by json.Unmarshal; only the sibling
RawPrevResult map[string]interface{} field (tag "prevResult,omitempty")
actually receives the previous plugin's result. This is a pre-existing
quirk of that library, not something introduced by this split, and
existing code elsewhere in this repo already works around it by
reading RawPrevResult directly (e.g. ops_check.go). inferFromPrevResult
follows that same existing pattern. Left as-is rather than fixed here,
since fixing it is unrelated to this split's scope.
Taskfile.yaml, containers/galactic-cni/Dockerfile, and
internal/installer/installer.go (SourceBGPBinary) gain galactic-bgp
alongside galactic-cni/galactic-tap-cni/galactic-ipam, following the
exact same pattern established for those two in prior steps.
internal/cni/resource.go's resourceTracker drops all BGP/eBPF fields,
leaving only vpc, vpcAttachment, vrfCreated, routesCreated — cleanup()
no longer needs a Kubernetes client at all, since it never touches BGP
CRDs. internal/cni/result.go's buildVethResult calls hostgw.Configure-
HostGateway directly and returns only an error rather than threading
an IPAMResult/MAC address back up for the caller to hand to a bgp
helper that no longer lives in this package. Mirrored identically in
internal/cnitap.
Verification: task lint (0 issues), task build, task test:unit all
green. task test:e2e not run in this step (matches prior two steps in
this stack — requires sudo modprobe vrf plus a Kind cluster bring-up,
deferred to the end of the full stack per the plan's verification
approach).
privateip
force-pushed
the
refactor/cni-chain-2-galactic-bgp
branch
from
August 8, 2026 12:37
fd81bfd to
e33092c
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stack (merge bottom to top):
Summary
Third branch in the CNI plugin-chain split stack (based on #304).
galactic-bgpis now its own chained CNI plugin, invoked last in the conflist after the master plugin andgalactic-ipam, instead of in-process BGP/SRv6/eBPF publishing insidegalactic-cni/galactic-tap-cni.What moved
internal/cnibgpmirrorscniipam's shape:RunPlugin(),PluginConf{VPC, VPCAttachment, Namespace}parsed from stdin. It reusesinternal/config.CNIConfig'sGALACTIC_CNI_*env vars — shared node-level settings, not a BGP-specific flag.galactic-bgpnever touches the container netns. It learns interface kind (veth vs tap) and the allocated IPAM result fromprevResultalone (prevresult.go): two interfaces means veth, one means tap. It publishes BGP state, then passesprevResultthrough unchanged as the last plugin in the chain.cmdCheckis new: verifies theBGPVRFInstance/BGPAdvertisementCRDs and the eBPFvrf_tableentry.cmdStatusprobes the API server, matching STATUS across the chain.cmdDelstays a no-op — orphaned VRF/CRD state is reconciled bygalactic-router's GC controller, not by any CNI DEL path.Rollback scoping
resourceTrackerincnibgp/resource.gocovers only whatgalactic-bgp's own ADD creates:BGPVRFInstance,BGPAdvertisement, the eBPFvrf_tableentry.internal/cni/resource.go's tracker drops tovpc, vpcAttachment, vrfCreated, routesCreatedand no longer needs a Kubernetes client.hostgw extraction
ConfigureHostGatewayconfigures the container's default route — kernel-interface workgalactic-bgpshouldn't depend on. It now lives ininternal/cni/hostgw, called directly by both master plugins before handing off down the chain.PrevResult vs RawPrevResult
types.PluginConf.PrevResulthas JSON tag"-"and is never populated; onlyRawPrevResult map[string]interface{}is. Pre-existing library quirk, not introduced here —ops_check.goalready readsRawPrevResult, andinferFromPrevResultfollows the same pattern. Out of scope to fix.Verification
task lint✅task build✅task test:unit✅task test:e2enot run, same caveat as refactor(cni): split galactic-cni into veth/tap master plugins #303/refactor(cni): extract galactic-ipam as a real delegated CNI IPAM plugin #304.🤖 Generated with Claude Code