Skip to content

net: adopt and acknowledge node assignments in the agent - #917

Draft
Patrick W. Healy (phealy) wants to merge 3 commits into
phealy/net-api-assignment-controllerfrom
phealy/net-api-assignments
Draft

Patrick W. Healy (phealy) wants to merge 3 commits into
phealy/net-api-assignment-controllerfrom
phealy/net-api-assignments

Conversation

@phealy

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

Copy link
Copy Markdown
Contributor

Summary

Have the node agent adopt its Site and GatewayPool assignment once per process and acknowledge it.

  • Adoption. At startup, the agent reads its own Node and fixes the Site and GatewayPool for the life of the process. Managed CNI setup is tied to that assignment. Gateways still get their CNI configuration from the Site.
  • Acknowledgment. After its first successful reconciliation, the agent records the assignment it is running on its own Pod. The controller from net: publish node assignments and replace stale agents #916 uses this to replace outdated agents.
  • Startup-only Node watch:
    • The agent watches only its own Node, from adoption until the acknowledgment is saved. The watch uses a field selector and starts from a validating GET.
    • Any of these restarts the container: a UID, Site or pool change, or the Node being deleted.
    • This prevents an agent whose first reconciliation keeps failing from being stuck on an old assignment, a case the controller skips because the Pod hasn't acknowledged.
    • The watch stops once the acknowledgment is written, so no long-lived Node watch remains.
  • Taint removal. When a gateway is demoted, removing its taint retries only on patch races, with a capped exponential backoff. An admission denial is returned immediately.

Testing

  • Unit tests for:
    • adoption and acknowledgment ownership
    • startup watch cancellation and revalidation after a disconnect
    • which taint-removal errors are retried, and the retry limits
  • Tested on an isolated AKS cluster with remote workers:
    • Set a GatewayPool label that didn't exist, so the agent couldn't acknowledge.
    • Removed the label. The startup watch restarted the container without the controller replacing the Pod, and the agent then acknowledged the correct assignment.
    • Audit logs showed every watch on the agent's own Node closed. The last one closed 1.8 ms after the acknowledgment.

Stack

This is one of five stacked PRs that reduce API server load from unbounded-net-controller and unbounded-net-node. Kubernetes remains the source of truth. Review them bottom to top:

  1. net: skip no-op managed kube-proxy reconciliation #914 Controller no-op reconciliation
  2. net: reconcile SiteNodeSlices per Site #915 Targeted SiteNodeSlice reconciliation
  3. net: publish node assignments and replace stale agents #916 Controller-published node assignments
  4. net: adopt and acknowledge node assignments in the agent #917 Node agent assignment adoption
  5. net: scope worker gateway topology watches #918 Scoped worker gateway watches

Motivation. A large cluster running v0.8.0 had the controller continuously writing objects that hadn't changed. Every node agent also watched the whole cluster's gateway topology. On a canary cluster, the full stack cut controller API calls by 54.4% and node agent API calls by 73.8%.

Pin Site and GatewayPool membership for the agent lifetime, acknowledge the effective values, and fence managed CNI bootstrap to the adopted Node assignment while retaining Site-driven gateway CNI.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ab9b78ee-42b8-4841-8d74-cd78d002d48b
Monitor only the own Node from adoption through bootstrap and initial reconciliation, cancel startup on assignment or identity changes independently of dataplane success, and stop the watch after effective acknowledgment is persisted. Revalidate after disconnects and preserve mismatch causes without allowing stale acknowledgments.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ab9b78ee-42b8-4841-8d74-cd78d002d48b
Retry only patch races (conflicts and failed JSON patch tests) with capped
exponential backoff, and return admission denials immediately instead of
spinning until the cleanup context expires.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ab9b78ee-42b8-4841-8d74-cd78d002d48b

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

Transition cleanup has unresolved concurrency and recovery issues that can leave stale networking configuration, advertisements, or taints.

4 open findings
What changed in this PR

Pins each node agent’s assignment for its process lifetime and acknowledges it for controller-driven replacement, supporting the API-load reduction stack.

Changes:

  • Adopt assignments and monitor changes until acknowledgment succeeds.
  • Tie CNI setup and transition cleanup to the adopted assignment.
  • Add bounded taint-removal retries and lifecycle tests.
File Description
cmd/​unbounded-net-node/​startup_assignment_watch_test.go Tests watch cancellation and revalidation.
cmd/​unbounded-net-node/​site_watch_reconcile.go Integrates acknowledgment and transition cleanup.
cmd/​unbounded-net-node/​node_types.go Tracks adopted assignment and taint state.
cmd/​unbounded-net-node/​main.go Wires adoption into startup.
cmd/​unbounded-net-node/​gateway_taint_test.go Tests retry classification and limits.
cmd/​unbounded-net-node/​gateway_pool_peering_protocol_test.go Tests gateway readiness requirements.
cmd/​unbounded-net-node/​bootstrap_helpers.go Validates bootstrap assignments and removes taints.
cmd/​unbounded-net-node/​bootstrap_helpers_test.go Checks taint removal.
cmd/​unbounded-net-node/​bootstrap_assignment_test.go Tests assignment-aware CNI bootstrap.
cmd/​unbounded-net-node/​assignment.go Implements adoption, monitoring, and acknowledgment.
cmd/​unbounded-net-node/​assignment_test.go Tests assignment lifecycle helpers.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

}

defer cleanupNodeNetworkingOnShutdown(cfg, state)
defer cleanupForAssignmentTransition(cfg, state, dynamicClient, gatewayNodeInformer)
Comment on lines +533 to +534
if adopted.gatewayPool != "" {
if err := syncGatewayNodeRoutesStatus(cleanupCtx, dynamicClient, gatewayNodeInformer, state.nodeName, nil); err != nil {
Comment on lines +539 to +541
if adopted.gatewayPool != "" && current.gatewayPool == "" {
if err := removeGatewayNodeTaint(cleanupCtx, state.clientset, state.nodeName); err != nil {
klog.Errorf("Failed to remove gateway taint during role demotion: %v", err)
}
}

cleanupNodeNetworkingOnShutdown(cfg, state, transition)

This branch has not been deployed

No deployments
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.

2 participants