Support nested nodegroups

Nodegroups can be recursively expanded without worrying about other
compound matcher expansions.  Once the nodegroups are expanded on the
master the remaining flattened compound match can be expanded without the
nodegroup information.

salt/utils/minions.py

@@ -69,31 +69,39 @@ def get_minion_data(minion, opts):
     return minion if minion else None, None, None
 
 
-def nodegroup_comp(group, nodegroups, skip=None):
+def nodegroup_comp(nodegroup, nodegroups, skip=None):
     '''
-    Take the nodegroup and the nodegroups and fill in nodegroup refs
+    Recursively expand ``nodegroup`` from ``nodegroups``; ignore nodegroups in ``skip``
     '''
-    k = 1
+
     if skip is None:
-        skip = set([group])
-        k = 0
-    if group not in nodegroups:
+        skip = set()
+    elif nodegroup in skip:
+        log.error('Failed nodegroup expansion: illegal nested nodegroup "{0}"'.format(nodegroup))
         return ''
-    gstr = nodegroups[group]
-    ret = ''
-    for comp in gstr.split(','):
-        if not comp.startswith('N@'):
-            ret += '{0} or '.format(comp)
-            continue
-        ngroup = comp[2:]
-        if ngroup in skip:
-            continue
-        skip.add(ngroup)
-        ret += nodegroup_comp(ngroup, nodegroups, skip)
-    if k == 1:
-        return ret
-    else:
-        return ret[:-3]
+
+    skip.add(nodegroup)
+
+    if nodegroup not in nodegroups:
+        log.error('Failed nodegroup expansion: unknown nodegroup "{0}"'.format(nodegroup))
+        return ''
+
+    nglookup = nodegroups[nodegroup]
+
+    ret = []
+    opers = ['and', 'or', 'not', '(', ')']
+    tokens = nglookup.split()
+    for match in tokens:
+        if match in opers:
+            ret.append(match)
+        elif len(match) >= 3 and match[:2] == 'N@':
+            ret.append(nodegroup_comp(match[2:], nodegroups, skip=skip))
+        else:
+            ret.append(match)
+
+    expanded = '( {0} )'.format(' '.join(ret)) if ret else ''
+    log.debug('nodegroup_comp("{0}") => {1}'.format(nodegroup, expanded))
+    return expanded
 
 
 class CkMinions(object):

tests/integration/files/conf/master

@@ -58,3 +58,8 @@ external_auth:
 
 master_tops:
   master_tops_test: True
+
+nodegroups:
+  min: minion
+  sub_min: sub_minion
+  mins: N@min or N@sub_min

tests/integration/shell/matcher.py

@@ -65,6 +65,29 @@ class MatchTest(integration.ShellCase, integration.ShellCaseCommonTestsMixIn):
         self.assertIn('sub_minion', data)
         self.assertNotIn('minion', data.replace('sub_minion', 'stub'))
 
+    def test_nodegroup(self):
+        '''
+        test salt nodegroup matcher
+        '''
+        def minion_in_target(minion, lines):
+            return sum([line == '{0}:'.format(minion) for line in lines])
+
+        data = self.run_salt('-N min test.ping')
+        self.assertTrue(minion_in_target('minion', data))
+        self.assertFalse(minion_in_target('sub_minion', data))
+        time.sleep(2)
+        data = self.run_salt('-N sub_min test.ping')
+        self.assertFalse(minion_in_target('minion', data))
+        self.assertTrue(minion_in_target('sub_minion', data))
+        time.sleep(2)
+        data = self.run_salt('-N mins test.ping')
+        self.assertTrue(minion_in_target('minion', data))
+        self.assertTrue(minion_in_target('sub_minion', data))
+        time.sleep(2)
+        data = self.run_salt('-N unknown_nodegroup test.ping')
+        self.assertFalse(minion_in_target('minion', data))
+        self.assertFalse(minion_in_target('sub_minion', data))
+
     def test_glob(self):
         '''
         test salt glob matcher