Return runner return code in a way compatible with check_state_result #39641
Conversation
Previously, the return code of the runner (if any) was supplied in ret['data']['retcode']. This was problematic if ret['data'] was later processed by check_state_result. With this change, runners return the optional return code in ret['retcode'], like the other code (modules, etc.) already did before.
|
@smarsching, thanks for your PR! By analyzing the history of the files in this pull request, we identified @anttix, @isbm and @s0undt3ch to be potential reviewers. |
|
Sorry, for the mess. I accidentally hit the return key, so the PR was created before I had put in all data. I made this PR against 2016.3 because it is the earliest release that has the bug, but I also have a patch against the develop branch, in case you prefer that one. |
smarsching
changed the title
|
@smarsching does tests pass? |
|
I finally managed to install all the dependencies needed to run the tests. Running the tests took ages and a few of them failed:
I do not think that these failures are related to my changes (e.g. I just saw that in the meantime, Jenkins has also finished running and the checks have passed, so I guess the changes are okay. |
|
@smarsching Naw, only related tests to this. Particularly from my PR, I've mentioned in the issue earlier. Does it still passes? |
|
I will check that. However, it might be a while because I just completely messed up my test VM and have to restore it from a backup... 🤦 |
|
So |
|
Seems OK. @cachedout seems we are clean. Merge? |
|
@isbm Yup, I agree. This looks good. |
|
Looks like 2016.11.4 includes PR#39641 for 2016.3. I will see if this one is fixed in 2016.11 now. |
What does this PR do?
This PR changes how a runner returns return code that should be returned on the command-line. Before this change, it was returned in
ret['data']['retcode'], after the change it is returned inret['retcode']. The code insalt/cli/run.pywas changed in a way so that it can still deal with runners using the old format. The only runner in the core distribution (salt.orchestrate) is also changed by this PR so that it uses the new format.What issues does this PR fix or reference?
This PR fixed #39169. The problems described in #39169 happended when a runner (currently only
state.orchestrate) included a return code in theret['data']dict. Whenret['data']was later processed bycheck_state_result, the check failed becauseret['data']['retcode']is an integer and not a dict, but only dicts are expected as values inret['data'].Previous Behavior
salt-runexpected a runner to return an (optional) return code inret['data']['retcode'].state.orchestrateused this feature.New Behavior
salt-runexpectes a runner to return an (optional) return code inret['retcode'], but will fall back toret['data']['retcode']ifret['retcode']is not present.state.orchestrateuses the new format.Tests written?
No