From eedb78a8ba88d11a8e2d6acf0e42f930912eb9f5 Mon Sep 17 00:00:00 2001 From: Nilashish Chakraborty Date: Thu, 31 May 2018 12:06:27 +0530 Subject: Fix eos_logging idempotency issue CP into 2.4 (#40933) * 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 (cherry picked from commit b9ea6468398d23c4b9c39f0c91cd79bf0b61af5d) * Added eos_logging fix in CHANGELOG --- CHANGELOG.md | 3 + lib/ansible/modules/network/eos/eos_logging.py | 74 ++++++++++++++++++---- .../targets/eos_logging/tests/cli/basic.yaml | 36 ++++++++++- 3 files changed, 98 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b8085c991..11e178d0b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ Ansible Changes By Release (https://github.com/ansible/ansible/pull/37964) * Fix Windows setup.ps1 for slow performance in large domain environments (https://github.com/ansible/ansible/pull/38646) +* Fix eos_logging idempotency issue when trying to set both logging destination & facility + (https://github.com/ansible/ansible/pull/40604) + diff --git a/lib/ansible/modules/network/eos/eos_logging.py b/lib/ansible/modules/network/eos/eos_logging.py index d769e45cc3..e895e3eb78 100644 --- a/lib/ansible/modules/network/eos/eos_logging.py +++ b/lib/ansible/modules/network/eos/eos_logging.py @@ -113,8 +113,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 import remove_default_spec from ansible.module_utils.eos import get_config, load_config @@ -149,19 +149,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)) @@ -170,7 +183,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) @@ -221,11 +255,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) @@ -239,19 +282,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 @@ -360,8 +405,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 @@ -376,5 +421,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 8a0b7fa156..2cd06cd6b9 100644 --- a/test/integration/targets/eos_logging/tests/cli/basic.yaml +++ b/test/integration/targets/eos_logging/tests/cli/basic.yaml @@ -67,6 +67,7 @@ - 'result.changed == true' - '"logging console warnings" in result.commands' + - name: Configure buffer size eos_logging: dest: buffered @@ -80,13 +81,45 @@ - '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 } authorize: yes provider: "{{ cli }}" + become: yes register: result - assert: @@ -94,3 +127,4 @@ - 'result.changed == true' - '"no logging console" in result.commands' - '"no logging buffered" in result.commands' + - '"no logging facility local7" in result.commands' -- cgit v1.2.1