iosxr modules aggregate check and other minor refactor (#28360)

*  iosxr aggregate validation
*  vyos_user, junos_user minor changes
*  Aggregated documentation

lib/ansible/modules/network/iosxr/iosxr_interface.py

@@ -87,6 +87,33 @@ EXAMPLES = """
   iosxr_interface:
     name: GigabitEthernet0/0/0/2
     enabled: False
+
+- name: Create interface using aggregate
+  iosxr_interface:
+    aggregate:
+    - name: GigabitEthernet0/0/0/3
+    - name: GigabitEthernet0/0/0/2
+    state: present
+
+- name: Delete interface using aggregate
+  iosxr_interface:
+    aggregate:
+    - name: GigabitEthernet0/0/0/3
+    - name: GigabitEthernet0/0/0/2
+    state: absent
+
+- name: Check intent arguments
+  iosxr_interface:
+    name: GigabitEthernet0/0/0/5
+    state: up
+    delay: 20
+
+- name: Config + intent
+  iosxr_interface:
+    name: GigabitEthernet0/0/0/5
+    enabled: False
+    state: down
+    delay: 20
 """
 
 RETURN = """
@@ -103,13 +130,14 @@ commands:
 import re
 
 from time import sleep
+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.iosxr import get_config, load_config
 from ansible.module_utils.iosxr import iosxr_argument_spec, check_args
-from ansible.module_utils.network_common import conditional
+from ansible.module_utils.network_common import conditional, remove_default_spec
 
 DEFAULT_DESCRIPTION = "configured by iosxr_interface"
 
@@ -158,37 +186,19 @@ def map_params_to_obj(module):
 
     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:
@@ -342,7 +352,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(),
@@ -352,11 +362,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(iosxr_argument_spec)
 
     required_one_of = [['name', 'aggregate']]

lib/ansible/modules/network/iosxr/iosxr_logging.py

@@ -46,10 +46,6 @@ options:
     default: debugging
   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.
@@ -63,24 +59,41 @@ EXAMPLES = """
     dest: hostnameprefix
     name: 172.16.0.1
     state: present
+
 - name: remove hostnameprefix logging configuration
   iosxr_logging:
     dest: hostnameprefix
     name: 172.16.0.1
     state: absent
+
 - name: configure console logging level and facility
   iosxr_logging:
     dest: console
     facility: local7
     level: debugging
     state: present
+
 - name: enable logging to all
   iosxr_logging:
     dest : on
+
 - name: configure buffer size
   iosxr_logging:
     dest: buffered
     size: 5000
+
+- name: Configure logging using aggregate
+  iosxr_logging:
+    aggregate:
+      - { dest: console, level: warning }
+      - { dest: buffered, size: 4800000 }
+
+- name: Delete logging using aggregate
+  iosxr_logging:
+    aggregate:
+      - { dest: console, level: warning }
+      - { dest: buffered, size: 4800000 }
+    state: absent
 """
 
 RETURN = """
@@ -95,14 +108,17 @@ commands:
 
 import re
 
+from copy import deepcopy
+
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.iosxr import get_config, load_config
 from ansible.module_utils.iosxr import iosxr_argument_spec, check_args
+from ansible.module_utils.network_common import remove_default_spec
 
 
 def validate_size(value, module):
     if value:
-        if not int(307200) <= value <= int(125000000):
+        if value and not int(307200) <= value <= int(125000000):
             module.fail_json(msg='size must be between 307200 and 125000000')
         else:
             return value
@@ -238,22 +254,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 = []
 
-    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]
+
+            module._check_required_if(required_if, item)
+            d = item.copy()
+
             if d['dest'] != 'hostnameprefix':
                 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))
@@ -303,17 +319,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', 'hostnameprefix', '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(iosxr_argument_spec)
 
     required_if = [('dest', 'hostnameprefix', ['name'])]
@@ -327,7 +351,7 @@ def main():
 
     result = {'changed': False}
 
-    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/iosxr/iosxr_user.py

@@ -110,10 +110,12 @@ commands:
     - username ansible secret password group sysadmin
     - username admin secret admin
 """
-
 from functools import partial
 
+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.iosxr import get_config, load_config
 from ansible.module_utils.iosxr import iosxr_argument_spec, check_args
 
@@ -243,19 +245,27 @@ def map_params_to_obj(module):
 def main():
     """ main entry point for module execution
     """
-    argument_spec = dict(
+    element_spec = dict(
-        aggregate=dict(type='list', aliases=['users', 'collection']),
         name=dict(),
 
         configured_password=dict(no_log=True),
         update_password=dict(default='always', choices=['on_create', 'always']),
 
         group=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(iosxr_argument_spec)
     mutually_exclusive = [('name', 'aggregate')]
 

lib/ansible/modules/network/junos/junos_user.py

@@ -218,6 +218,8 @@ def map_params_to_obj(module):
     if not aggregate:
         if not module.params['name'] and module.params['purge']:
             return list()
+        elif not module.params['name']:
+            module.fail_json(msg='missing required argument: name')
         else:
             collection = [{'name': module.params['name']}]
     else:
@@ -225,6 +227,8 @@ def map_params_to_obj(module):
         for item in aggregate:
             if not isinstance(item, dict):
                 collection.append({'username': item})
+            elif 'name' not in item:
+                module.fail_json(msg='missing required argument: name')
             else:
                 collection.append(item)
 
@@ -277,14 +281,10 @@ def main():
     argument_spec.update(element_spec)
     argument_spec.update(junos_argument_spec)
 
-    required_one_of = [['aggregate', 'name']]
     mutually_exclusive = [['aggregate', 'name']]
 
-    argument_spec.update(junos_argument_spec)
-
     module = AnsibleModule(argument_spec=argument_spec,
                            mutually_exclusive=mutually_exclusive,
-                           required_one_of=required_one_of,
                            supports_check_mode=True)
 
     warnings = list()

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

@@ -278,7 +278,6 @@ def main():
         configured_password=dict(no_log=True),
         update_password=dict(default='always', choices=['on_create', 'always']),
 
-        purge=dict(type='bool', default=False),
         state=dict(default='present', choices=['present', 'absent'])
     )
 
@@ -290,6 +289,7 @@ def main():
 
     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)

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

@@ -173,8 +173,8 @@
 - name: Change interface parameters in aggregate
   iosxr_interface:
     aggregate:
-    - { name: GigabitEthernet0/0/0/3 }
+    - name: GigabitEthernet0/0/0/3
-    - { name: GigabitEthernet0/0/0/2 }
+    - name: GigabitEthernet0/0/0/2
     state: present
     provider: "{{ cli }}"
   register: result
@@ -209,8 +209,9 @@
 - name: Enable interface aggregate
   iosxr_interface:
     aggregate:
-    - { name: GigabitEthernet0/0/0/3, enabled: True }
+    - name: GigabitEthernet0/0/0/3
-    - { name: GigabitEthernet0/0/0/2, enabled: True }
+    - name: GigabitEthernet0/0/0/2
+    enabled: True
     state: present
     provider: "{{ cli }}"
   register: result
@@ -221,11 +222,20 @@
       - '"no interface GigabitEthernet0/0/0/3 shutdown" in result.commands'
       - '"no interface GigabitEthernet0/0/0/2 shutdown" in result.commands'
 
+- name: interface aggregate (setup)
+  iosxr_interface:
+    aggregate:
+    - name: GigabitEthernet0/0/0/4
+    - name: GigabitEthernet0/0/0/5
+    description: test-interface-initial
+    provider: "{{ cli }}"
+  register: result
+
 - name: Create interface aggregate
   iosxr_interface:
     aggregate:
-    - { name: GigabitEthernet0/0/0/4 }
+    - name: GigabitEthernet0/0/0/4
-    - { name: GigabitEthernet0/0/0/5 }
+    - name: GigabitEthernet0/0/0/5
     state: present
     provider: "{{ cli }}"
   register: result
@@ -239,9 +249,9 @@
 - name: Delete interface aggregate
   iosxr_interface:
     aggregate:
-    - { name: GigabitEthernet0/0/0/4, state: absent }
+    - name: GigabitEthernet0/0/0/4
-    - { name: GigabitEthernet0/0/0/5, state: absent }
+    - name: GigabitEthernet0/0/0/5
-    state: present
+    state: absent
     provider: "{{ cli }}"
   register: result
 
@@ -254,9 +264,9 @@
 - name: Delete interface aggregate (idempotent)
   iosxr_interface:
     aggregate:
-    - { name: GigabitEthernet0/0/0/4, state: absent }
+    - name: GigabitEthernet0/0/0/4
-    - { name: GigabitEthernet0/0/0/5, state: absent }
+    - name: GigabitEthernet0/0/0/5
-    state: present
+    state: absent
     provider: "{{ cli }}"
   register: result
 

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

@@ -100,11 +100,26 @@
       - 'result.changed == true'
       - '"logging buffered 4800000" in result.commands'
 
+- name: Change logging parameters using aggregate
+  iosxr_logging:
+    aggregate:
+      - { dest: console, level: notifications }
+      - { dest: buffered, size: 4700000 }
+    provider: "{{ cli }}"
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == true'
+      - '"logging buffered 4700000" in result.commands'
+      - '"logging console notifications" in result.commands'
+
 - name: remove logging as collection tearDown
   iosxr_logging:
     aggregate:
-      - { dest: console, level: warning, state: absent }
+      - { dest: console, level: notifications }
-      - { dest: buffered, size: 4800000, state: absent }
+      - { dest: buffered, size: 4700000 }
+    state: absent
     provider: "{{ cli }}"
   register: result