Fix non-deterministic pillar rendering across pillar_roots envs (#44937)#69598
Open
dwoz wants to merge 1 commit into
Open
Fix non-deterministic pillar rendering across pillar_roots envs (#44937)#69598dwoz wants to merge 1 commit into
dwoz wants to merge 1 commit into
Conversation
Pillar.get_tops collected saltenvs into a set and iterated them in hash order, so top-file processing depended on PYTHONHASHSEED and varied between salt-call invocations. Commit b091998 (Jan 2021) made _get_envs return an ordered list to fix issue saltstack#24501, but the caller immediately wrapped the result back into a set, throwing the order away. The defect was present and identical on 3006.x, 3007.x, 3008.x, and master. Switch saltenvs to an insertion-ordered dict so iteration follows _get_envs order (which itself follows pillar_roots config order). The "pillar_source_merging_strategy: none" branch keeps its intersection-with-saltenv semantic but preserves the source ordering. Add a unit regression test that builds four pillar envs all matching the same minion and asserts get_tops returns them in _get_envs order. Fixes saltstack#44937 Helps saltstack#24501
642fd84 to
004418c
Compare
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 #44937 - non-deterministic pillar rendering when a minion is
matched by top.sls files in multiple
pillar_rootsenvironments.Pillar.get_topscollected saltenvs into asetand iterated themin hash order. Because CPython randomizes
PYTHONHASHSEEDperprocess, the order in which top files were processed (and therefore
which env's pillar data won on key conflicts) changed between
salt-callinvocations.Why the existing fix is not enough
Commit b091998 (Jan 2021, "Preserve order of pillar_roots in
_get_envs()") was intended to fix this class of bug for #24501. It
made
_get_envsreturn an ordered list. However the caller inget_topsimmediately wrapped that list back into aset:The defect was present and byte-identical on 3006.x, 3007.x, 3008.x,
and master.
What this PR changes
Replaces the
setwith an insertion-ordereddict, so iterationfollows
_get_envsorder (which itself followspillar_rootsconfig order). The
pillar_source_merging_strategy: nonebranchpreserves its intersection-with-saltenv semantic while keeping source
ordering.
Tests
Adds a unit regression test
test_pillar_get_tops_iterates_envs_in_pillar_roots_orderthat:
pillar_rootsenvs whose top.sls all match the sameminion
Pillar.get_tops()topskeys are in_get_envsorderThe test fails reliably on unpatched 3006.x (e.g.
['baz', 'base', 'bar', 'foo'] != ['base', 'foo', 'bar', 'baz'])and passes 20/20 with the patch under
PYTHONHASHSEED=random.The user-supplied repro script from the issue (10 iterations of
salt-call --local pillar.items) is also stable after the patch(50/50 deterministic output) where it produced mixed results before.
Related
env_orderis not respected during pillar data merging withmerge_all#68785 (env_order)Merge-forward
The same defect exists on 3007.x, 3008.x, and master; this PR targets
3006.x as the oldest supported branch so the fix can be
merged-forward.
Commits signed-off?
N/A — Apache 2.0 contribution, no DCO required.