Fixes ios_logging idempotency issues (#40701)

* Fixes ios_logging idempotency issues

* Added intergration tests & minor fixes

* Minor fixes in tests

* Minor fixes in tests #2

* eos_logging fixes after PR review

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

@@ -121,7 +121,6 @@ commands:
 import re
 
 from copy import deepcopy
-
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.network.common.utils import remove_default_spec, validate_ip_address
 from ansible.module_utils.network.ios.ios import get_config, load_config
@@ -138,6 +137,7 @@ def validate_size(value, module):
 
 
 def map_obj_to_commands(updates, module, os_version):
+    dest_group = ('console', 'monitor', 'buffered', 'on')
     commands = list()
     want, have = updates
     for w in want:
@@ -149,23 +149,36 @@ def map_obj_to_commands(updates, module, os_version):
         state = w['state']
         del w['state']
 
+        if facility:
+            w['dest'] = 'facility'
+
         if state == 'absent' and w in have:
-            if dest == 'host':
+            if dest:
-                if '12.' in os_version:
+                if dest == 'host':
-                    commands.append('no logging {0}'.format(name))
+                    if '12.' in os_version:
+                        commands.append('no logging {0}'.format(name))
+                    else:
+                        commands.append('no logging host {0}'.format(name))
+
+                elif dest in dest_group:
+                    commands.append('no logging {0}'.format(dest))
+
                 else:
-                    commands.append('no logging host {0}'.format(name))
+                    module.fail_json(msg='dest must be among console, monitor, buffered, host, on')
-            elif dest:
-                commands.append('no logging {0}'.format(dest))
-            else:
-                module.fail_json(msg='dest must be among console, monitor, buffered, host, on')
 
             if facility:
                 commands.append('no logging facility {0}'.format(facility))
 
         if state == 'present' and w not in have:
             if facility:
-                commands.append('logging facility {0}'.format(facility))
+                present = False
+
+                for entry in have:
+                    if entry['dest'] == 'facility' and entry['facility'] == facility:
+                        present = True
+
+                if not present:
+                    commands.append('logging facility {0}'.format(facility))
 
             if dest == 'host':
                 if '12.' in os_version:
@@ -177,10 +190,17 @@ def map_obj_to_commands(updates, module, os_version):
                 commands.append('logging on')
 
             elif dest == 'buffered' and size:
-                if level and level != 'debugging':
+                present = False
-                    commands.append('logging buffered {0} {1}'.format(size, level))
+
-                else:
+                for entry in have:
-                    commands.append('logging buffered {0}'.format(size))
+                    if entry['dest'] == 'buffered' and entry['size'] == size and entry['level'] == level:
+                        present = True
+
+                if not present:
+                    if level and level != 'debugging':
+                        commands.append('logging buffered {0} {1}'.format(size, level))
+                    else:
+                        commands.append('logging buffered {0}'.format(size))
 
             else:
                 if dest:
@@ -293,7 +313,6 @@ def map_config_to_obj(module):
                         'facility': parse_facility(line, dest),
                         'level': parse_level(line, dest)
                     })
-
     return obj
 
 
@@ -355,7 +374,6 @@ def map_params_to_obj(module, required_if=None):
                 'level': module.params['level'],
                 'state': module.params['state']
             })
-
     return obj
 
 
@@ -412,5 +430,6 @@ def main():
 
     module.exit_json(**result)
 
+
 if __name__ == '__main__':
     main()

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

@@ -99,6 +99,7 @@
       - 'result.changed == true'
       - '"logging buffered 8000" in result.commands'
 
+
 - name: Change logging parameters using aggregate
   ios_logging:
     aggregate:
@@ -113,11 +114,42 @@
       - '"logging buffered 9000" in result.commands'
       - '"logging console notifications" in result.commands'
 
+- name: Set both logging destination and facility
+  ios_logging:
+    dest: buffered
+    facility: uucp
+    level: alerts
+    size: 4096
+    state: present
+    provider: "{{ cli }}"
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == true'
+      - '"logging buffered 4096 alerts" in result.commands'
+      - '"logging facility uucp" in result.commands'
+
+- name: Set both logging destination and facility (idempotent)
+  ios_logging:
+    dest: buffered
+    facility: uucp
+    level: alerts
+    size: 4096
+    state: present
+    provider: "{{ cli }}"
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == false'
+
 - name: remove logging as collection tearDown
   ios_logging:
     aggregate:
       - { dest: console, level: notifications }
-      - { dest: buffered, size: 9000 }
+      - { dest: buffered, size: 4096, level: alerts }
+      - { facility: uucp }
     state: absent
     provider: "{{ cli }}"
   register: result
@@ -127,3 +159,4 @@
       - 'result.changed == true'
       - '"no logging console" in result.commands'
       - '"no logging buffered" in result.commands'
+      - '"no logging facility uucp" in result.commands'