Merge pull request #46713 from DSRCorporation/bugs/43941_grains_refre…

…sh_twice

Don't refresh modules twice per sync or refresh ops

salt/minion.py

@@ -2188,6 +2188,8 @@ def pillar_refresh(self, force_refresh=False):
         '''
         Refresh the pillar
         '''
+        self.module_refresh(force_refresh)
+
         if self.connected:
             log.debug('Refreshing pillar')
             async_pillar = salt.pillar.get_async_pillar(
@@ -2205,7 +2207,6 @@ def pillar_refresh(self, force_refresh=False):
                           'One or more masters may be down!')
             finally:
                 async_pillar.destroy()
-        self.module_refresh(force_refresh)
         self.matchers_refresh()
         self.beacons_refresh()
         evt = salt.utils.event.get_event('minion', opts=self.opts)

salt/modules/saltutil.py

@@ -383,9 +383,11 @@ def refresh_grains(**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:
+        # we don't need to call refresh_modules here because it's done by refresh_pillar
         refresh_pillar()
+    else:
+        refresh_modules()
     return True
 
 
@@ -425,7 +427,7 @@ def sync_grains(saltenv=None, refresh=True, extmod_whitelist=None, extmod_blackl
     '''
     ret = _sync('grains', saltenv, extmod_whitelist, extmod_blacklist)
     if refresh:
-        refresh_modules()
+        # we don't need to call refresh_modules here because it's done by refresh_pillar
         refresh_pillar()
     return ret
 
@@ -913,7 +915,7 @@ def sync_pillar(saltenv=None, refresh=True, extmod_whitelist=None, extmod_blackl
         )
     ret = _sync('pillar', saltenv, extmod_whitelist, extmod_blacklist)
     if refresh:
-        refresh_modules()
+        # we don't need to call refresh_modules here because it's done by refresh_pillar
         refresh_pillar()
     return ret
 
@@ -1026,7 +1028,7 @@ def sync_all(saltenv=None, refresh=True, extmod_whitelist=None, extmod_blacklist
     if __opts__['file_client'] == 'local':
         ret['pillar'] = sync_pillar(saltenv, False, extmod_whitelist, extmod_blacklist)
     if refresh:
-        refresh_modules()
+        # we don't need to call refresh_modules here because it's done by refresh_pillar
         refresh_pillar()
     return ret
 

tests/integration/modules/test_grains.py

@@ -10,10 +10,13 @@
 import time
 import pprint
 
+# Import Salt libs
+from salt.ext.six.moves import range
+
 # Import Salt Testing libs
 from tests.support.case import ModuleCase
-from tests.support.unit import skipIf
 from tests.support.helpers import flaky
+from tests.support.unit import skipIf
 
 log = logging.getLogger(__name__)
 
@@ -174,7 +177,8 @@ def test_grains_append_val_already_present(self):
               'salttesting-grain-key'.format(self.GRAIN_VAL)
 
         # First, make sure the test grain is present
-        self.run_function('grains.append', [self.GRAIN_KEY, self.GRAIN_VAL])
+        ret = self.run_function('grains.append', [self.GRAIN_KEY, self.GRAIN_VAL])
+        self.assertEqual(ret[self.GRAIN_KEY], [self.GRAIN_VAL])
 
         # Now try to append again
         ret = self.run_function('grains.append', [self.GRAIN_KEY, self.GRAIN_VAL])
@@ -226,3 +230,17 @@ def test_grains_append_call_twice(self):
                 pprint.pformat(append_2)
             )
         )
+
+    def test_grains_remove_add(self):
+        second_grain = self.GRAIN_VAL + '-2'
+        ret = self.run_function('grains.get', [self.GRAIN_KEY])
+        self.assertEqual(ret, [])
+
+        for i in range(10):
+            self.run_function('grains.setval', [self.GRAIN_KEY, []])
+            ret = self.run_function('grains.append', [self.GRAIN_KEY, self.GRAIN_VAL])
+            self.assertEqual(ret[self.GRAIN_KEY], [self.GRAIN_VAL])
+
+            self.run_function('grains.setval', [self.GRAIN_KEY, []])
+            ret = self.run_function('grains.append', [self.GRAIN_KEY, [self.GRAIN_VAL, second_grain]])
+            self.assertEqual(ret[self.GRAIN_KEY], [self.GRAIN_VAL, second_grain])

tests/integration/modules/test_saltutil.py

@@ -65,6 +65,12 @@ def test_wheel_with_kwarg(self):
         self.assertIn('priv', ret['return'])
 
 
+class SyncGrainsTest(ModuleCase):
+    def test_sync_grains(self):
+        ret = self.run_function('saltutil.sync_grains')
+        self.assertEqual(ret, [])
+
+
 class SaltUtilSyncModuleTest(ModuleCase):
     '''
     Testcase for the saltutil sync execution module