From a3178a1153b76a77b7cec5b561eb2ba570f56b1b Mon Sep 17 00:00:00 2001 From: Nilashish Chakraborty Date: Thu, 31 May 2018 20:07:24 +0530 Subject: [PATCH] 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 (cherry picked from commit a013cdc747f2892d6de19a5647d0efef216d19cb) --- .../modules/network/ios/ios_logging.py | 51 +++++++++++++------ .../targets/ios_logging/tests/cli/basic.yaml | 35 ++++++++++++- 2 files changed, 69 insertions(+), 17 deletions(-) diff --git a/lib/ansible/modules/network/ios/ios_logging.py b/lib/ansible/modules/network/ios/ios_logging.py index 74a3bc193b8..10923d77975 100644 --- a/lib/ansible/modules/network/ios/ios_logging.py +++ b/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 '12.' in os_version: - commands.append('no logging {0}'.format(name)) + if dest: + if dest == 'host': + 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)) - elif dest: - commands.append('no logging {0}'.format(dest)) - else: - module.fail_json(msg='dest must be among console, monitor, buffered, host, on') + 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': - commands.append('logging buffered {0} {1}'.format(size, level)) - else: - commands.append('logging buffered {0}'.format(size)) + present = False + + for entry in have: + 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() diff --git a/test/integration/targets/ios_logging/tests/cli/basic.yaml b/test/integration/targets/ios_logging/tests/cli/basic.yaml index 2ac13b1a2d6..ced1c65d8ba 100644 --- a/test/integration/targets/ios_logging/tests/cli/basic.yaml +++ b/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'