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

Back-port #48193 to 2017.7 #48295

Merged
merged 3 commits into from Jun 26, 2018

Conversation

Projects
None yet
4 participants
@rallytime
Contributor

rallytime commented Jun 25, 2018

Back-port #48193 to 2017.7

Refs #42659

jacksontj added some commits Jun 18, 2018

Properly wait on returns in saltnado
This was broken because the behavior was to simply check the ckminions
and wait for only those returns to complete. This works assuming
ckminions is accurate (which there are many cases where it isn't, such
as syndics).

_disbatch_local's waiting on returns needs to match LocalClient's
behavior (namely that in get_iter_returns). This means we are allowed to
return when (1) we have waitged the min_wait_time (0 if not a syndic)
(2) no minions are running the job (3) all minions we saw running it are
done running the job. The only method allowed for earlier termination is
if the gather_job_timeout is exceeded.

Fixes #42659
Properly configure syndic in test case
Without this option we aren't really a syndic and it won't wait

@rallytime rallytime requested review from gtmanfred and DmitryKuzmenko Jun 25, 2018

@salt-jenkins salt-jenkins requested a review from saltstack/team-netapi Jun 25, 2018

@DmitryKuzmenko

Sorry my fault. I've missed this in the original PR. =(

# minimum time required for return to complete. By default no waiting, if
# we are a syndic then we must wait syndic_wait at a minimum
min_wait_time = Future().set_result(True)

This comment has been minimized.

@DmitryKuzmenko

DmitryKuzmenko Jun 25, 2018

Contributor

This shall be:

min_wait_time = Future()
min_wait_time.set_result(True)

The same is in the original PR.

This comment has been minimized.

@DmitryKuzmenko

DmitryKuzmenko Jun 25, 2018

Contributor

It's the reason of the tests failures.

This comment has been minimized.

@DmitryKuzmenko

DmitryKuzmenko Jun 25, 2018

Contributor

@jacksontj could you please confirm my point?

@rallytime rallytime requested a review from DmitryKuzmenko Jun 25, 2018

@rallytime rallytime merged commit 21ed5b9 into saltstack:2017.7 Jun 26, 2018

6 of 9 checks passed

default Build finished.
Details
jenkins/PR/salt-pr-linode-ubuntu16-py3 Pull Requests » Salt PR - Linode Ubuntu16.04 - PY3 #10982 — FAILURE
Details
jenkins/PR/salt-pr-rs-cent7-n Pull Requests » Salt PR - RS CentOS 7 #20065 — FAILURE
Details
WIP ready for review
Details
jenkins/PR/salt-pr-clone Pull Requests » Salt PR - Clone #26216 — SUCCESS
Details
jenkins/PR/salt-pr-docs-n Pull Requests » Salt PR - Docs #18264 — SUCCESS
Details
jenkins/PR/salt-pr-linode-cent7-py3 Pull Requests » Salt PR - Linode CentOS 7 - PY3 #6012 — SUCCESS
Details
jenkins/PR/salt-pr-linode-ubuntu14-n Pull Requests » Salt PR - Linode Ubuntu14.04 #23940 — SUCCESS
Details
jenkins/PR/salt-pr-lint-n Pull Requests » Salt PR - Code Lint #22899 — SUCCESS
Details

@rallytime rallytime deleted the rallytime:bp-48193 branch Jun 26, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment