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

Documentation inconsistency for minion ping_interval timing #44734

Closed
cruscio opened this issue Nov 29, 2017 · 4 comments
Labels
Milestone

Comments

@cruscio
Copy link

@cruscio cruscio commented Nov 29, 2017

Description of Issue/Question

The documentation for minion ping_interval indicates the value is defined in seconds, but it's applied as minutes in minion.py

Documentation

https://docs.saltstack.com/en/latest/ref/configuration/minion.html#ping-interval

Instructs the minion to ping its master(s) every n number of seconds. Used primarily as a mitigation technique against minion disconnects

Code

https://github.com/saltstack/salt/blob/2017.7.2/salt/minion.py#L2207

# schedule the stuff that runs every interval
ping_interval = self.opts.get(u'ping_interval', 0) * 60
if ping_interval > 0 and self.connected:
  def ping_master():
    try:
      def ping_timeout_handler(*_):
        if not self.opts.get(u'auth_safemode', True):
          log.error(u'** Master Ping failed. Attempting to restart minion**')
          delay = self.opts.get(u'random_reauth_delay', 5)
          log.info(u'delaying random_reauth_delay %ss', delay)
          # regular sys.exit raises an exception -- which isn't sufficient in a thread
          os._exit(salt.defaults.exitcodes.SALT_KEEPALIVE)

      self._fire_master('ping', 'minion_ping', sync=False, timeout_handler=ping_timeout_handler)
    except Exception:
      log.warning(u'Attempt to ping master failed.', exc_on_loglevel=logging.DEBUG)
  self.periodic_callbacks[u'ping'] = tornado.ioloop.PeriodicCallback(ping_master, ping_interval * 1000, io_loop=self.io_loop)

Looks like the code should be changed to not multiply ping_interval by 60

@gtmanfred gtmanfred added this to the Blocked milestone Nov 29, 2017
@gtmanfred

This comment has been minimized.

Copy link
Contributor

@gtmanfred gtmanfred commented Nov 29, 2017

@saltstack/team-core any opinions on if we should change the documentation, or fix the code so that the documentation is correct?

Thanks,
Daniel

@thatch45

This comment has been minimized.

Copy link
Member

@thatch45 thatch45 commented Nov 29, 2017

Change the docs to match the code, this code is 3 years old. No reason to pull the rug out from under people because we had a bad doc.

@gtmanfred gtmanfred modified the milestones: Blocked, Approved Nov 29, 2017
@gtmanfred

This comment has been minimized.

Copy link
Contributor

@gtmanfred gtmanfred commented Nov 29, 2017

@cruscio would you mind submitting a PR to correct this doc?

Thanks,
Daniel

@rallytime

This comment has been minimized.

Copy link
Contributor

@rallytime rallytime commented Dec 11, 2017

Closed via #44770

@rallytime rallytime closed this Dec 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.