Delete leaked minion keys at teardown in integration test fixtures (#69728)#69729
Open
ggiesen wants to merge 2 commits into
Open
Delete leaked minion keys at teardown in integration test fixtures (#69728)#69729ggiesen wants to merge 2 commits into
ggiesen wants to merge 2 commits into
Conversation
The startup_states and salt_call ownership test modules start extra
minions against the shared session master via 'with factory.started():
yield'. That stops the minion process at teardown but leaves the
accepted key on the master, so later tests in the same session that
target '*' (the netapi integration tests) key-match minions that no
longer exist. The result is the Rocky Linux 9 integration tcp/zeromq
pair failing most 3006.x PR runs:
AssertionError: assert ['minion-X', 'minion-X-empty-string',
'minion-X-highstate', 'minion-X-sls', 'minion-X-top',
'non-root-minion-Y', 'sub-minion-Z'] == ['minion-X', 'sub-minion-Z']
plus 30 second TimeoutErrors waiting for returns from the dead minions.
Nightly runs do not hit it because test sharding separates these
modules from the netapi tests.
Remove each extra minion's key from the master once the minion is
stopped, restoring the isolation the netapi assertions rely on.
Fixes saltstack#69728
This was referenced Jul 6, 2026
2 tasks
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
2 participants
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?
Implements option 1 from #69728: the
startup_statesand salt-call ownership integration test fixtures now delete their extra minions' accepted keys from the shared session master at teardown.Both modules start additional minions with
with factory.started(): yield factory, which stops the minion process but leaves its accepted key on the master. Later tests in the same pytest session that target*-- the netapi integration tests -- key-match those dead minions, producing the failure pair seen on most 3006.x PR runs ("Rocky Linux 9 integration tcp/zeromq"):plus
TimeoutError: Operation timed out after 30 secondson the synchronous variants waiting for returns from minions that no longer exist. Nightly runs never see it because test sharding separates the leaking modules from the netapi assertions.What issues does this PR fix or reference?
Fixes #69728
Previous Behavior
Extra minions' keys accumulated on the session master; netapi
*-targeting tests failed with inflated minion lists and 30 s timeouts on most PR runs, while nightly stayed green.New Behavior
Each fixture removes its minion's key (
salt-key -d <id> -yvia the master factory) once the minion is stopped, restoring the isolation the netapi assertions rely on. The cleanup call semantics were validated against a live salt-factories master (planted an accepted key file, ran the exact call, key removed, exit code 0). The tests themselves are unchanged -- only teardown is added.This keeps the netapi tests' strict
*assertions intact. Hardening those tests against foreign keys (option 2 in the issue) can still be done separately if desired.Merge requirements satisfied?
Test-only change; the affected tests are exercised by the same CI jobs this PR aims to stabilize.
Commits signed with GPG?
No