Skip to content

Integrate startup-verified IPAM into the node agent - #943

Merged
Patrick W. Healy (phealy) merged 14 commits into
mainfrom
phealy/ipam
Oct 21, 2026
Merged

Patrick W. Healy (phealy) merged 14 commits into
mainfrom
phealy/ipam

Conversation

@phealy

@phealy Patrick W. Healy (phealy) commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Summary

  • Publish in-memory IPAM ranges only after startup verification of the Kubernetes Node assignment; retain the managed bridge safety gate.
  • Atomically install the bundled plugin, expose runtime configuration and host mounts, and switch managed conflists to unbounded-ipam without embedded ranges.
  • Default PodCIDR assignment to manageCniPlugin while preserving explicit assignment overrides.
  • Support delayed delegated assignments and opt-in same-site pod routing; leave unmanaged local aggregates to the external CNI.
  • Apply reverse-path-filter setup to WireGuard interfaces created after delegated-CNI startup.
  • Keep external-CNI kube-proxy provisioning and declared-pool overlap validation independent of allocator defaults.

Scope

Depends on #942; review this layer against phealy/ipam-plugin. Azure-specific NodeNetworkConfig discovery and the azure-ipam alias are excluded and remain local on phealy/azure-managed-cilium.

Review fixes

  • 0fd6be9d9: merge gateway route sources before excluding unmanaged local pools; honor mesh and gateway peer route-skip flags in fallback routes.
  • 82579b038: preserve cluster CIDR selection and kube-proxy DaemonSet creation for external-CNI Sites with omitted assignment settings.
  • 1ac5e9e2b: preserve declared-pool overlap checks for those Sites, with IPv4 and IPv6 regression coverage.
  • f7776b70e: normalize local pool exclusions so equivalent IPv6 spellings cannot reintroduce unmanaged aggregates.
  • da872f158: document the persistent socket ownership lock inherited from Add verified Unix-socket unbounded-ipam CNI plugin #942.
  • 6577fcd64: track installed preferred sources so delayed PodCIDR assignment replaces otherwise-unchanged IPv4 and IPv6 routes.

CI repairs

  • a606d64e9: wait for fake Pod watch registration before testing deletion propagation; initial cache synchronization alone can lose the fake-client event.
  • f821e45ac: wait for the Azure GET response body wrapper before cancellation so the exactly-once-close assertion measures the intended interleaving.
  • Both flakes reproduced locally before the fixes. No production behavior, test assertions, or timeouts were weakened.

Validation

  • Affected package race tests, lint, and rendered deployment/operator tests passed.
  • Managed-CNI live tests: verified allocation, concurrency, offline CHECK/DEL, stale-bridge readiness/sandbox blocking, delayed assignment, controller/API outages, Site edits, restart, and transport transitions.
  • Delegated-Cilium live tests: allocation/connectivity, concurrency/offline cleanup, outages, fresh-node delayed assignment, and VXLAN/WireGuard transitions with strict reverse-path-filter defaults.
  • Review regressions reproduced before fixes. Full node, webhook, and netlink package race tests, focused managed kube-proxy controller race tests, and affected-package formatting/lint checks passed after fixes.
  • The broader controller suite retains an unrelated TestPeerIdentityCacheReuse allocation-count failure reproduced on main.
  • Both repaired tests passed 1,000 race-enabled repetitions with Go 1.26.10; full affected-package race suites and non-mutating lint passed.

Operational limitations

Verification is startup-only, not a continuous Node-identity watch. Legacy runtime-cached conflists invoking host-local bypass the gate until reloaded. No automatic leaked-reservation GC is added. Delegated Cilium currently requires restricting its managed devices to avoid TC ownership conflicts; the proposed staged-device datapath is not part of this PR.

@phealy
Patrick W. Healy (phealy) requested a review from a team as a code owner October 16, 2026 23:07
@phealy
Patrick W. Healy (phealy) added this pull request to stack #944 October 16, 2026 23:07
@phealy Patrick W. Healy (phealy) changed the title phealy/ipam Integrate startup-verified IPAM into the node agent Oct 16, 2026
@Lotarcc

Copy link
Copy Markdown
Contributor

Thanks for picking this up, Patrick W. Healy (@phealy). Defaulting PodCIDR assignment to manageCniPlugin is cleaner than the special case I had in #926, and it covers the controller half of #921.

One gap on the node side, in case it helps: configureTunnelPeers still skips gateway peers without PodCIDRs (cmd/unbounded-net-node/tunnel_config.go:485). With this branch, a new gateway in an unmanaged Site reaches remote nodes without PodCIDRs, so its routed CIDRs never get BPF entries. Today it only works because the gateway picks up a stray podCIDR. Existing gateways keep theirs since spec.podCIDR is immutable, so this only hits new gateway nodes.

I ran this branch (8a2867a) in a kind lab that mirrors #921: an unmanaged Calico Site with an Internal gateway pool, and a remote managed Site over GENEVE. Each run is a fresh cluster, so the gateway is a new node.

Path this PR + gatewayPeerReadyForTunnel from #926
remote pod -> gateway pod FAIL OK
gateway pod -> remote pod FAIL OK
remote pod -> member pod (via gateway) FAIL OK

No unmanaged-Site node got a podCIDR in either run, so the controller change works as intended. (member pod -> remote fails in both columns; that is the conntrack issue in #930, and it passes with #930 added.)

I've reduced #926 to just that node-side piece (one commit on main), so it no longer overlaps with this PR. It applies here with one trivial conflict. If you'd rather fold it in here, feel free to take it and I'll close #926.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

External-CNI configurations have unresolved overlap-validation, kube-proxy provisioning, and route-ownership regressions.

4 open issues
What changed in this PR

Integrates verified IPAM into the node agent, connecting the plugin from #942 to startup assignment checks and managed CNI configuration.

Changes:

  • Adds IPAM serving, atomic plugin installation, configuration, and host mounts.
  • Makes assignment defaults follow CNI management while preserving explicit overrides.
  • Supports delayed delegated assignments and optional same-site routing, with tests and documentation.
File Description
internal/​net/​webhook/​validation.go Applies assignment defaults to validation.
internal/​net/​webhook/​validation_test.go Tests unmanaged assignment validation.
internal/​net/​controller/​site_controller.go Updates allocation defaults and labeling.
internal/​net/​controller/​site_controller_podcidr_test.go Tests assignment overrides and labeling.
internal/​net/​controller/​site_controller_helpers_test.go Updates helper assertions.
internal/​net/​controller/​kubeproxy_controller.go Changes cluster CIDR selection.
internal/​net/​controller/​kubeproxy_controller_test.go Tests unmanaged CIDR selection.
internal/​net/​config/​runtime_config.go Adds IPAM and routing settings.
images/​net/​Containerfile Bundles the IPAM binary.
docs/​net/​custom-resources.md Explains assignment inheritance.
docs/​net/​configuration.md Documents IPAM operation and migration.
docs/​content/​reference/​networking/​custom-resources.md Updates assignment reference.
docs/​content/​reference/​networking/​architecture.md Clarifies external-CNI allocation.
deploy/​net/​node/​03-daemonset.yaml.tmpl Adds binary and socket mounts.
deploy/​net/​01-configmap.yaml.tmpl Exposes runtime settings.
deploy/​machina/​crd/​unbounded-cloud.io_sites.yaml Updates schema descriptions.
cmd/​unbounded-net-node/​wireguard_config.go Disables interface reverse-path filtering.
cmd/​unbounded-net-node/​tunnel_config.go Gates local aggregate routes.
cmd/​unbounded-net-node/​status_server.go Tracks verified assignments.
cmd/​unbounded-net-node/​site_watch_reconcile.go Reconciles delayed assignments and routing.
cmd/​unbounded-net-node/​main.go Wires IPAM into startup.
cmd/​unbounded-net-node/​main_update_test.go Tests routing overrides.
cmd/​unbounded-net-node/​main_config_test.go Registers IPAM test flags.
cmd/​unbounded-net-node/​ipam.go Implements serving support and installation.
cmd/​unbounded-net-node/​ipam_test.go Tests verification, installation, and configuration.
cmd/​unbounded-net-node/​delegated_routes_test.go Tests unmanaged pool exclusion.
cmd/​unbounded-net-node/​cni_safety_test.go Checks range-free conflists.
cmd/​unbounded-net-node/​cni_reconcile_test.go Tests delayed assignment reconciliation.
cmd/​unbounded-net-node/​bootstrap_helpers.go Validates assignments and switches IPAM.
cmd/​unbounded-net-node/​bootstrap_helpers_test.go Checks generated IPAM configuration.
api/​net/​v1alpha1/​types.go Documents assignment defaults.
api/​machina/​v1alpha3/​site_types.go Adds assignment-default resolution.
api/​machina/​v1alpha3/​site_types_test.go Tests inheritance and overrides.

Review effort: Balanced

Comment thread cmd/unbounded-net-node/tunnel_config.go Outdated
Comment thread cmd/unbounded-net-node/tunnel_config.go
Comment thread internal/net/controller/kubeproxy_controller.go Outdated
Comment thread internal/net/webhook/validation.go Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Host-network allocation and routing changes need human validation, and a routing correctness issue remains unresolved.

1 open issue
4 resolved since last review

Review effort: Balanced

Comment thread cmd/unbounded-net-node/tunnel_config.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Allocation gating, privileged host installation, and multi-transport routing require human review, with a delayed-assignment route-source issue still unresolved.

1 open issue
1 resolved since last review

Review effort: Balanced

Comment thread cmd/unbounded-net-node/site_watch_reconcile.go

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Allocation gating, host binary rollout, and managed/delegated routing transitions warrant final human review despite no additional confirmed defects.

No open issues 🎉

1 resolved since last review

Review effort: Balanced

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Needs a closer look

Host-level plugin installation, allocation gating, and routing lifecycle changes require human review of rollout safety across managed and delegated deployments.

No open issues 🎉

Review effort: Balanced

Base automatically changed from phealy/ipam-plugin to main October 21, 2026 18:48
Gate managed allocation on verified PodCIDRs and the existing bridge guard, and support delegated IPAM without managing third-party CNI configuration. Install the bundled binary atomically, expose ConfigMap and flag controls, package host mounts, and switch managed conflists to argument-free unbounded-ipam. Document migration and startup-only verification limits.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8bb0fa01-6bf2-4bcd-9143-5c3f634e282f
Resolve omitted assignmentEnabled values from manageCniPlugin while preserving explicit overrides. Use the same effective setting for both allocation paths, admission validation, and kube-proxy CIDR selection. Keep site labeling independent of assignment enablement, cover all setting combinations and label failures, and regenerate API descriptions and update delegated-IPAM documentation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8bb0fa01-6bf2-4bcd-9143-5c3f634e282f
Match runtime range option filtering to Go JSON decoding and verify rejection before reservations. Reconcile newly verified delegated assignments into routing state, retrying after dataplane failures.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8bb0fa01-6bf2-4bcd-9143-5c3f634e282f
Keep CNI ownership independent from explicit same-site routing ownership. Preserve the unmanaged-CNI default while allowing delegated IPAM deployments without an external native-route controller.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8bb0fa01-6bf2-4bcd-9143-5c3f634e282f
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 8bb0fa01-6bf2-4bcd-9143-5c3f634e282f
Do not rely on the init container disabling reverse-path filtering before another CNI changes the default for newly created interfaces. Apply the existing per-interface setup after bringing up mesh and gateway WireGuard links, matching the shared tunnel setup.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 8bb0fa01-6bf2-4bcd-9143-5c3f634e282f
Filter local pools after merging all gateway route sources and honor peer SkipPodCIDRRoutes flags in fallback routes. Add gateway-input and skipped-peer regression coverage.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6b70e00f-eec0-4611-97c6-1b6200092b62
Keep omitted assignment settings eligible for kube-proxy CIDR selection independently of allocator defaults. Cover unmanaged Site DaemonSet creation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6b70e00f-eec0-4611-97c6-1b6200092b62
Preserve declared-pool overlap checks when allocator enablement inherits the unmanaged CNI default. Cover overlapping and disjoint IPv4 and IPv6 pools.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6b70e00f-eec0-4611-97c6-1b6200092b62
Compare canonical CIDR identities so equivalent IPv6 gateway aggregates cannot bypass external CNI route ownership. Extend the gateway regression to cover alternate IPv6 spellings.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6b70e00f-eec0-4611-97c6-1b6200092b62
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6b70e00f-eec0-4611-97c6-1b6200092b62
Snapshot each installed route source and include it in differential updates. Cover delayed IPv4/IPv6 assignment, source changes and removal, stable destinations and nexthops, and unchanged-source no-ops.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6b70e00f-eec0-4611-97c6-1b6200092b62
The fake tracker does not replay deletions between its initial list and watch registration. Wait for the existing watch reactor to register before asserting deletion propagation; retain the original lifecycle and scope checks.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6b70e00f-eec0-4611-97c6-1b6200092b62
Wait until the GET response body is wrapped before canceling the request whose body-close count is asserted. Server-side header readiness alone races with client transport completion.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 6b70e00f-eec0-4611-97c6-1b6200092b62
@phealy
Patrick W. Healy (phealy) added this pull request to the merge queue Oct 21, 2026
Merged via the queue into main with commit fa8ef09 Oct 21, 2026
99 of 138 checks passed
@phealy
Patrick W. Healy (phealy) deleted the phealy/ipam branch October 21, 2026 19:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants