Fix failure with nested nodegroups:

If a nodegroup was re-used - even if it wasn't a circular reference - it
would fail the nodegroup expansion with the following message:

    Failed nodegroup expansion: illegal nested nodegroup "min"

This was simply because already-visited nodegroups are pushed into `skip`
but were never popped.

   * Add another test case for this situation.
   * Pop the current nodegroup from `skip` when leaving the expansion
     context

Fixes: #22797

salt/utils/minions.py

@@ -80,12 +80,11 @@ def nodegroup_comp(nodegroup, nodegroups, skip=None):
         log.error('Failed nodegroup expansion: illegal nested nodegroup "{0}"'.format(nodegroup))
         return ''
 
-    skip.add(nodegroup)
-
     if nodegroup not in nodegroups:
         log.error('Failed nodegroup expansion: unknown nodegroup "{0}"'.format(nodegroup))
         return ''
 
+    skip.add(nodegroup)
     nglookup = nodegroups[nodegroup]
 
     ret = []
@@ -94,11 +93,13 @@ def nodegroup_comp(nodegroup, nodegroups, skip=None):
     for match in tokens:
         if match in opers:
             ret.append(match)
-        elif len(match) >= 3 and match[:2] == 'N@':
+        elif len(match) >= 3 and match.startswith('N@'):
             ret.append(nodegroup_comp(match[2:], nodegroups, skip=skip))
         else:
             ret.append(match)
 
+    skip.remove(nodegroup)
+
     expanded = '( {0} )'.format(' '.join(ret)) if ret else ''
     log.debug('nodegroup_comp("{0}") => {1}'.format(nodegroup, expanded))
     return expanded

tests/integration/files/conf/master

@@ -63,3 +63,4 @@ nodegroups:
   min: minion
   sub_min: sub_minion
   mins: N@min or N@sub_min
+  redundant_minions: N@min or N@mins

tests/integration/shell/matcher.py

@@ -87,6 +87,10 @@ class MatchTest(integration.ShellCase, integration.ShellCaseCommonTestsMixIn):
         data = self.run_salt('-N unknown_nodegroup test.ping')
         self.assertFalse(minion_in_target('minion', data))
         self.assertFalse(minion_in_target('sub_minion', data))
+        time.sleep(2)
+        data = self.run_salt('-N redundant_minions test.ping')
+        self.assertTrue(minion_in_target('minion', data))
+        self.assertTrue(minion_in_target('sub_minion', data))
 
     def test_glob(self):
         '''