Rework minion return_retry_timer

This feature originally would randomize between 1 -> return_retry_timer, which meant you could always had the possibility of haing a return retry with a timeout of 1s. This change maxes both ends of the range configurable (similar to how acceptance_wait_time is configured). In addition I've added coverage in the minion configuration documentation.

Cleanup of #27286
Fixes #28577

doc/ref/configuration/minion.rst

@@ -145,7 +145,7 @@ the master hostname if name resolution fails. Defaults to 30 seconds.
 Set to zero if the minion should shutdown and not retry.
 
 .. code-block:: yaml
-    
+
     retry_dns: 30
 
 .. conf_minion:: master_port
@@ -458,6 +458,35 @@ behavior is to have time-frame within all minions try to reconnect.
 
     recon_randomize: True
 
+.. conf_minion:: return_retry_timer
+
+``return_retry_timer``
+-------------------
+
+Default: ``5``
+
+The default timeout for a minion return attempt.
+
+.. code-block:: yaml
+
+    return_retry_timer: 5
+
+
+.. conf_minion:: return_retry_timer_max
+
+``return_retry_timer_max``
+-------------------
+
+Default: ``10``
+
+The maximum timeout for a minion return attempt. If non-zero the minion return
+retry timeout will be a random int beween ``return_retry_timer`` and
+``return_retry_timer_max``
+
+.. code-block:: yaml
+
+    return_retry_timer_max: 10
+
 .. conf_minion:: cache_sreqs
 
 ``cache_sreqs``

salt/config.py

@@ -151,7 +151,7 @@ VALID_OPTS = {
     'recon_default': float,
     'recon_randomize': float,
     'return_retry_timer': int,
-    'return_retry_random': bool,
+    'return_retry_timer_max': int,
     'event_return': str,
     'event_return_queue': int,
     'event_return_whitelist': list,
@@ -415,8 +415,8 @@ DEFAULT_MINION_OPTS = {
     'recon_max': 10000,
     'recon_default': 1000,
     'recon_randomize': True,
-    'return_retry_timer': 4,
-    'return_retry_random': True,
+    'return_retry_timer': 5,
+    'return_retry_timer_max': 10,
     'syndic_log_file': os.path.join(salt.syspaths.LOGS_DIR, 'syndic'),
     'syndic_pidfile': os.path.join(salt.syspaths.PIDFILE_DIR, 'salt-syndic.pid'),
     'random_reauth_delay': 10,

salt/minion.py

@@ -908,20 +908,22 @@ class Minion(MinionBase):
         just return the value of the return_retry_timer.
         '''
         msg = 'Minion return retry timer set to {0} seconds'
-        if self.opts['return_retry_random']:
+        if self.opts['return_retry_timer_max']:
             try:
-                random_retry = randint(1, self.opts['return_retry_timer'])
+                random_retry = randint(self.opts['return_retry_timer'], self.opts['return_retry_timer_max'])
+                log.debug(msg.format(random_retry) + ' (randomized)')
+                return random_retry
             except ValueError:
                 # Catch wiseguys using negative integers here
                 log.error(
-                    'Invalid value ({0}) for return_retry_timer, must be a '
-                    'positive integer'.format(self.opts['return_retry_timer'])
+                    'Invalid value (return_retry_timer: {0} or return_retry_timer_max: {1})'
+                    'both must be a positive integers'.format(
+                        self.opts['return_retry_timer'],
+                        self.opts['return_retry_timer_max'],
+                    )
                 )
                 log.debug(msg.format(DEFAULT_MINION_OPTS['return_retry_timer']))
                 return DEFAULT_MINION_OPTS['return_retry_timer']
-            else:
-                log.debug(msg.format(random_retry) + ' (randomized)')
-                return random_retry
         else:
             log.debug(msg.format(self.opts['return_retry_timer']))
             return self.opts['return_retry_timer']