[v3.32] [BPF] Re-wire SetTriggerFn on syncer swap in proxy.SetSyncer#12743
Merged
tomastigera merged 1 commit intoMay 12, 2026
Conversation
proxy.New() wires the syncer's expand-NodePort fixup callback to the proxy's runner via dp.SetTriggerFn(p.runner.Run). proxy.SetSyncer() swaps in a fresh syncer (e.g. on a host-IP change inside KubeProxy.run()) but did not re-wire that callback. Result: after any host-IP change the new syncer's expand-NodePort fixup goroutine resolves previously-missed routes for ExternalTrafficPolicy=Local NodePort backends, but cannot schedule the dataplane Apply that would program them — the fix-up is silently lost until something else piggybacks an Apply. Pointed out by review on the v3.31 backport (projectcalico#12693) of projectcalico#12667; the asymmetry is pre-existing (it was equally broken before projectcalico#12667 restructured run()) but became plain to read after the restructure. Make SetSyncer call s.SetTriggerFn(p.runner.Run) on the new syncer under the same lock as the dpSyncer swap, mirroring what proxy.New() does for the initial syncer. Add a regression test that swaps a syncer via SetSyncer and verifies the new syncer received a non-nil trigger callback that schedules an Apply when invoked. The test fails on master before this change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
marvin-tigera
added
release-note-required
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes an eBPF kube-proxy regression where swapping the dataplane syncer via proxy.SetSyncer() failed to re-wire the syncer’s “route-fixup” trigger callback, causing NodePort externalTrafficPolicy=Local route-fixups to be unable to schedule a dataplane Apply after host-IP changes.
Changes:
- Re-wire
DPSyncer.SetTriggerFn(p.runner.Run)insideproxy.SetSyncer()under the syncer swap lock, mirroringproxy.New(). - Add a regression test to validate that a syncer swap re-arms the trigger and that invoking it schedules an Apply.
- Extend the test mock syncer to record the trigger function passed via
SetTriggerFn.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| felix/bpf/proxy/proxy.go | Ensures SetSyncer re-wires the new syncer’s trigger callback so route-fixup can schedule Applies after a swap. |
| felix/bpf/proxy/proxy_test.go | Adds regression coverage for trigger wiring on syncer swap and updates the mock syncer to capture the trigger function. |
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
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.
Description
Cherry-pick of #12701 to
release-v3.32.proxy.New()wires the syncer's expand-NodePort fixup callback viadp.SetTriggerFn(p.runner.Run).proxy.SetSyncer()swaps in a fresh syncer (e.g. on a host-IP change insideKubeProxy.run()) but did not re-wire that callback. After any host-IP change the new syncer's expand-NodePort fixup goroutine resolves previously-missed routes forExternalTrafficPolicy=LocalNodePort backends, but cannot schedule the dataplane Apply that would program them — the fix-up is silently lost until something else piggybacks an Apply.Pre-existing bug (
kp.run()always created a fresh syncer and calledSetSynceron it); #12667 made it plain to read because the first call now goes throughproxy.New()which sets the trigger, leaving the asymmetry withSetSyncerobvious. Caught on the v3.31 backport review (#12693).Fix
Have
SetSyncercalls.SetTriggerFn(p.runner.Run)on the new syncer, under the same lock as the dpSyncer swap, mirroringproxy.New().Test
Regression test in
proxy_test.goswaps a syncer viaSetSyncer, verifies the new syncer received a non-nil trigger callback, and confirms invoking it schedules an Apply. The test fails on master / 3.32 / 3.31 before this change.Related issues/PRs
Cherry-pick of #12701. Follow-up to #12667; pairs with #12694 (already merged on this branch).
Release Note
Reminder for the reviewer
release-note-requireddocs-not-required(no user-facing config change; bug fix)