Skip to content

net: skip no-op managed kube-proxy reconciliation - #914

Merged
Patrick W. Healy (phealy) merged 4 commits into
mainfrom
phealy/net-api-controller-noops
Oct 23, 2026
Merged

Patrick W. Healy (phealy) merged 4 commits into
mainfrom
phealy/net-api-controller-noops

Conversation

@phealy

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

Copy link
Copy Markdown
Contributor

Summary

Stop the unbounded-net controller from writing managed kube-proxy objects that haven't changed.

  • Cache comparison. Compare the desired DaemonSet with the informer cache, normalizing fields the API server defaults. The controller writes only when there is real drift. It confirms against a live GET before updating.
  • Service links default. Normalize the enableServiceLinks default so a value the server filled in doesn't count as drift. A value someone actually disabled is still repaired.
  • Narrower requeues. DaemonSet and Site events trigger reconciliation only when they change something the reconcile depends on. For example, status-only updates don't.
  • Design doc. designs/managed-kube-proxy.md describes the new behavior.

Testing

Unit tests cover:

  • an unchanged cache making no client calls
  • server-defaulted fields
  • real spec drift and owned-label drift
  • service links that were explicitly disabled
  • a stale cache whose live object is unchanged
  • GET errors and update conflicts
  • recreating the DaemonSet after deletion
  • event relevance for DaemonSet and Site updates

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%.

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

Event filtering can miss same-name DaemonSet replacements, leaving managed drift or provider coverage unreconciled.

1 open finding
What changed in this PR

Reduces API server load by avoiding unchanged managed kube-proxy writes in the network controller.

Changes:

  • Compares cached and live DaemonSets with normalized server defaults before writing.
  • Filters DaemonSet and Site events to reduce unnecessary reconciliation.
  • Adds regression tests and documents the reconciliation behavior.
File Description
internal/​net/​controller/​kubeproxy_controller.go Adds no-op detection, default normalization, and event filtering.
internal/​net/​controller/​kubeproxy_controller_test.go Tests reconciliation, drift repair, errors, and event relevance.
designs/​managed-kube-proxy.md Documents cache-first comparison and live verification.

🧠 Review effort: Balanced


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

Comment thread internal/net/controller/kubeproxy_controller.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.

🟢 Approval recommended

Static review identified no blocking issues; tests were inspected but not executed.

0 open findings

1 resolved since last review

🧠 Review effort: Balanced

Use informer cache comparisons and normalized server defaults to avoid no-op API writes, while narrowing DaemonSet and Site event requeues.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

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

Copilot-Session: ab9b78ee-42b8-4841-8d74-cd78d002d48b
Generations are only comparable within one UID, so treat a same-name
replacement observed as an update as relevant.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: ab9b78ee-42b8-4841-8d74-cd78d002d48b
Reuse the controller fixture so reconciliation has both a fake client and the informer-backed lister required by the cache fast path.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 46ff6261-c4cc-4fb0-9fb7-4e66d2b9c203
@phealy
Patrick W. Healy (phealy) force-pushed the phealy/net-api-controller-noops branch from b497d1b to ff4e37b Compare October 23, 2026 21:02
stacked auto-merge was automatically disabled October 23, 2026 23:31

Auto-merge was disabled for another pull request in the stack

@phealy
Patrick W. Healy (phealy) added this pull request to the merge queue Oct 23, 2026
Merged via the queue into main with commit 9d007e9 Oct 23, 2026
35 checks passed
@phealy
Patrick W. Healy (phealy) deleted the phealy/net-api-controller-noops branch October 23, 2026 23:47
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.

3 participants