Repository navigation
fix(net): program unmanaged-Site gateways without pod CIDRs - #926
Patrick W. Healy (phealy) merged 3 commits into
Conversation
|
@microsoft-github-policy-service agree company="MODUS X LLC"
|
|
Patrick W. Healy (@phealy) Cameron Childress (@cchildress) could one of you approve the workflow runs for this and #930 / #931? They are The three PRs come from running Unbounded on AKS with Azure CNI pod subnets and an on-prem Flex I see #914-#918 rework the same area; happy to rebase onto them or adjust if you prefer a |
cd84160 to
579b1f3
Compare
Code reviewNo issues found. Checked for bugs and CLAUDE.md compliance. Reviewed head Nonblocking limitations:
One wording correction for eventual integration with #943: unmanaged Sites do not necessarily lack PodCIDRs, because explicit assignmentEnabled=true or an external allocator can provide them. The implementation already handles that case correctly; only the “never gets a podCIDR” comments are too absolute. Leaving this PR separate for the author/user to merge; no cherry-pick or rebase performed. |
Patrick W. Healy (phealy)
left a comment
There was a problem hiding this comment.
Still need to deal with health checks of gateway pool nodes that don't have a podCIDR
Resolve gateway peer construction by preserving both the unmanaged-Site tunnel flag and the opt-in same-site PodCIDR route ownership setting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6b70e00f-eec0-4611-97c6-1b6200092b62
Remote nodes skip gateway peers without pod CIDRs (
configureTunnelPeers), so the gateway's routed CIDRs never get BPF entries and remote -> local traffic through it fails. A gateway in a Site withmanageCniPlugin: falseshould not have a podCIDR at all; today it only works because the controller assigns a stray one. #943 stops that assignment, so new gateways in unmanaged Sites hit this.A gateway peer without pod CIDRs is now programmed when its Site is unmanaged and it has routed CIDRs. Managed-Site gateways still wait for their podCIDR, as before (they publish their WireGuard key before CNI setup).
This PR used to also skip podCIDR assignment in the controller. #943 does that more cleanly, so this is now the node-side piece only. Happy to fold it into #943 instead if that is easier.
Lab (kind, unmanaged Calico Site with an Internal gateway pool, remote managed Site over GENEVE), on #943 at 8a2867a, fresh cluster per run:
Also checked with a two-gateway pool on #943 + this PR: remote pod <-> both gateway pods pass.
Not changed: health checks skip gateway peers without pod CIDRs (
peer_healthcheck.go), so an unmanaged gateway is not health-checked or failed over. I have a follow-up for that and will send it once this lands.Part of #921.
Testing:
TestGatewayPeerReadyForTunnel,TestUpdateWireGuardFromSlices_UnmanagedSiteGatewayWithoutPodCIDRIsTunnelReady;go test ./cmd/unbounded-net-node/ ./internal/net/netlink/ ./internal/net/controller/on Linux; golangci-lint on the changed packages; lab runs above.