Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi-command salt calls return highest exit #47544

Merged
merged 2 commits into from Jun 1, 2018

Conversation

Projects
None yet
5 participants
@KaiSforza
Copy link
Contributor

commented May 8, 2018

What does this PR do?

Running a multiple command salt call will currently make salt think that
all of the minions returned with a failed state regardless of the return
values of the modules actually run. Right now it just returns the value
of retcode in the return dictionary if the return value is a
dictionary.

This checks if the retcode value is a dictionary itself, and then
returns the maximum return code that we recieved. This means that we
will continue to fail if the jobs did fail, but will return a zero if
all the jobs did so.

What issues does this PR fix or reference?

#18510

Previous Behavior

The salt summary would say that any multi-module salt call had failed, because it did not know how to parse returns with a dictionary retcode.

New Behavior

Salt now handles dictionary retcode values by taking the highest return value from multi-module call and using that to determine if it failed.

Tests written?

No

Commits signed with GPG?

Yes

@KaiSforza KaiSforza requested a review from saltstack/team-core as a code owner May 8, 2018

@salt-jenkins salt-jenkins requested a review from saltstack/team-suse May 8, 2018

@KaiSforza KaiSforza force-pushed the KaiSforza:multicommandexit branch from b92ab60 to c1b5cd6 May 8, 2018

@KaiSforza

This comment has been minimized.

Copy link
Contributor Author

commented May 8, 2018

Updated the default value from the .get() to be a dict so it actually has .values()

@KaiSforza KaiSforza force-pushed the KaiSforza:multicommandexit branch from c1b5cd6 to a8d6b8d May 8, 2018

@KaiSforza

This comment has been minimized.

Copy link
Contributor Author

commented May 8, 2018

Fixed code lint check failure

18:16:33 salt/cli/salt.py:405: [C0325(superfluous-parens), ] Unnecessary parens after u'return' keyword

@isbm

isbm approved these changes May 9, 2018

Copy link
Contributor

left a comment

@rallytime the code looks good dealing with the different exit code types and structures 😉 but it would be still good to run a tests over it.

Multi-command salt calls return highest exit
Running a multiple command salt call will currently make salt think that
all of the minions returned with a failed state regardless of the return
values of the modules actually run. Right now it just returns the value
of `retcode` in the return dictionary if the return value is a
dictionary.

This checks if the `retcode` value is a dictionary itself, and then
returns the maximum return code that we recieved. This means that we
will continue to fail if the jobs did fail, but will return a zero if
all the jobs did so.

@KaiSforza KaiSforza force-pushed the KaiSforza:multicommandexit branch from a8d6b8d to 9d1a077 May 11, 2018

@KaiSforza

This comment has been minimized.

Copy link
Contributor Author

commented May 11, 2018

Okay it looks like the tests are failing outside of my changes.

@rallytime rallytime requested a review from terminalmage May 22, 2018

@terminalmage terminalmage merged commit 7fb420c into saltstack:develop Jun 1, 2018

5 of 10 checks passed

jenkins/PR/salt-pr-linode-cent7-py3 Pull Requests » Salt PR - Linode CentOS 7 - PY3 #5134 — ABORTED
Details
jenkins/PR/salt-pr-linode-ubuntu14-n Pull Requests » Salt PR - Linode Ubuntu14.04 #23067 — ABORTED
Details
jenkins/PR/salt-pr-linode-ubuntu16-py3 Pull Requests » Salt PR - Linode Ubuntu16.04 - PY3 #10104 — ABORTED
Details
jenkins/PR/salt-pr-rs-cent7-n Pull Requests » Salt PR - RS CentOS 7 #19190 — ABORTED
Details
default Build finished.
Details
WIP ready for review
Details
codeclimate 2 fixed issues
Details
jenkins/PR/salt-pr-clone Pull Requests » Salt PR - Clone #25324 — SUCCESS
Details
jenkins/PR/salt-pr-docs-n Pull Requests » Salt PR - Docs #17415 — SUCCESS
Details
jenkins/PR/salt-pr-lint-n Pull Requests » Salt PR - Code Lint #22045 — SUCCESS
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.