Merge pull request #22491 from plastikos/improvement-nested_nodegroups

improvement: Support nested nodegroups

doc/ref/cli/index.rst

@@ -136,8 +136,8 @@ shorthand for having to type out complicated compound expressions.
     nodegroups:
       group1: 'L@foo.domain.com,bar.domain.com,baz.domain.com and bl*.domain.com'
       group2: 'G@os:Debian and foo.domain.com'
+      group3: 'G@os:Debian and N@group1'
 
-More info on using nodegroups can be found :ref:`here <targeting-nodegroups>`.
 
 Calling the Function
 --------------------

doc/ref/configuration/master.rst

@@ -2286,6 +2286,9 @@ A group consists of a group name and a compound target.
     nodegroups:
       group1: 'L@foo.domain.com,bar.domain.com,baz.domain.com or bl*.domain.com'
       group2: 'G@os:Debian and foo.domain.com'
+      group3: 'G@os:Debian and N@group1'
+
+More information on using nodegroups can be found :ref:`here <targeting-nodegroups>`.
 
 
 Range Cluster Settings

doc/topics/targeting/nodegroups.rst

@@ -16,6 +16,7 @@ nodegroups. Here's an example nodegroup configuration within
     nodegroups:
       group1: 'L@foo.domain.com,bar.domain.com,baz.domain.com or bl*.domain.com'
       group2: 'G@os:Debian and foo.domain.com'
+      group3: 'G@os:Debian and N@group1'
 
 .. note::
 
@@ -23,6 +24,12 @@ nodegroups. Here's an example nodegroup configuration within
     group2 is matching specific grains. See the :doc:`compound matchers
     <compound>` documentation for more details.
 
+.. note::
+
+    Nodgroups can reference other nodegroups as seen in ``group3``.  Ensure
+    that you do not have circular references.  Circular references will be
+    detected and cause partial expansion with a logged error message.
+
 To match a nodegroup on the CLI, use the ``-N`` command-line option:
 
 .. code-block:: bash
@@ -45,4 +52,4 @@ nodegroup`` on the line directly following the nodegroup name.
     for those changes to be fully recognized.
 
     A limited amount of functionality, such as targeting with -N from the command-line may be
-    available without a restart.
+    available without a restart.

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