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.

Sign up for GitHub

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

Jump to bottom

Fix minion start retry on Windows (2016.3) #33285

Merged
merged 2 commits into from
May 17, 2016
Merged

Fix minion start retry on Windows (2016.3) #33285

merged 2 commits into from
May 17, 2016

Conversation

twangboy
Copy link
Contributor

What does this PR do?

Fixes problem where minion would try to connect to the master 7 times and then fail

What issues does this PR fix or reference?

#33277

Previous Behavior

Minion would try to connect to the master 7 times and then fail.

New Behavior

Minion continues to try to connect to the master.... forever...

Tests written?

No

@cachedout cachedout mentioned this pull request May 16, 2016
Closed
@twangboy twangboy changed the title Fix retry minion start for windows [WIP] Fix retry minion start for windows May 16, 2016
@skizunov
Copy link
Contributor

@twangboy : Looking at the code, I can't see how moving this code down:

minion = salt.cli.daemons.Minion()
minion.start()

Would change the behavior. Could you explain?

@skizunov
Copy link
Contributor

skizunov commented May 16, 2016 •
edited
Loading

@twangboy : Starting in 2016.3, there is a configuration way of fixing this issue. If you add this to the minion config file, it will work:

master_tries: -1

This is what my company does for this issue. It also works well when using multi-master since it will properly round-robin the various masters. One way of solving this issue is by making this the configuration default in salt/config/__init__.py. That way, if the user wanted it to fail after 1 try, they can get the old behavior back by changing this setting in their configuration.

@twangboy
Copy link
Contributor Author

@skizunov I implemented your fix. Thanks for the help!

@twangboy twangboy changed the title [WIP] Fix retry minion start for windows Fix minion start retry for windows (2016.3) May 17, 2016
@twangboy twangboy changed the title Fix minion start retry for windows (2016.3) Fix minion start retry on Windows (2016.3) May 17, 2016
@twangboy
Copy link
Contributor Author

@cachedout This can now be merged when tests pass if you're good with it

@cachedout
Copy link
Contributor

@twangboy Looking...

@cachedout cachedout merged commit 348c036 into saltstack:2016.3 May 17, 2016
gitebra pushed a commit to gitebra/salt that referenced this pull request May 18, 2016
@jojohans
Merge remote-tracking branch 'upstream/develop' into develop
383597b 
* upstream/develop: (23 commits)
  Don't merge-forward the change to salt/scripts.py
  Cleanup comments in smbios.get output (fixes saltstack#33266) (saltstack#33306)
  Back-port saltstack#32993 to 2016.3 (saltstack#33304)
  Fix iptables --match-set (saltstack#23643) (saltstack#33314)
  Improve package verification documentation
  Accept verify_options and ignore_types as a comma-separated list
  Fix minion start retry on Windows (2016.3) (saltstack#33285)
  update 2015.8.9 release notes (saltstack#33310)
  Added some more docs for master and minion config settings (saltstack#33292)
  linux_acl: Allow '-' as a separation character in ACL permissions. Fixes saltstack#31270 (saltstack#33172) (saltstack#33305)
  Server Density agentv2 support (saltstack#32772) (saltstack#33303)
  man pages updated (saltstack#33307)
  Fix iptables --match-set (saltstack#23643) (saltstack#33301)
  Handle more ipv6 error as an exception saltstack#33299 (saltstack#33300)
  Ignore retcode when checking service's status (saltstack#33294)
  fix "loose" typo (saltstack#33290)
  Fix minion start retry on Windows
  Add auth_tries config option to minion.rst docs (saltstack#33287)
  Document new master and minion config opts for 2016.3.0
  Document minion_id_caching config value (saltstack#33282)
  ...
@twangboy twangboy deleted the fix_33277_2016_3 branch August 30, 2016 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@twangboy @skizunov @cachedout