[B] Set node attributes from device/all groups (ipspace#2745)

The device/all groups are not allowed to have static members. It was
thus impossible to set node attributes in a device/all group.

This change adds dynamic calculation of group members for device/all
groups. Making that work reliably required several other changes:

* The 'set node device/module' function has to apply group values in
  two passes. It has to apply group.device in the first pass and
  group.module in the second pass because changing node.device could
  change group membership for a device group.
* Computing members of a BGP autogroup now takes into account whether
  the global BGP module will be propagated into a node during the
  module initialization.
* The 'should we propagate modules' check was refactored into a
  separate function because it's used during module initialization as
  well as during BGP autogroup creation.
* node.device could be unspecified when BGP autogroup function calls
  the 'propagate_global_modules' check, but we know the only possible
  value that could be applied later on is the default device (because
  group-based devices were already set).

Author: ipspace

netsim/augment/groups.py

@@ -15,9 +15,21 @@
 from ..data import get_box
 from ..data.types import must_be_dict, must_be_id, must_be_list, transform_asdot
 from ..data.validate import get_object_attributes, validate_attributes
-from ..modules import bgp
+from ..modules import bgp, propagate_global_modules
 from ..utils import log
 
+'''
+Return computed members of special-purpose node group ('all' or device)
+'''
+def special_node_group_members(group: str, topology: Box) -> typing.Optional[list]:
+  if group == 'all':
+    return list(topology.nodes)
+
+  if group in topology.defaults.devices:
+    return [ node for node,ndata in topology.nodes.items() if ndata.get('device',None) == group ]
+
+  return None
+
 '''
 Return members of the specified group. Recurse through child groups if needed
 '''
@@ -37,10 +49,16 @@ def group_members(topology: Box, group: str, grp_type: str = 'node', count: int
       header=True)
 
   gdata = topology.groups[group]
-  if gdata.get('type','node') != grp_type:
+  g_type = gdata.get('type','node')
+  if g_type != grp_type:
     return []
 
-  for m in gdata.members:
+  if g_type == 'node':
+    sg_members = special_node_group_members(group,topology)
+    if not sg_members is None:
+      return sg_members
+
+  for m in gdata.get('members',[]):
     m_ns = grp_type + 's'
     if m in topology[m_ns]:
       members = members + [ m ]
@@ -236,6 +254,11 @@ def check_group_data_structure(
         more_hints='Device names or "all" cannot be used as names for user-defined groups',
         category=log.IncorrectValue,
         module='groups')
+    if grp in topology.defaults.devices and 'device' in gdata:
+      log.error(
+        text=f'Trying to change the device to {gdata.device} in device group {grp} is ridiculous',
+        category=log.IncorrectAttr,
+        module='groups')
 
     must_be_list(gdata,'module',gpath,create_empty=False,module='groups',valid_values=sorted(list_of_modules))
 
@@ -452,39 +475,64 @@ def reverse_topsort(topology: Box) -> list:
 Copy group-level module or device setting into node data
 '''
 def copy_group_device_module(topology: Box) -> None:
-  for grp in reverse_topsort(topology):
+  sorted_groups = reverse_topsort(topology)
+  for grp in sorted_groups:                                 # First, set the device type
     gdata = topology.groups[grp]
-    if not 'device' in gdata and not 'module' in gdata:
-      continue                                                        # This group is not interesting, move on
+    if not 'device' in gdata:
+      continue                                              # This group is not interesting, move on
 
     if log.debug_active('groups'):
-      print(f'Setting device/module for group {grp}')
+      print(f'Setting device for group {grp}: {gdata.device}')
     g_members = group_members(topology,grp)
     if not g_members and not gdata.get('_default_group',False):
       log.error(
-        f'Cannot use "module" or "device" attribute on in group {grp} that has no direct or indirect members',
+        f'Cannot use "device" attribute in group {grp} that has no direct or indirect members',
         log.IncorrectValue,
         'groups')
       continue
 
-    for name in g_members:                                            # Iterate over group members
-      if not name in topology.nodes:                                  # Member not a node? Move on...
+    for name in g_members:                                  # Iterate over group members
+      if not name in topology.nodes:                        # Member not a node? Weird, move on...
         continue
 
       ndata = topology.nodes[name]
-      if 'device' in gdata and not 'device' in ndata:                 # Copy device from group data to node data
+      if 'device' not in ndata:                             # Copy device from group data to node data
         ndata.device = gdata.device
         if log.debug_active('groups'):
           print(f'... setting {name}.device to {gdata.device}')
 
-      if 'module' in gdata:                                           # Merge group modules with device modules
-        ndata.module = ndata.module or []                             # Make sure node.module is a list
-        for m in gdata.module:                                        # Now iterate over group modules
-          if not m in ndata.module:                                   # ... and add missing modules to nodes
-            ndata.module.append(m)
+  for grp in sorted_groups:                                 # Next, augment device modules
+    gdata = topology.groups[grp]                            # We have to do this after augmenting devices
+                                                            # ... because modules could be specified on a device group
+    if 'module' not in gdata:                               # No module specified on this group
+      continue                                              # ... move on
 
-        if log.debug_active('groups'):
-          print(f'... adding module {gdata.module} to {name}. Node modules now {ndata.module}')
+    if log.debug_active('groups'):
+      print(f'Setting module for group {grp}: {gdata.module}')
+    g_members = group_members(topology,grp)
+    if not g_members and not gdata.get('_default_group',False):
+      log.error(
+        f'Cannot use "module" attribute in group {grp} that has no direct or indirect members',
+        log.IncorrectValue,
+        'groups')
+      continue
+
+    for name in g_members:                                  # Iterate over group members
+      if not name in topology.nodes:                        # Member not a node? Move on...
+        continue
+
+      ndata = topology.nodes[name]                          # Get node data
+      if 'module' not in ndata:                             # Group can set module(s) to empty, so we have to
+        ndata.module = []                                   # ... start with an empty list
+
+      if not isinstance(ndata.module,list):                 # We'll let someone else deal with incorrect
+        continue                                            # ... ndata.module data type
+
+      for m in gdata.module:                                # ... iterate over group modules
+        data.append_to_list(ndata,'module',m)               # ... and append group modules to node.module list
+
+      if log.debug_active('groups'):
+        print(f'... adding module {gdata.module} to {name}. Node modules now {ndata.module}')
 
 '''
 Copy node data from group into group members
@@ -587,7 +635,12 @@ def create_bgp_autogroups(topology: Box) -> None:
           'groups')
 
   for n_name,n_data in topology.nodes.items():                  # Now iterate over nodes
-    n_module = n_data.get('module',g_module)                    # Get node or global module (global modules haven't been propagated yet)
+    #
+    # Get node or global module (global modules haven't been propagated yet) taking into account
+    # whether the global modules will be propagated into the node
+    #
+    g_propagate = propagate_global_modules(n_data,topology)
+    n_module = n_data.get('module',g_module if g_propagate else [])
     if not isinstance(n_module,list):                           # Node list of modules is insane, someone else will complain
       continue
 

netsim/modules/__init__.py

@@ -141,6 +141,28 @@ def remove_module(node: Box, module: str, extra: list = []) -> None:
 
   node.module = [ m for m in node.module if m != module ]   # Remove module from the list of node modules
 
+"""
+Should we propagate global modules to a node?
+
+This function is called during module initialization, but also very early in the
+transformation process during BGP group creation, so we cannot assume that
+node.device is set
+"""
+def propagate_global_modules(n: Box, topology: Box) -> bool:
+  if 'device' not in n:                                     # Device might not be set yet
+    if 'device' not in topology.defaults:                   # Do we have a default device?
+      return False                                          # ... we'll have a problem anyway, so we can return whatever
+
+    n = n + {'device': topology.defaults.device}            # Assume default device
+
+  if n.device not in topology.defaults.devices:             # Have to do one more check for valid device type
+    return False                                            # ... because we cannot assume it has been checked
+
+  daemon   = devices.get_device_attribute(n,'daemon',topology.defaults) or False
+  is_host  = devices.get_device_attribute(n,'role',topology.defaults) == 'host' or n.get('role') == 'host'
+
+  return not is_host or daemon
+
 # Set default list of modules for nodes without specific module list
 #
 def augment_node_module(topology: Box) -> None:
@@ -160,9 +182,7 @@ def augment_node_module(topology: Box) -> None:
       #
       # non-host nodes are nodes that do not have 'role' set to 'host' or have 'daemon' set to True
       #
-      daemon   = devices.get_device_attribute(n,'daemon',topology.defaults)
-      is_host  = devices.get_device_attribute(n,'role',topology.defaults) == 'host' or n.get('role') == 'host'
-      if g_module and (not is_host or daemon):
+      if g_module and propagate_global_modules(n,topology):
         n.module = g_module
 
 # Check whether the modules defined on individual nodes are valid module names

tests/errors/group-device-no-members.log

@@ -1,2 +1,2 @@
-IncorrectValue in groups: Cannot use "module" or "device" attribute on in group g2 that has no direct or indirect members
+IncorrectValue in groups: Cannot use "device" attribute in group g2 that has no direct or indirect members
 Fatal error in netlab: Cannot proceed beyond this point due to errors, exiting

tests/topology/expected/groups-hierarchy.yml

@@ -11,6 +11,10 @@ groups:
   as65000:
     members:
     - e
+  eos:
+    members: []
+    module:
+    - dhcp
   g1:
     members:
     - a
@@ -62,6 +66,7 @@ links:
     ipv4: 172.16.0.0/24
   type: lan
 module:
+- dhcp
 - ospf
 - bgp
 name: input
@@ -157,7 +162,8 @@ nodes:
       ifname: Management1
       ipv4: 192.168.121.103
       mac: ca:fe:00:03:00:00
-    module: []
+    module:
+    - dhcp
     name: c
     role: router
   d:

tests/topology/input/groups-hierarchy.yml

@@ -30,6 +30,8 @@ groups:
     device: iosv
     ospf.area: 51
     bgp.as: 65000
+  eos:
+    module: [ dhcp ]
 
 links:
 - a-e-d