Surface per-minion detail when salt.state orchestrate fails (#68326)#69425
Open
dwoz wants to merge 1 commit into
Open
Surface per-minion detail when salt.state orchestrate fails (#68326)#69425dwoz wants to merge 1 commit into
dwoz wants to merge 1 commit into
Conversation
When a minion targeted by the salt.state orchestrate state returned
False (e.g. state.sls hit a pre-compilation error or the master-level
saltutil.cmd response was flagged failed with no useful ret payload),
the orchestrate run reported only "Run failed on minions: <minion>"
and an opaque False in changes. Operators had to re-run with extra
logging to find out anything about why their orchestrate failed.
Capture per-minion failure detail during the loop over cmd_ret and
append it to the orchestrate comment. The detail distinguishes three
cases:
- the minion returned no usable ret at all,
- the minion returned a list of error strings (queue conflict, pillar
errors, etc.), and
- the minion returned a scalar (typically False) instead of a state
result.
A new helper, _format_failure_detail, encapsulates the formatting and
deliberately returns an empty string when the minion's ret is a dict so
the existing pre-existing comment shape ("Run failed on minions: X") is
preserved for that path; the structured changes payload already speaks
for itself there.
Fixes saltstack#68326
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
1 participant
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?
Adds per-minion failure detail to the comment that
salt.statewrites when an orchestrate target minion fails to return a state-result dict, so operators see why an orchestrate run failed instead of justRun failed on minions: <minion>.What issues does this PR fix or reference?
Fixes #68326
Previous Behavior
When
salt-run state.orchestratetargeted a minion that returnedFalse(e.g.state.slshit a pre-compilation error, a queue conflict, or the master-levelsaltutil.cmdflagged the response as failed with no useful payload), the orchestrate state reported only:with no indication of what
Falsemeant or what actually went wrong on the wire. The reporter had no way to diagnose without re-running with extra logging.New Behavior
When the minion's return value is not a state-result dict the orchestrate comment now appends a per-minion detail line, distinguishing three cases:
False) instead of a state result.For example, the reporter's scenario now produces:
When the minion did return a valid state-result dict (the historical happy-path-with-
failed: Trueshape covered by the existingtest_state_failed_and_expected_minions), the comment text is unchanged because the structuredchangespayload already speaks for itself.Merge requirements satisfied?
Commits signed with GPG?
Yes