Repository: ansible/ansible
Pull request: Fix baud rate and nxos_logging default case idempotency bug  
##### SUMMARY
- Fixes #43072 
- Added regex in nxos terminal plugin to detect baud rate error messages while configuring console logging with level > 2
- Added more code to detect monitor and console logging configurations with default levels, that doesn't show up in running-config
- Added integration tests for the above fixes 

##### ISSUE TYPE
 - Bugfix Pull Request

##### COMPONENT NAME
nxos_logging.py

##### ANSIBLE VERSION
```
devel
```


Problem statement:
nxos_logging not idempotent
<!---
Verify first that your issue/request is not already reported on GitHub.
THIS FORM WILL BE READ BY A MACHINE, COMPLETE ALL SECTIONS AS DESCRIBED.
Also test if the latest release, and devel branch are affected too.
ALWAYS add information AFTER (OUTSIDE) these html comments.
Otherwise it may end up being automatically closed by our bot. -->

##### SUMMARY
<!--- Explain the problem briefly -->
nxos_logging is not idempotent for "monitor level 5" and "console level 2". It seems to work for other levels just not these ones which are the device defaults.

Another issue is if you try to set console level 3-7, ansible returns changed but the command actually fails on the device. If you'd enter the command on the CLI the following error is returned: "Baud rate of console should be 38400 to increase severity level." 
So when trying to set the console level between 3-7 the module should first check that under "line console" speed is configured to be higher or equal to 38400.

##### ISSUE TYPE
 - Bug Report

##### COMPONENT NAME
<!--- Insert, BELOW THIS COMMENT, the name of the module, plugin, task or feature.
Do not include extra details here, e.g. "vyos_command" not "the network module vyos_command" or the full path-->
nxos_logging

##### ANSIBLE VERSION
<!--- Paste, BELOW THIS COMMENT, verbatim output from "ansible --version" between quotes below -->
```
ansible 2.7.0.dev0
ansible python module location = /usr/lib/python3.6/site-packages/ansible
executable location = /usr/bin/ansible
python version = 3.6.5 (default, Apr 10 2018, 17:08:37) [GCC 4.8.5 20150623 (Red Hat 4.8.5-16)]
```

##### CONFIGURATION
<!--- If using Ansible 2.4 or above, paste, BELOW THIS COMMENT, the results of "ansible-config dump --only-changed"
Otherwise, mention any settings you have changed/added/removed in ansible.cfg
(or using the ANSIBLE_* environment variables).-->

##### OS / ENVIRONMENT
<!--- Mention, BELOW THIS COMMENT, the OS you are running Ansible from, and the OS you are
managing, or say "N/A" for anything that is not platform-specific.
Also mention the specific version of what you are trying to control,
e.g. if this is a network bug the version of firmware on the network device.-->
Ansible Server: CentOS Linux release 7.5.1804
Device: N9K running NXOS: version 7.0(3)I6(1)

##### STEPS TO REPRODUCE
<!--- For bugs, show exactly how to reproduce the problem, using a minimal test-case.
For new features, show how the feature would be used. -->

<!--- Paste example playbooks or commands between quotes below -->
```yaml
- name: configure device
  hosts: sw
  connection: network_cli
  tasks:
    - nxos_logging:
        state: present
        dest: monitor
        dest_level: 5
    - nxos_logging:
        state: present
        dest: console
        dest_level: 2
```

<!--- You can also paste gist.github.com links for larger files -->

##### EXPECTED RESULTS
<!--- What did you expect to happen when running the steps above? -->
ok: [192.168.16.3] => {"changed": false, "commands": []}

##### ACTUAL RESULTS
<!--- What actually happened? If possible run with extra verbosity (-vvvv) -->
changed: [192.168.16.3] => {"changed": true, "commands": ["logging monitor 5"]}
changed: [192.168.16.3] => {"changed": true, "commands": ["logging console 2"]}

<!--- Paste verbatim command output between quotes below -->
```

```

Hints from issue discussion:
Files identified in the description:
* [lib/ansible/modules/network/nxos/nxos_logging.py](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/network/nxos/nxos_logging.py)

If these files are inaccurate, please update the `component name` section of the description or use the `!component` bot command.

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
cc @mikewiebe @privateip @rahushen @rcarrillocruz @trishnaguha @tstoner
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->

Commit message: Fix Shippable errors

Patch to review:
diff --git a/lib/ansible/modules/network/nxos/nxos_logging.py b/lib/ansible/modules/network/nxos/nxos_logging.py
index 1c805c45fd7124..6db04815cd8dda 100644
--- a/lib/ansible/modules/network/nxos/nxos_logging.py
+++ b/lib/ansible/modules/network/nxos/nxos_logging.py
@@ -114,8 +114,7 @@
 
 import re
 
-
-from ansible.module_utils.network.nxos.nxos import get_config, load_config
+from ansible.module_utils.network.nxos.nxos import get_config, load_config, run_commands
 from ansible.module_utils.network.nxos.nxos import nxos_argument_spec, check_args
 from ansible.module_utils.basic import AnsibleModule
 
@@ -286,6 +285,30 @@ def map_config_to_obj(module):
                         'dest_level': parse_dest_level(line, dest, parse_name(line, dest)),
                         'facility_level': parse_facility_level(line, facility)})
 
+    cmd_1 = [{'command': 'show logging | section enabled | section console', 'output': 'text'}]
+    cmd_2 = [{'command': 'show logging | section enabled | section monitor', 'output': 'text'}]
+
+    default_data = run_commands(module, cmd_1)
+    default_data.append(run_commands(module, cmd_2))
+
+    for line in default_data:
+        flag = False
+        match = re.search(r'Logging (\w+):(?:\s+) (?:\w+) (?:\W)Severity: (\w+)', str(line), re.M)
+        if match:
+            if match.group(1) == 'console' and match.group(2) == 'critical':
+                dest_level = '2'
+                flag = True
+            elif match.group(1) == 'monitor' and match.group(2) == 'notifications':
+                dest_level = '5'
+                flag = True
+        if flag:
+            obj.append({'dest': match.group(1),
+                        'remote_server': None,
+                        'name': None,
+                        'facility': None,
+                        'dest_level': dest_level,
+                        'facility_level': None})
+
     return obj
 
 
diff --git a/lib/ansible/plugins/terminal/nxos.py b/lib/ansible/plugins/terminal/nxos.py
index 2db3dfdd456c5e..1972342f1f28a8 100644
--- a/lib/ansible/plugins/terminal/nxos.py
+++ b/lib/ansible/plugins/terminal/nxos.py
@@ -47,7 +47,8 @@ class TerminalModule(TerminalBase):
         re.compile(br"syntax error"),
         re.compile(br"unknown command"),
         re.compile(br"user not present"),
-        re.compile(br"invalid (.+?)at '\^' marker", re.I)
+        re.compile(br"invalid (.+?)at '\^' marker", re.I),
+        re.compile(br"baud rate of console should be (\d*) to increase severity level", re.I)
     ]
 
     def on_become(self, passwd=None):
diff --git a/test/integration/targets/nxos_logging/tests/common/basic.yaml b/test/integration/targets/nxos_logging/tests/common/basic.yaml
index de0d8995e41520..28f79af184356b 100644
--- a/test/integration/targets/nxos_logging/tests/common/basic.yaml
+++ b/test/integration/targets/nxos_logging/tests/common/basic.yaml
@@ -24,6 +24,60 @@
     that:
       - 'result.changed == false'
 
+- name: Set up console logging with level 2 (edge case)
+  nxos_logging: &clog2
+    dest: console
+    dest_level: 2
+    provider: "{{ connection }}"
+    state: present
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == true'
+      - '"logging console 2" in result.commands'
+
+- name: Set up console logging with level 2 (edge case) (idempotent)
+  nxos_logging: *clog2
+  register: result
+
+- assert: *false
+
+- name: Set Baud Rate to less than 38400
+  nxos_config:
+    lines:
+      - speed 9600
+    parents: line console
+    provider: "{{ connection }}"
+
+- name: Enable console logging with level 3 (will fail)
+  nxos_logging: &con3
+    dest: console
+    dest_level: 3
+  register: result
+  provider: "{{ connection }}"
+  ignore_errors: yes
+
+- assert:
+    that:
+      - 'result.failed == true'
+
+- name: Set Baud Rate to 38400
+  nxos_config:
+    lines:
+      - speed 38400
+    parents: line console
+    provider: "{{ connection }}"
+
+- name: Enable console logging with level 3 (will pass)
+  nxos_logging: *con3
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == true'
+      - '"logging console 3" in result.commands'
+
 - name: Logfile logging with level
   nxos_logging: &llog
     dest: logfile
@@ -80,6 +134,24 @@
 
 - assert: *false
 
+- name: Configure monitor with level 5 (edge case)
+  nxos_logging: &mlog5
+    dest: monitor
+    dest_level: 5
+    provider: "{{ connection }}"
+  register: result
+
+- assert:
+    that:
+      - 'result.changed == true'
+      - '"logging monitor 5" in result.commands'
+
+- name: Configure monitor with level 5 (edge case) (idempotent)
+  nxos_logging: *mlog5
+  register: result
+
+- assert: *false
+
 - name: Configure facility with level
   nxos_logging: &flog
     facility: daemon
@@ -122,9 +194,9 @@
 - name: remove logging as collection tearDown
   nxos_logging: &agg
     aggregate:
-      - { dest: console, dest_level: 0 }
+      - { dest: console, dest_level: 3 }
       - { dest: module, dest_level: 2 }
-      - { dest: monitor, dest_level: 3 }
+      - { dest: monitor, dest_level: 5 }
       - { dest: logfile, dest_level: 1, name: test }
       - { facility: daemon, facility_level: 4 }
       - { dest: server, remote_server: test-syslogserver.com, facility: auth, dest_level: 1 }


Code review comments:
diff_hunk: @@ -286,6 +285,30 @@ def map_config_to_obj(module):
                         'dest_level': parse_dest_level(line, dest, parse_name(line, dest)),
                         'facility_level': parse_facility_level(line, facility)})
 
+    cmd_1 = [{'command': 'show logging | section enabled | section console', 'output': 'text'}]
path: lib/ansible/modules/network/nxos/nxos_logging.py
text: @user1:
Why not just make this one call?

e.g.:

```
cmds = [{'command': <...>}, {'command': <...>}]
default_data = run_commands(module, cmds)
```