fix nxos_udld issues (#37418)

2 files changed, 76 insertions, 63 deletions

diff --git a/lib/ansible/modules/network/nxos/nxos_udld.py b/lib/ansible/modules/network/nxos/nxos_udld.py
index c0a4536588..4486a5b70e 100644
--- a/lib/ansible/modules/network/nxos/nxos_udld.py
+++ b/lib/ansible/modules/network/nxos/nxos_udld.py
@@ -34,8 +34,6 @@ author:
     - Jason Edelman (@jedelman8)
 notes:
     - Tested against NXOSv 7.3.(0)D1(1) on VIRL
-    - When C(state=absent), it unconfigures existing settings C(msg_time) and set it
-      to its default value of 15.  It is cleaner to always use C(state=present).
     - Module will fail if the udld feature has not been previously enabled.
 options:
     aggressive:
@@ -46,18 +44,20 @@ options:
         choices: ['enabled','disabled']
     msg_time:
         description:
-            - Message time in seconds for UDLD packets.
+            - Message time in seconds for UDLD packets or keyword 'default'.
         required: false
         default: null
     reset:
         description:
-            - Ability to reset UDLD down interfaces.
+            - Ability to reset all ports shut down by UDLD. 'state' parameter
+              cannot be 'absent' when this is present.
         required: false
         default: null
-        choices: ['true','false']
+        choices: ['true']
     state:
         description:
-            - Manage the state of the resource.
+            - Manage the state of the resource. When set to 'absent',
+              aggressive and msg_time are set to their default values.
         required: false
         default: present
         choices: ['present','absent']
@@ -117,6 +117,11 @@ from ansible.module_utils.network.nxos.nxos import get_capabilities, nxos_argume
 from ansible.module_utils.basic import AnsibleModule
 
 
+PARAM_TO_DEFAULT_KEYMAP = {
+    'msg_time': '15',
+}
+
+
 def execute_show_command(command, module, command_type='cli_show'):
     device_info = get_capabilities(module)
     network_api = device_info.get('network_api', 'nxapi')
@@ -156,42 +161,32 @@ def apply_key_map(key_map, table):
     return new_dict
 
 
-def get_commands_config_udld_global(delta, reset):
-    config_args = {
-        'enabled': 'udld aggressive',
-        'disabled': 'no udld aggressive',
-        'msg_time': 'udld message-time {msg_time}'
-    }
+def get_commands_config_udld_global(delta, reset, existing):
     commands = []
     for param, value in delta.items():
         if param == 'aggressive':
-            if value == 'enabled':
-                command = 'udld aggressive'
-            elif value == 'disabled':
-                command = 'no udld aggressive'
-        else:
-            command = config_args.get(param, 'DNE').format(**delta)
-        if command and command != 'DNE':
+            command = 'udld aggressive' if value == 'enabled' else 'no udld aggressive'
             commands.append(command)
-        command = None
-
+        elif param == 'msg_time':
+            if value == 'default':
+                if existing.get('msg_time') != PARAM_TO_DEFAULT_KEYMAP.get('msg_time'):
+                    commands.append('no udld message-time')
+            else:
+                commands.append('udld message-time ' + value)
     if reset:
         command = 'udld reset'
         commands.append(command)
     return commands
 
 
-def get_commands_remove_udld_global(delta):
-    config_args = {
-        'aggressive': 'no udld aggressive',
-        'msg_time': 'no udld message-time {msg_time}',
-    }
+def get_commands_remove_udld_global(existing):
     commands = []
-    for param, value in delta.items():
-        command = config_args.get(param, 'DNE').format(**delta)
-        if command and command != 'DNE':
-            commands.append(command)
-        command = None
+    if existing.get('aggressive') == 'enabled':
+        command = 'no udld aggressive'
+        commands.append(command)
+    if existing.get('msg_time') != PARAM_TO_DEFAULT_KEYMAP.get('msg_time'):
+        command = 'no udld message-time'
+        commands.append(command)
     return commands
 
 
@@ -222,7 +217,6 @@ def main():
     argument_spec.update(nxos_argument_spec)
 
     module = AnsibleModule(argument_spec=argument_spec,
-                           required_one_of=[['aggressive', 'msg_time', 'reset']],
                            supports_check_mode=True)
 
     warnings = list()
@@ -232,20 +226,8 @@ def main():
     reset = module.params['reset']
     state = module.params['state']
 
-    if (aggressive or reset) and state == 'absent':
-        module.fail_json(msg="It's better to use state=present when "
-                             "configuring or unconfiguring aggressive mode "
-                             "or using reset flag. state=absent is just for "
-                             "when using msg_time param.")
-
-    if msg_time:
-        try:
-            msg_time_int = int(msg_time)
-            if msg_time_int < 7 or msg_time_int > 90:
-                raise ValueError
-        except ValueError:
-            module.fail_json(msg='msg_time must be an integer'
-                                 'between 7 and 90')
+    if reset and state == 'absent':
+        module.fail_json(msg="state must be present when using reset flag.")
 
     args = dict(aggressive=aggressive, msg_time=msg_time, reset=reset)
     proposed = dict((k, v) for k, v in args.items() if v is not None)
@@ -259,13 +241,12 @@ def main():
     commands = []
     if state == 'present':
         if delta:
-            command = get_commands_config_udld_global(dict(delta), reset)
+            command = get_commands_config_udld_global(dict(delta), reset, existing)
             commands.append(command)
 
     elif state == 'absent':
-        common = set(proposed.items()).intersection(existing.items())
-        if common:
-            command = get_commands_remove_udld_global(dict(common))
+        command = get_commands_remove_udld_global(existing)
+        if command:
             commands.append(command)
 
     cmds = flatten_list(commands)
diff --git a/test/integration/targets/nxos_udld/tests/common/sanity.yaml b/test/integration/targets/nxos_udld/tests/common/sanity.yaml
index 4bf7243a1c..21944765fd 100644
--- a/test/integration/targets/nxos_udld/tests/common/sanity.yaml
+++ b/test/integration/targets/nxos_udld/tests/common/sanity.yaml
@@ -5,7 +5,7 @@
 
 - set_fact: udld_run="true"
 - set_fact: udld_run="false"
-  when: (( platform is search('N9K-F')) and (imagetag and ( imagetag is version('F3', 'lt'))))
+  when: ((platform is search('N9K-F')) and (imagetag and (imagetag is version_compare('F3', 'lt'))))
 - set_fact: udld_run="false"
   when: titanium
 
@@ -16,14 +16,9 @@
       state: enabled
       provider: "{{ connection }}"
 
-  - name: Reset udld
-    nxos_udld:
-      reset: True
-      provider: "{{ connection }}"
-
-  - name: Ensure udld agg mode is globally disabled and msg time is 20
+  - name: Configure udld
     nxos_udld: &conf1
-      aggressive: disabled
+      aggressive: enabled
       msg_time: 20
       provider: "{{ connection }}"
     register: result
@@ -32,7 +27,7 @@
       that:
         - "result.changed == true"
 
-  - name: "Conf1 Idempotence"
+  - name: "Check Idempotence"
     nxos_udld: *conf1
     register: result
 
@@ -40,22 +35,59 @@
       that:
         - "result.changed == false"
 
+  - name: Reset udld
+    nxos_udld:
+      reset: True
+      provider: "{{ connection }}"
 
-  - name: Ensure udld agg mode is globally enabled and msg time is 15
+  - name: Configure udld2
     nxos_udld: &conf2
-      aggressive: enabled
-      msg_time: 15
+      aggressive: disabled
       provider: "{{ connection }}"
     register: result
 
   - assert: *true
 
-  - name: "conf2 Idempotence"
+  - name: "Check Idempotence"
     nxos_udld: *conf2
     register: result
 
   - assert: *false
 
+  - name: Configure udld3
+    nxos_udld: &conf3
+      msg_time: default
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Check Idempotence"
+    nxos_udld: *conf3
+    register: result
+
+  - assert: *false
+
+  - name: Configure udld again
+    nxos_udld: *conf1
+    register: result
+
+  - assert: *true
+
+  - name: Remove udld config
+    nxos_udld: &conf4
+      state: absent
+      provider: "{{ connection }}"
+    register: result
+
+  - assert: *true
+
+  - name: "Check Idempotence"
+    nxos_udld: *conf4
+    register: result
+
+  - assert: *false
+
   when: udld_run
 
   always: