Scrap event-based approach for refreshing grains

The call to refresh_modules() in saltutil.refresh_grains was resulting
in a race condition with the event I added to refresh the grains. This
meant that, even though self.opts['grains'] was being changed by the new
event, before that process could finish the module refresh was itself
re-loading the grains and re-packing these new grains into the
__grains__ dunder, negating the refresh.

Since the module refresh loads the grains anyway, this commit changes
saltutil.refresh_grains to refresh the modules. It also removes the
refresh argument recently added to the grains module functions, as we're
no longer using it to conditionally refresh modules.

salt/minion.py

@@ -1695,15 +1695,6 @@ class Minion(MinionBase):
             tagify([self.opts['id'], 'start'], 'minion'),
         )
 
-    def grains_refresh_manual(self):
-        '''
-        Perform a manual grains refresh
-        '''
-        self.opts['grains'] = salt.loader.grains(
-            self.opts,
-            force_refresh=True,
-            proxy=getattr(self, 'proxy', None))
-
     def module_refresh(self, force_refresh=False, notify=False):
         '''
         Refresh the functions and returners.
@@ -1873,12 +1864,9 @@ class Minion(MinionBase):
         elif package.startswith('manage_beacons'):
             self.manage_beacons(tag, data)
         elif package.startswith('grains_refresh'):
-            if package == 'grains_refresh_manual':
-                self.grains_refresh_manual()
             if self.grains_cache != self.opts['grains']:
                 self.grains_cache = self.opts['grains']
-                if package != 'grains_refresh_manual':
-                    self.pillar_refresh(force_refresh=True)
+                self.pillar_refresh(force_refresh=True)
         elif package.startswith('environ_setenv'):
             self.environ_setenv(tag, data)
         elif package.startswith('_minion_mine'):

salt/modules/grains.py

@@ -192,7 +192,7 @@ def item(*args, **kwargs):
     return ret
 
 
-def setvals(grains, destructive=False, refresh=True):
+def setvals(grains, destructive=False):
     '''
     Set new grains values in the grains config file
 
@@ -200,10 +200,6 @@ def setvals(grains, destructive=False, refresh=True):
         If an operation results in a key being removed, delete the key, too.
         Defaults to False.
 
-    refresh
-        Refresh modules and pillar after adding the new grains.
-        Defaults to True.
-
     CLI Example:
 
     .. code-block:: bash
@@ -279,12 +275,12 @@ def setvals(grains, destructive=False, refresh=True):
         log.error(msg.format(fn_))
     if not __opts__.get('local', False):
         # Refresh the grains
-        __salt__['saltutil.refresh_grains'](refresh=refresh)
+        __salt__['saltutil.refresh_grains']()
     # Return the grains we just set to confirm everything was OK
     return new_grains
 
 
-def setval(key, val, destructive=False, refresh=True):
+def setval(key, val, destructive=False):
     '''
     Set a grains value in the grains config file
 
@@ -298,10 +294,6 @@ def setval(key, val, destructive=False, refresh=True):
         If an operation results in a key being removed, delete the key, too.
         Defaults to False.
 
-    refresh
-        Refresh modules and pillar after adding the new grain.
-        Defaults to True.
-
     CLI Example:
 
     .. code-block:: bash
@@ -309,7 +301,7 @@ def setval(key, val, destructive=False, refresh=True):
         salt '*' grains.setval key val
         salt '*' grains.setval key "{'sub-key': 'val', 'sub-key2': 'val2'}"
     '''
-    return setvals({key: val}, destructive, refresh)
+    return setvals({key: val}, destructive)
 
 
 def append(key, val, convert=False, delimiter=DEFAULT_TARGET_DELIM):
@@ -412,7 +404,7 @@ def remove(key, val, delimiter=DEFAULT_TARGET_DELIM):
     return setval(key, grains)
 
 
-def delval(key, destructive=False, refresh=True):
+def delval(key, destructive=False):
     '''
     .. versionadded:: 0.17.0
 
@@ -424,9 +416,6 @@ def delval(key, destructive=False, refresh=True):
     destructive
         Delete the key, too. Defaults to False.
 
-    refresh
-        Refresh modules and pillar after removing the grain.
-
     CLI Example:
 
     .. code-block:: bash
@@ -434,7 +423,7 @@ def delval(key, destructive=False, refresh=True):
         salt '*' grains.delval key
     '''
 
-    setval(key, None, destructive=destructive, refresh=refresh)
+    setval(key, None, destructive=destructive)
 
 
 def ls():  # pylint: disable=C0103

salt/modules/saltutil.py

@@ -310,16 +310,19 @@ def sync_states(saltenv=None, refresh=True):
     return ret
 
 
-def refresh_grains(refresh=True):
+def refresh_grains(**kwargs):
     '''
     .. versionadded:: 2016.3.6,2016.11.4,Nitrogen
 
     Refresh the minion's grains without syncing custom grains modules from
     ``salt://_grains``.
 
-    refresh : True
-        If ``True``, refresh the available execution modules and recompile
-        pillar data for the minion. Set to ``False`` to prevent this refresh.
+    .. note::
+        The available execution modules will be reloaded as part of this
+        proceess, as grains can affect which modules are available.
+
+    refresh_pillar : True
+        Set to ``False`` to keep pillar data from being refreshed.
 
     CLI Examples:
 
@@ -327,15 +330,17 @@ def refresh_grains(refresh=True):
 
         salt '*' saltutil.refresh_grains
     '''
-    try:
-        ret = __salt__['event.fire']({}, 'grains_refresh_manual')
-    except Exception:
-        log.error('Failed to refresh grains', exc_info=True)
-        return False
-    if refresh:
-        refresh_modules()
+    kwargs = salt.utils.clean_kwargs(**kwargs)
+    _refresh_pillar = kwargs.pop('refresh_pillar', True)
+    if kwargs:
+        salt.utils.invalid_kwargs(kwargs)
+    # Modules and pillar need to be refreshed in case grains changes affected
+    # them, and the module refresh process reloads the grains and assigns the
+    # newly-reloaded grains to each execution module's __grains__ dunder.
+    refresh_modules()
+    if _refresh_pillar:
         refresh_pillar()
-    return ret
+    return True
 
 
 def sync_grains(saltenv=None, refresh=True):