ios aggregate and common argument support (#28316)

*  ios aggregate spec validation
*  ios common argument for aggregate support

lib/ansible/modules/network/ios/ios_interface.py

@@ -101,6 +101,22 @@ EXAMPLES = """
     name: GigabitEthernet0/2
     enabled: False
     state: down
+
+- name: Add interface using aggregate
+  ios_interface:
+    aggregate:
+    - { name: GigabitEthernet0/1, mtu: 256, description: test-interface-1 }
+    - { name: GigabitEthernet0/2, mtu: 516, description: test-interface-2 }
+    duplex: full
+    speed: 100
+    state: present
+
+- name: Delete interface using aggregate
+  ios_interface:
+    aggregate:
+    - name: Loopback9
+    - name: Loopback10
+    state: absent
 """
 
 RETURN = """
@@ -116,6 +132,7 @@ commands:
 """
 import re
 
+from copy import deepcopy
 from time import sleep
 
 from ansible.module_utils._text import to_text
@@ -124,7 +141,7 @@ from ansible.module_utils.connection import exec_command
 from ansible.module_utils.ios import get_config, load_config
 from ansible.module_utils.ios import ios_argument_spec, check_args
 from ansible.module_utils.netcfg import NetworkConfig
-from ansible.module_utils.network_common import conditional
+from ansible.module_utils.network_common import conditional, remove_default_spec
 
 DEFAULT_DESCRIPTION = "configured by ios_interface"
 
@@ -202,45 +219,24 @@ def map_config_to_obj(module):
 
 def map_params_to_obj(module):
     obj = []
-    args = ['name', 'description', 'speed', 'duplex', 'mtu']
-
     aggregate = module.params.get('aggregate')
     if aggregate:
-        for param in aggregate:
+        for item in aggregate:
-            validate_param_values(module, args, param)
+            for key in item:
-            d = param.copy()
+                if item.get(key) is None:
+                    item[key] = module.params[key]
 
-            if 'name' not in d:
+            validate_param_values(module, item, item)
-                module.fail_json(msg="missing required arguments: %s" % 'name')
+            d = item.copy()
-
-            # set default value
-            for item in args:
-                if item not in d:
-                    if item == 'description':
-                        d['description'] = DEFAULT_DESCRIPTION
-                    else:
-                        d[item] = None
-                else:
-                    d[item] = str(d[item])
-
-            if not d.get('state'):
-                d['state'] = module.params['state']
-            if d.get('enabled') is None:
-                d['enabled'] = module.params['enabled']
 
             if d['enabled']:
                 d['disable'] = False
             else:
                 d['disable'] = True
 
-            if d.get('delay') is None:
-                d['delay'] = module.params['delay']
-
             obj.append(d)
 
     else:
-        validate_param_values(module, args)
-
         params = {
             'name': module.params['name'],
             'description': module.params['description'],
@@ -253,6 +249,7 @@ def map_params_to_obj(module):
             'rx_rate': module.params['rx_rate']
         }
 
+        validate_param_values(module, params)
         if module.params['enabled']:
             params.update({'disable': False})
         else:
@@ -361,7 +358,7 @@ def check_declarative_intent_params(module, want, result):
 def main():
     """ main entry point for module execution
     """
-    argument_spec = dict(
+    element_spec = dict(
         name=dict(),
         description=dict(default=DEFAULT_DESCRIPTION),
         speed=dict(),
@@ -371,11 +368,21 @@ def main():
         tx_rate=dict(),
         rx_rate=dict(),
         delay=dict(default=10, type='int'),
-        aggregate=dict(type='list'),
         state=dict(default='present',
                    choices=['present', 'absent', 'up', 'down'])
     )
 
+    aggregate_spec = deepcopy(element_spec)
+    aggregate_spec['name'] = dict(required=True)
+
+    # remove default in aggregate spec, to handle common arguments
+    remove_default_spec(aggregate_spec)
+
+    argument_spec = dict(
+        aggregate=dict(type='list', elements='dict', options=aggregate_spec),
+    )
+
+    argument_spec.update(element_spec)
     argument_spec.update(ios_argument_spec)
 
     required_one_of = [['name', 'aggregate']]

lib/ansible/modules/network/ios/ios_logging.py

@@ -54,10 +54,6 @@ options:
       - Set logging severity levels.
   aggregate:
     description: List of logging definitions.
-  purge:
-    description:
-      - Purge logging not defined in the aggregates parameter.
-    default: no
   state:
     description:
       - State of the logging configuration.
@@ -71,24 +67,41 @@ EXAMPLES = """
     dest: host
     name: 172.16.0.1
     state: present
+
 - name: remove host logging configuration
   ios_logging:
     dest: host
     name: 172.16.0.1
     state: absent
+
 - name: configure console logging level and facility
   ios_logging:
     dest: console
     facility: local7
     level: debugging
     state: present
+
 - name: enable logging to all
   ios_logging:
     dest : on
+
 - name: configure buffer size
   ios_logging:
     dest: buffered
     size: 5000
+
+- name: Configure logging using aggregate
+  ios_logging:
+    aggregate:
+      - { dest: console, level: notifications }
+      - { dest: buffered, size: 9000 }
+
+- name: remove logging using aggregate
+  ios_logging:
+    aggregate:
+      - { dest: console, level: notifications }
+      - { dest: buffered, size: 9000 }
+    state: absent
 """
 
 RETURN = """
@@ -103,7 +116,10 @@ commands:
 
 import re
 
+from copy import deepcopy
+
 from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.network_common import remove_default_spec
 from ansible.module_utils.ios import get_config, load_config
 from ansible.module_utils.ios import ios_argument_spec, check_args
 
@@ -119,7 +135,6 @@ def validate_size(value, module):
 def map_obj_to_commands(updates, module):
     commands = list()
     want, have = updates
-
     for w in want:
         dest = w['dest']
         name = w['name']
@@ -159,7 +174,6 @@ def map_obj_to_commands(updates, module):
                     dest_cmd += ' {}'.format(level)
 
                 commands.append(dest_cmd)
-
     return commands
 
 
@@ -247,22 +261,22 @@ def map_config_to_obj(module):
     return obj
 
 
-def map_params_to_obj(module):
+def map_params_to_obj(module, required_if=None):
     obj = []
+    aggregate = module.params.get('aggregate')
 
-    if 'aggregate' in module.params and module.params['aggregate']:
+    if aggregate:
-        for c in module.params['aggregate']:
+        for item in aggregate:
-            d = c.copy()
+            for key in item:
+                if item.get(key) is None:
+                    item[key] = module.params[key]
+
+            module._check_required_if(required_if, item)
+
+            d = item.copy()
             if d['dest'] != 'host':
                 d['name'] = None
 
-            if 'state' not in d:
-                d['state'] = module.params['state']
-            if 'facility' not in d:
-                d['facility'] = module.params['facility']
-            if 'level' not in d:
-                d['level'] = module.params['level']
-
             if d['dest'] == 'buffered':
                 if 'size' in d:
                     d['size'] = str(validate_size(d['size'], module))
@@ -312,17 +326,25 @@ def map_params_to_obj(module):
 def main():
     """ main entry point for module execution
     """
-    argument_spec = dict(
+    element_spec = dict(
         dest=dict(type='str', choices=['on', 'host', 'console', 'monitor', 'buffered']),
         name=dict(type='str'),
         size=dict(type='int'),
         facility=dict(type='str', default='local7'),
         level=dict(type='str', default='debugging'),
         state=dict(default='present', choices=['present', 'absent']),
-        aggregate=dict(type='list'),
-        purge=dict(default=False, type='bool')
     )
 
+    aggregate_spec = deepcopy(element_spec)
+
+    # remove default in aggregate spec, to handle common arguments
+    remove_default_spec(aggregate_spec)
+
+    argument_spec = dict(
+        aggregate=dict(type='list', elements='dict', options=aggregate_spec),
+    )
+
+    argument_spec.update(element_spec)
     argument_spec.update(ios_argument_spec)
 
     required_if = [('dest', 'host', ['name'])]
@@ -338,7 +360,7 @@ def main():
     if warnings:
         result['warnings'] = warnings
 
-    want = map_params_to_obj(module)
+    want = map_params_to_obj(module, required_if=required_if)
     have = map_config_to_obj(module)
 
     commands = map_obj_to_commands((want, have), module)

lib/ansible/modules/network/ios/ios_static_route.py

@@ -49,10 +49,6 @@ options:
     default: 1
   aggregate:
     description: List of static route definitions
-  purge:
-    description:
-      - Purge static routes not defined in the aggregates parameter.
-    default: no
   state:
     description:
       - State of the static route configuration.
@@ -74,11 +70,18 @@ EXAMPLES = """
     next_hop: 10.0.0.1
     state: absent
 
-- name: configure aggregates of static routes
+- name: Add static route aggregates
   ios_static_route:
     aggregate:
-      - { prefix: 192.168.2.0, mask 255.255.255.0, next_hop: 10.0.0.1 }
+      - { prefix: 172.16.32.0, mask: 255.255.255.0, next_hop: 10.0.0.8 }
-      - { prefix: 192.168.3.0, mask 255.255.255.0, next_hop: 10.0.2.1 }
+      - { prefix: 172.16.33.0, mask: 255.255.255.0, next_hop: 10.0.0.8 }
+
+- name: Add static route aggregates
+  ios_static_route:
+    aggregate:
+      - { prefix: 172.16.32.0, mask: 255.255.255.0, next_hop: 10.0.0.8 }
+      - { prefix: 172.16.33.0, mask: 255.255.255.0, next_hop: 10.0.0.8 }
+    state: absent
 """
 
 RETURN = """
@@ -89,10 +92,12 @@ commands:
   sample:
     - ip route 192.168.2.0 255.255.255.0 10.0.0.1
 """
+from copy import deepcopy
 
 from ansible.module_utils._text import to_text
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.connection import exec_command
+from ansible.module_utils.network_common import remove_default_spec
 from ansible.module_utils.ios import load_config, run_commands
 from ansible.module_utils.ios import ios_argument_spec, check_args
 from ipaddress import ip_network
@@ -143,32 +148,28 @@ def map_config_to_obj(module):
     return obj
 
 
-def map_params_to_obj(module):
+def map_params_to_obj(module, required_together=None):
     obj = []
 
-    if 'aggregate' in module.params and module.params['aggregate']:
+    aggregate = module.params.get('aggregate')
-        for c in module.params['aggregate']:
+    if aggregate:
-            d = c.copy()
+        for item in aggregate:
+            for key in item:
+                if item.get(key) is None:
+                    item[key] = module.params[key]
 
-            if 'state' not in d:
+            module._check_required_together(required_together, item)
-                d['state'] = module.params['state']
+            d = item.copy()
-            if 'admin_distance' not in d:
             d['admin_distance'] = str(module.params['admin_distance'])
 
             obj.append(d)
     else:
-        prefix = module.params['prefix'].strip()
-        mask = module.params['mask'].strip()
-        next_hop = module.params['next_hop'].strip()
-        admin_distance = str(module.params['admin_distance'])
-        state = module.params['state']
-
         obj.append({
-            'prefix': prefix,
+            'prefix': module.params['prefix'].strip(),
-            'mask': mask,
+            'mask': module.params['mask'].strip(),
-            'next_hop': next_hop,
+            'next_hop': module.params['next_hop'].strip(),
-            'admin_distance': admin_distance,
+            'admin_distance': str(module.params['admin_distance']),
-            'state': state
+            'state': module.params['state']
         })
 
     return obj
@@ -177,17 +178,27 @@ def map_params_to_obj(module):
 def main():
     """ main entry point for module execution
     """
-    argument_spec = dict(
+    element_spec = dict(
         prefix=dict(type='str'),
         mask=dict(type='str'),
         next_hop=dict(type='str'),
         admin_distance=dict(default=1, type='int'),
-        aggregate=dict(type='list'),
-        purge=dict(type='bool'),
         state=dict(default='present', choices=['present', 'absent'])
     )
 
+    aggregate_spec = deepcopy(element_spec)
+    aggregate_spec['prefix'] = dict(required=True)
+
+    # remove default in aggregate spec, to handle common arguments
+    remove_default_spec(aggregate_spec)
+
+    argument_spec = dict(
+        aggregate=dict(type='list', elements='dict', options=aggregate_spec),
+    )
+
+    argument_spec.update(element_spec)
     argument_spec.update(ios_argument_spec)
+
     required_one_of = [['aggregate', 'prefix']]
     required_together = [['prefix', 'mask', 'next_hop']]
     mutually_exclusive = [['aggregate', 'prefix']]
@@ -195,6 +206,7 @@ def main():
     module = AnsibleModule(argument_spec=argument_spec,
                            required_one_of=required_one_of,
                            required_together=required_together,
+                           mutually_exclusive=mutually_exclusive,
                            supports_check_mode=True)
 
     warnings = list()
@@ -203,7 +215,7 @@ def main():
     result = {'changed': False}
     if warnings:
         result['warnings'] = warnings
-    want = map_params_to_obj(module)
+    want = map_params_to_obj(module, required_together=required_together)
     have = map_config_to_obj(module)
 
     commands = map_obj_to_commands((want, have), module)

lib/ansible/modules/network/ios/ios_user.py

@@ -105,9 +105,11 @@ EXAMPLES = """
     name: ansible
     nopassword: True
     state: present
+
 - name: remove all users except admin
   ios_user:
     purge: yes
+
 - name: set multiple users to privilege level 15
   ios_user:
     aggregate:
@@ -115,17 +117,34 @@ EXAMPLES = """
       - name: netend
     privilege: 15
     state: present
+
 - name: set user view/role
   ios_user:
     name: netop
     view: network-operator
     state: present
+
 - name: Change Password for User netop
   ios_user:
     name: netop
     password: "{{ new_password }}"
     update_password: always
     state: present
+
+- name: Aggregate of users
+  ios_user:
+    aggregate:
+      - name: ansibletest2
+      - name: ansibletest3
+    view: network-admin
+
+- name: Delete users with aggregate
+  ios_user:
+    aggregate:
+      - name: ansibletest1
+      - name: ansibletest2
+      - name: ansibletest3
+    state: absent
 """
 
 RETURN = """
@@ -137,6 +156,7 @@ commands:
     - username ansible secret password
     - username admin secret admin
 """
+from copy import deepcopy
 
 import re
 import json
@@ -144,13 +164,14 @@ import json
 from functools import partial
 
 from ansible.module_utils.basic import AnsibleModule
+from ansible.module_utils.network_common import remove_default_spec
 from ansible.module_utils.ios import get_config, load_config
 from ansible.module_utils.six import iteritems
 from ansible.module_utils.ios import ios_argument_spec, check_args
 
 
 def validate_privilege(value, module):
-    if not 1 <= value <= 15:
+    if value and not 1 <= value <= 15:
         module.fail_json(msg='privilege must be between 1 and 15, got %s' % value)
 
 
@@ -306,8 +327,7 @@ def update_objects(want, have):
 def main():
     """ main entry point for module execution
     """
-    argument_spec = dict(
+    element_spec = dict(
-        aggregate=dict(type='list', aliases=['users', 'collection']),
         name=dict(),
 
         password=dict(no_log=True),
@@ -317,11 +337,22 @@ def main():
         privilege=dict(type='int'),
         view=dict(aliases=['role']),
 
-        purge=dict(type='bool', default=False),
         state=dict(default='present', choices=['present', 'absent'])
     )
+    aggregate_spec = deepcopy(element_spec)
+    aggregate_spec['name'] = dict(required=True)
 
+    # remove default in aggregate spec, to handle common arguments
+    remove_default_spec(aggregate_spec)
+
+    argument_spec = dict(
+        aggregate=dict(type='list', elements='dict', options=aggregate_spec, aliases=['users', 'collection']),
+        purge=dict(type='bool', default=False)
+    )
+
+    argument_spec.update(element_spec)
     argument_spec.update(ios_argument_spec)
+
     mutually_exclusive = [('name', 'aggregate')]
 
     module = AnsibleModule(argument_spec=argument_spec,

lib/ansible/modules/network/vyos/vyos_static_route.py

@@ -229,7 +229,6 @@ def main():
     argument_spec.update(element_spec)
     argument_spec.update(vyos_argument_spec)
 
-    argument_spec.update(vyos_argument_spec)
     required_one_of = [['aggregate', 'prefix']]
     required_together = [['prefix', 'next_hop']]
     mutually_exclusive = [['aggregate', 'prefix']]

test/integration/targets/ios_interface/tests/cli/basic.yaml

@@ -155,8 +155,10 @@
 - name: Add interface aggregate
   ios_interface:
     aggregate:
-    - { name: GigabitEthernet0/1, speed: 10, duplex: half, mtu: 256, description: test-interface-1 }
+    - { name: GigabitEthernet0/1, mtu: 256, description: test-interface-1 }
-    - { name: GigabitEthernet0/2, speed: 100, duplex: full, mtu: 516, description: test-interface-2 }
+    - { name: GigabitEthernet0/2, mtu: 516, description: test-interface-2 }
+    duplex: full
+    speed: 100
     state: present
     authorize: yes
     provider: "{{ cli }}"
@@ -166,9 +168,9 @@
     that:
       - 'result.changed == true'
       - '"interface GigabitEthernet0/1" in result.commands'
-      - '"speed 10" in result.commands'
+      - '"speed 100" in result.commands'
       - '"description test-interface-1" in result.commands'
-      - '"duplex half" in result.commands'
+      - '"duplex full" in result.commands'
       - '"mtu 256" in result.commands'
       - '"interface GigabitEthernet0/2" in result.commands'
       - '"speed 100" in result.commands'
@@ -179,8 +181,10 @@
 - name: Add interface aggregate (idempotent)
   ios_interface:
     aggregate:
-    - { name: GigabitEthernet0/1, speed: 10, duplex: half, mtu: 256, description: test-interface-1 }
+    - { name: GigabitEthernet0/1, mtu: 256, description: test-interface-1 }
-    - { name: GigabitEthernet0/2, speed: 100, duplex: full, mtu: 516, description: test-interface-2 }
+    - { name: GigabitEthernet0/2, mtu: 516, description: test-interface-2 }
+    duplex: full
+    speed: 100
     state: present
     authorize: yes
     provider: "{{ cli }}"
@@ -205,9 +209,9 @@
     that:
       - 'result.changed == true'
       - '"interface GigabitEthernet0/1" in result.commands'
-      - '"no speed 10" in result.commands'
+      - '"no speed 100" in result.commands'
       - '"description configured by ios_interface" in result.commands'
-      - '"no duplex half" in result.commands'
+      - '"no duplex full" in result.commands'
       - '"no mtu 256" in result.commands'
       - '"interface GigabitEthernet0/2" in result.commands'
       - '"no speed 100" in result.commands'
@@ -218,8 +222,9 @@
 - name: Disable interface aggregate
   ios_interface:
     aggregate:
-    - { name: GigabitEthernet0/1, enabled: False }
+    - name: GigabitEthernet0/1
-    - { name: GigabitEthernet0/2, enabled: False }
+    - name: GigabitEthernet0/2
+    enabled: False
     state: present
     authorize: yes
     provider: "{{ cli }}"
@@ -236,8 +241,9 @@
 - name: Enable interface aggregate
   ios_interface:
     aggregate:
-    - { name: GigabitEthernet0/1, enabled: True }
+    - name: GigabitEthernet0/1
-    - { name: GigabitEthernet0/2, enabled: True }
+    - name: GigabitEthernet0/2
+    enabled: True
     state: present
     authorize: yes
     provider: "{{ cli }}"
@@ -254,8 +260,8 @@
 - name: Create loopback interface aggregate
   ios_interface:
     aggregate:
-    - { name: Loopback9 }
+    - name: Loopback9
-    - { name: Loopback10 }
+    - name: Loopback10
     state: present
     authorize: yes
     provider: "{{ cli }}"
@@ -272,9 +278,9 @@
 - name: Delete loopback interface aggregate
   ios_interface:
     aggregate:
-    - { name: Loopback9, state: absent }
+    - name: Loopback9
-    - { name: Loopback10, state: absent }
+    - name: Loopback10
-    state: present
+    state: absent
     authorize: yes
     provider: "{{ cli }}"
   register: result
@@ -288,9 +294,9 @@
 - name: Delete loopback interface aggregate (idempotent)
   ios_interface:
     aggregate:
-    - { name: Loopback9, state: absent }
+    - name: Loopback9
-    - { name: Loopback10, state: absent }
+    - name: Loopback10
-    state: present
+    state: absent
     authorize: yes
     provider: "{{ cli }}"
   register: result

test/integration/targets/ios_logging/tests/cli/basic.yaml

@@ -107,11 +107,27 @@
       - 'result.changed == true'
       - '"logging buffered 8000" in result.commands'
 
+- name: Change logging parameters using aggregate
+  ios_logging:
+    aggregate:
+      - { dest: console, level: notifications }
+      - { dest: buffered, size: 9000 }
+    authorize: yes
+    provider: "{{ cli }}"
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == true'
+      - '"logging buffered 9000" in result.commands'
+      - '"logging console notifications" in result.commands'
+
 - name: remove logging as collection tearDown
   ios_logging:
     aggregate:
-      - { dest: console, level: warnings, state: absent }
+      - { dest: console, level: notifications }
-      - { dest: buffered, size: 8000, state: absent }
+      - { dest: buffered, size: 9000 }
+    state: absent
     authorize: yes
     provider: "{{ cli }}"
   register: result