Skip to content

Give each deltaproxy sub-proxy its own schedule and beacon storage (#65088)#69788

Open
ggiesen wants to merge 1 commit into
saltstack:3006.xfrom
ggiesen:fix-65088-deltaproxy-keepalive
Open

Give each deltaproxy sub-proxy its own schedule and beacon storage (#65088)#69788
ggiesen wants to merge 1 commit into
saltstack:3006.xfrom
ggiesen:fix-65088-deltaproxy-keepalive

Conversation

@ggiesen

@ggiesen ggiesen commented Jul 12, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Gives each deltaproxy sub-proxy its own schedule and beacons storage, so
per-sub-proxy __proxy_keepalive jobs no longer collide.

subproxy_post_master_init builds each sub-proxy's opts with a shallow
opts.copy(), so proxyopts["schedule"] and proxyopts["beacons"] were the
same dict objects as the control minion's. The schedule and beacon helpers
mutate those dicts in place -- Schedule.add_job does
opts["schedule"].update(...) -- so every sub-proxy's
add_job("__proxy_keepalive", ...) overwrote the same key in the one shared
dict. Only the last sub-proxy kept a keepalive job, so only one of N sub-proxies
got a __proxy_keepalive. Per-sub-proxy beacons collided the same way.

Each sub-proxy's schedule and beacons come from its own pillar plus the
per-sub-proxy keepalive that subproxy_post_master_init adds, so they were
never meant to share the control minion's dicts; the sharing was an accident of
the shallow copy. The fix gives each sub-proxy its own storage.

What issues does this PR fix or reference?

Fixes #65088

Previous Behavior

With N napalm sub-proxies, only one __proxy_keepalive scheduled job existed
(the reporter saw 1 of 8), because all sub-proxies shared the control minion's
schedule dict and each keepalive add_job overwrote the previous. Schedules
defined in a sub-proxy's own pillar worked, because those come from
opts["pillar"]["schedule"] (distinct per sub-proxy), which is why the reporter
saw the pillar route work while __proxy_keepalive did not.

New Behavior

Each sub-proxy gets its own schedule and beacon dict, so every sub-proxy keeps
its own __proxy_keepalive (and any beacons), and the control minion's schedule
is untouched.

Merge requirements satisfied?

  • Docs
  • Changelog
  • Tests written/updated

tests/pytests/unit/metaproxy/test_deltaproxy.py::test_subproxy_post_master_init_isolates_schedule
asserts each sub-proxy's schedule and beacons are distinct dict objects
(not the control minion's and not each other's) and that the control minion's
dicts are left untouched. It fails against the pre-fix code (the sub-proxies
share the one dict).

Also reproduced and verified on real hardware: a napalm deltaproxy over live
Junos devices with four sub-proxies. Before the fix, sub-proxies shared a
schedule dict (colliding keepalives) and all shared one beacons dict; with the
fix each sub-proxy's schedule and beacon dicts are distinct.

Commits signed with GPG?

No

…altstack#65088)

subproxy_post_master_init builds each sub-proxy's opts with a shallow
opts.copy(), so proxyopts["schedule"] and proxyopts["beacons"] were the same
dict objects as the control minion's. The schedule and beacon helpers mutate
those dicts in place -- Schedule.add_job does opts["schedule"].update(...) --
so every sub-proxy's add_job("__proxy_keepalive", ...) overwrote the same key
in the one shared dict. Only the last sub-proxy kept a keepalive job, so only
one of N sub-proxies got a __proxy_keepalive (the reported symptom); per
sub-proxy beacons collided the same way.

Give each sub-proxy its own schedule and beacon storage. Their jobs and beacons
come from their own pillar plus the per-sub-proxy keepalive added below, so they
were never meant to share the control minion's dicts (the sharing was an
accident of the shallow copy).
@ggiesen ggiesen requested a review from a team as a code owner July 12, 2026 00:10
@dwoz dwoz added the test:full Run the full test suite label Jul 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

test:full Run the full test suite

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants