Resolve master_uri in minion engines to fix pillar.data KeyError (#57952)#69781
Open
ggiesen wants to merge 1 commit into
Open
Resolve master_uri in minion engines to fix pillar.data KeyError (#57952)#69781ggiesen wants to merge 1 commit into
ggiesen wants to merge 1 commit into
Conversation
A minion forks its engines early in startup, before it has connected and resolved master_uri into its opts, so the engine runs against a snapshot that lacks it. Any __salt__ call needing the master transport (e.g. pillar.data) then raises KeyError: 'master_uri' in salt.crypt. Engine.run() now completes the engine's opts by resolving master_uri in the engine's own process before the execution modules are loaded. It is best-effort and cannot block or break engine startup: master engines are skipped by role, masterless minions need no transport, and DNS is resolved once without the minion connect loop's retry so a transport-less engine still starts when the master is unresolvable. Fixes saltstack#57952
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.
What does this PR do?
Fixes
KeyError: 'master_uri'when a minion engine calls a salt function that needs the master transport (for examplepillar.data), by resolvingmaster_uriin the engine's own process.What issues does this PR fix or reference?
Fixes #57952
Previous Behavior
A minion forks its engines early in startup --
Minion.__init__schedulesstart_enginesviaio_loop.spawn_callback(salt/minion.py:1538-1541), andadd_processforks the engine subprocess right away -- before the minion connects and resolvesmaster_uriinto its opts (opts.update(resolve_dns(opts))at salt/minion.py:875, reached fromconnect_master). The engine subprocess therefore holds a snapshot of opts that lacksmaster_uri, andEngine.run()never refreshes it. Any__salt__call that builds the master transport (pillar.data->Pillar->ReqChannel.factory->AsyncAuth) readsopts["master_uri"]with no default insalt.cryptand raisesKeyError: 'master_uri'.Scheduled jobs (e.g.
mine.update) fork after connect, so their opts already havemaster_uri-- which is why the reporter sawmine.updatesucceed concurrently while the engine'spillar.datafailed.New Behavior
Engine.run()resolvesmaster_uriin the engine's own process before the execution modules are loaded/used. It is best-effort so it can never block or break engine startup:__role != "minion";master.pystarts engines as__role == "master"and needs nomaster_uri);file_client: local) are skipped (no transport needed);master_uriis left untouched;retry_dns=0) and any failure is caught and logged, so a transport-less engine, or one whose master is momentarily unresolvable at boot, still starts -- failing no worse than today (this covers the failover/list-master case, where a single host cannot be resolved yet).This works on both fork and spawning platforms: on fork the engine's
self.optsis the same object the loader closed over; on spawning platforms the execution modules are rebuilt againstself.optsafterrun()begins -- so completing the opts at the top ofrun()reaches__salt__either way.Verified with a real master+minion: an engine calling
pillar.datawent fromKeyError: 'master_uri'to returning the pillar dict.On the deeper fix
This is a contained fix: the engine completes its own opts. The underlying cause is that a regular minion forks its engines before it connects, so the engine snapshots incomplete opts in the first place. The more architectural fix would be to defer regular-minion engine start until after the first connect (from
_post_master_init/tune_in), which is exactly how proxy minions already start their engines (salt/metaproxy/proxy.py:161-166); the engine would then inherit a fully-resolved opts and need no in-engine resolution. I kept it contained here because deferring engine start touches minion startup ordering and would have to preserve behaviour for masterless minions (which never connect) and failover re-spawns. Happy to take that approach instead if you'd prefer the root-cause fix.Merge requirements satisfied?
changelog/57952.fixed.mdtests/pytests/unit/engines/test_engines.py(resolves for a minion; skips master/already-present/masterless; non-fatal on resolution failure)Commits signed with GPG?
No