From b9ea6468398d23c4b9c39f0c91cd79bf0b61af5d Mon Sep 17 00:00:00 2001 From: Nilashish Chakraborty Date: Wed, 30 May 2018 18:21:54 +0530 Subject: [PATCH] Fixes eos_logging idempotence issue #31862 (#40604) * eos_logging idempotence fix * fixed eos_logging idempotence issue * Fixed pylint and pep8 errors * Added tests for eos_logging & minor fix * Removed q statements --- .../modules/network/eos/eos_logging.py | 74 +++++++++++++++---- .../targets/eos_logging/tests/cli/basic.yaml | 35 ++++++++- 2 files changed, 94 insertions(+), 15 deletions(-) diff --git a/lib/ansible/modules/network/eos/eos_logging.py b/lib/ansible/modules/network/eos/eos_logging.py index ec5e525f17..79ee748ae9 100644 --- a/lib/ansible/modules/network/eos/eos_logging.py +++ b/lib/ansible/modules/network/eos/eos_logging.py @@ -114,8 +114,8 @@ commands: import re -from copy import deepcopy +from copy import deepcopy from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.network.common.utils import remove_default_spec from ansible.module_utils.network.eos.eos import get_config, load_config @@ -150,19 +150,32 @@ def map_obj_to_commands(updates, module): del w['state'] if state == 'absent' and w in have: - if dest == 'host': - commands.append('no logging host {}'.format(name)) - elif dest: - commands.append('no logging {}'.format(dest)) - else: - module.fail_json(msg='dest must be among console, monitor, buffered, host, on') + if dest: + if dest == 'host': + commands.append('no logging host {}'.format(name)) + + elif dest in DEST_GROUP: + commands.append('no logging {}'.format(dest)) + + else: + module.fail_json(msg='dest must be among console, monitor, buffered, host, on') if facility: commands.append('no logging facility {}'.format(facility)) if state == 'present' and w not in have: if facility: - commands.append('logging facility {}'.format(facility)) + present = False + + # Iterate over every dictionary in the 'have' list to check if + # similar configuration for facility exists or not + + for entry in have: + if not entry['dest'] and entry['facility'] == facility: + present = True + + if not present: + commands.append('logging facility {}'.format(facility)) if dest == 'host': commands.append('logging host {}'.format(name)) @@ -171,7 +184,28 @@ def map_obj_to_commands(updates, module): commands.append('logging on') elif dest == 'buffered' and size: - commands.append('logging buffered {}'.format(size)) + + present = False + + # Deals with the following two cases: + # Case 1: logging buffered + # logging buffered + # + # Case 2: Same buffered logging configuration + # already exists (i.e., both size & + # level are same) + + for entry in have: + if entry['dest'] == 'buffered' and entry['size'] == size: + + if not level or entry['level'] == level: + present = True + + if not present: + if size and level: + commands.append('logging buffered {} {}'.format(size, level)) + else: + commands.append('logging buffered {}'.format(size)) else: dest_cmd = 'logging {}'.format(dest) @@ -222,11 +256,20 @@ def parse_name(line, dest): return name -def parse_level(line, dest, module): +def parse_level(line, dest): level = None if dest is not 'host': - match = re.search(r'logging {} (\S+)'.format(dest), line, re.M) + + # Line for buffer logging entry in running-config is of the form: + # logging buffered + + if dest == 'buffered': + match = re.search(r'logging buffered (?:\d+) (\S+)', line, re.M) + + else: + match = re.search(r'logging {} (\S+)'.format(dest), line, re.M) + if match: if match.group(1) in LEVEL_GROUP: level = match.group(1) @@ -240,19 +283,21 @@ def map_config_to_obj(module): data = get_config(module, flags=['section logging']) for line in data.split('\n'): + match = re.search(r'logging (\S+)', line, re.M) if match: if match.group(1) in DEST_GROUP: dest = match.group(1) + else: - pass + dest = None obj.append({'dest': dest, 'name': parse_name(line, dest), 'size': parse_size(line, dest), 'facility': parse_facility(line), - 'level': parse_level(line, dest, module)}) + 'level': parse_level(line, dest)}) return obj @@ -361,8 +406,8 @@ def main(): if warnings: result['warnings'] = warnings - want = map_params_to_obj(module, required_if=required_if) have = map_config_to_obj(module) + want = map_params_to_obj(module, required_if=required_if) commands = map_obj_to_commands((want, have), module) result['commands'] = commands @@ -377,5 +422,6 @@ def main(): module.exit_json(**result) + if __name__ == '__main__': main() diff --git a/test/integration/targets/eos_logging/tests/cli/basic.yaml b/test/integration/targets/eos_logging/tests/cli/basic.yaml index 302cc55464..638a9d5e8a 100644 --- a/test/integration/targets/eos_logging/tests/cli/basic.yaml +++ b/test/integration/targets/eos_logging/tests/cli/basic.yaml @@ -64,6 +64,7 @@ - 'result.changed == true' - '"logging console warnings" in result.commands' + - name: Configure buffer size eos_logging: dest: buffered @@ -76,11 +77,42 @@ - 'result.changed == true' - '"logging buffered 480000" in result.commands' +- name: Set up logging destination and facility at the same time + eos_logging: + dest: buffered + size: 4096 + facility: local7 + level: informational + state: present + become: yes + register: result + +- assert: + that: + - 'result.changed == true' + - '"logging buffered 4096 informational" in result.commands' + - '"logging facility local7" in result.commands' + +- name: Set up logging destination and facility at the same time again (idempotent) + eos_logging: + dest: buffered + size: 4096 + facility: local7 + level: informational + state: present + become: yes + register: result + +- assert: + that: + - 'result.changed == false' + - name: remove logging as collection tearDown eos_logging: aggregate: - { dest: console, level: warnings, state: absent } - - { dest: buffered, size: 480000, state: absent } + - { dest: buffered, level: informational, size: 4096, state: absent } + - { facility: local7, state: absent } become: yes register: result @@ -89,3 +121,4 @@ - 'result.changed == true' - '"no logging console" in result.commands' - '"no logging buffered" in result.commands' + - '"no logging facility local7" in result.commands'