From 72ad0dd4ec68300b7e1f9a59c27966ff9106a6b8 Mon Sep 17 00:00:00 2001 From: Nilashish Chakraborty Date: Fri, 8 Jun 2018 05:35:47 +0530 Subject: [PATCH] Fix ios_logging idempotence CP into 2.5 (#41120) * Fixes to ios_logging (#41029) * Logging size may not show up in config * This is much simpler * Avoid repetition in tests * Both options of buffered are optional (cherry picked from commit 92a95368fe5246456a4bad9e133c930e3eff435f) * Added changelog --- changelogs/fragments/ios_logging_fix.yaml | 2 + .../modules/network/ios/ios_logging.py | 73 +++++++++++-------- .../targets/ios_logging/tests/cli/basic.yaml | 46 ++++++++---- 3 files changed, 75 insertions(+), 46 deletions(-) create mode 100644 changelogs/fragments/ios_logging_fix.yaml diff --git a/changelogs/fragments/ios_logging_fix.yaml b/changelogs/fragments/ios_logging_fix.yaml new file mode 100644 index 00000000000..e66428e6d98 --- /dev/null +++ b/changelogs/fragments/ios_logging_fix.yaml @@ -0,0 +1,2 @@ +bugfixes: +- Fix ios_logging issue (https://github.com/ansible/ansible/pull/41029) diff --git a/lib/ansible/modules/network/ios/ios_logging.py b/lib/ansible/modules/network/ios/ios_logging.py index 74a3bc193b8..3774be4654d 100644 --- a/lib/ansible/modules/network/ios/ios_logging.py +++ b/lib/ansible/modules/network/ios/ios_logging.py @@ -138,6 +138,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 +150,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 +191,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: @@ -205,18 +226,12 @@ def parse_size(line, dest): size = None if dest == 'buffered': - match = re.search(r'logging buffered (\S+)', line, re.M) + match = re.search(r'logging buffered(?: (\d+))?(?: [a-z]+)?', line, re.M) if match: - try: - int_size = int(match.group(1)) - except ValueError: - int_size = None - - if int_size: - if isinstance(int_size, int): - size = str(match.group(1)) - else: - size = str(4096) + if match.group(1) is not None: + size = match.group(1) + else: + size = "4096" return size @@ -241,15 +256,12 @@ def parse_level(line, dest): else: if dest == 'buffered': - match = re.search(r'logging buffered (?:\d+ )([a-z]+)', line, re.M) + match = re.search(r'logging buffered(?: \d+)?(?: ([a-z]+))?', line, re.M) else: match = re.search(r'logging {0} (\S+)'.format(dest), line, re.M) - if match: - if match.group(1) in level_group: - level = match.group(1) - else: - level = 'debugging' + if match and match.group(1) in level_group: + level = match.group(1) else: level = 'debugging' @@ -412,5 +424,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..7999c6a355a 100644 --- a/test/integration/targets/ios_logging/tests/cli/basic.yaml +++ b/test/integration/targets/ios_logging/tests/cli/basic.yaml @@ -1,7 +1,7 @@ --- # ensure logging configs are empty - name: Remove host logging - ios_logging: + ios_logging: &remove_host dest: host name: 172.16.0.1 state: absent @@ -45,16 +45,12 @@ provider: "{{ cli }}" register: result -- assert: +- assert: &unchanged that: - 'result.changed == false' - name: Delete/disable host logging - ios_logging: - dest: host - name: 172.16.0.1 - state: absent - provider: "{{ cli }}" + ios_logging: *remove_host register: result - assert: @@ -63,16 +59,10 @@ - '"no logging host 172.16.0.1" in result.commands' - name: Delete/disable host logging (idempotent) - ios_logging: - dest: host - name: 172.16.0.1 - state: absent - provider: "{{ cli }}" + ios_logging: *remove_host register: result -- assert: - that: - - 'result.changed == false' +- assert: *unchanged - name: Console logging with level warnings ios_logging: @@ -113,11 +103,34 @@ - '"logging buffered 9000" in result.commands' - '"logging console notifications" in result.commands' +- name: Set both logging destination and facility + ios_logging: &set_both + 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: *set_both + register: result + +- assert: *unchanged + - 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 +140,4 @@ - 'result.changed == true' - '"no logging console" in result.commands' - '"no logging buffered" in result.commands' + - '"no logging facility uucp" in result.commands'