nxos bugfixes cherry-pick (#38091)

* Fix nxos_switchport (#37328) (cherry picked from commit ff57fd0) * Fix nxos_l2_interface and test typo (#37336) Signed-off-by: Trishna Guha <trishnaguha17@gmail.com> (cherry picked from commit ba5e562) * fix ios_l2_interface (#37389) Signed-off-by: Trishna Guha <trishnaguha17@gmail.com> (cherry picked from commit dd37857) * fix required args for nxos_snapshot and docs improvement (#37232) Signed-off-by: Trishna Guha <trishnaguha17@gmail.com> (cherry picked from commit a10df8b) * add nxos_snapshot test for missing required param (#37248) Signed-off-by: Trishna Guha <trishnaguha17@gmail.com> (cherry picked from commit 2501834) * Ensure network_cli nxos test is run only once - remove unnecessary files (#37462) Signed-off-by: Trishna Guha <trishnaguha17@gmail.com> (cherry picked from commit 957ab02) * Integration Tests only: nxos_user (#37852) * add new integration file to nxos_user * fix shippable error * change nxapi to connection * review comments (cherry picked from commit 63da50e) * fix UnboundLocalError nxos_bgp_af module (#37610) Signed-off-by: Trishna Guha <trishnaguha17@gmail.com> (cherry picked from commit 00abe6d) * Fix nxos_vrf issues (#37092) * fix nxos_vrf issues * fix doc (cherry picked from commit dc61f4c) * fix nxos_vrf_af issues (#37211) (cherry picked from commit 74e79d9) * fix nxos_udld issues (#37418) (cherry picked from commit 05b266c) * fix nxos_vlan issues (#38008) (cherry picked from commit 6f2cb28) * add changelog Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>
ansible · Mar 29, 2018 · a716fe9 · a716fe9
1 parent 60e7aa0
commit a716fe9
Show file tree

Hide file tree

Showing 53 changed files with 733 additions and 343 deletions.
diff --git a/changelogs/fragments/ios_l2_interface_fix.yaml b/changelogs/fragments/ios_l2_interface_fix.yaml
@@ -0,0 +1,2 @@
+bugfixes:
+- ios_l2_interface - fix removal of trunk vlans (https://github.com/ansible/ansible/pull/37389)
diff --git a/changelogs/fragments/nxos_bugfixes.yaml b/changelogs/fragments/nxos_bugfixes.yaml
@@ -0,0 +1,11 @@
+bugfixes:
+- _nxos_switchport - fix removal of trunk vlans (https://github.com/ansible/ansible/pull/37328)
+- nxos_l2_interface - fix removal of trunk vlans (https://github.com/ansible/ansible/pull/37336)
+- nxos_snapshot - fix documentation and add required parameter logic (https://github.com/ansible/ansible/pull/37232, https://github.com/ansible/ansible/pull/37248)
+- Improve integration test - Ensure each transport test runs only once (https://github.com/ansible/ansible/pull/37462)
+- nxos_user - Integration test (https://github.com/ansible/ansible/pull/37852)
+- nxos_bgp_af - Fix UnboundLocalError (https://github.com/ansible/ansible/pull/37610)
+- nxos_vrf - Fix nxos_vrf issues (https://github.com/ansible/ansible/pull/37092)
+- nxos_vrf_af - Fix nxos_vrf_af issues (https://github.com/ansible/ansible/pull/37211)
+- nxos_udld - Fix nxos_udld issues (https://github.com/ansible/ansible/pull/37418)
+- nxos_vlan - Fix nxos_vlan issues (https://github.com/ansible/ansible/pull/38008)
diff --git a/lib/ansible/modules/network/ios/ios_l2_interface.py b/lib/ansible/modules/network/ios/ios_l2_interface.py
@@ -185,18 +185,21 @@ def remove_switchport_config_commands(name, existing, proposed, module):
             commands.append(command)
 
     elif mode == 'trunk':
-        tv_check = existing.get('trunk_vlans_list') == proposed.get('trunk_vlans_list')
-
-        if not tv_check:
-            existing_vlans = existing.get('trunk_vlans_list')
-            proposed_vlans = proposed.get('trunk_vlans_list')
-            vlans_to_remove = set(proposed_vlans).intersection(existing_vlans)
-
-            if vlans_to_remove:
-                proposed_allowed_vlans = proposed.get('trunk_allowed_vlans')
-                remove_trunk_allowed_vlans = proposed.get('trunk_vlans', proposed_allowed_vlans)
-                command = 'switchport trunk allowed vlan remove {0}'.format(remove_trunk_allowed_vlans)
-                commands.append(command)
+        # Supported Remove Scenarios for trunk_vlans_list
+        # 1) Existing: 1,2,3 Proposed: 1,2,3 - Remove all
+        # 2) Existing: 1,2,3 Proposed: 1,2   - Remove 1,2 Leave 3
+        # 3) Existing: 1,2,3 Proposed: 2,3   - Remove 2,3 Leave 1
+        # 4) Existing: 1,2,3 Proposed: 4,5,6 - None removed.
+        # 5) Existing: None  Proposed: 1,2,3 - None removed.
+        existing_vlans = existing.get('trunk_vlans_list')
+        proposed_vlans = proposed.get('trunk_vlans_list')
+        vlans_to_remove = set(proposed_vlans).intersection(existing_vlans)
+
+        if vlans_to_remove:
+            proposed_allowed_vlans = proposed.get('trunk_allowed_vlans')
+            remove_trunk_allowed_vlans = proposed.get('trunk_vlans', proposed_allowed_vlans)
+            command = 'switchport trunk allowed vlan remove {0}'.format(remove_trunk_allowed_vlans)
+            commands.append(command)
 
         native_check = existing.get('native_vlan') == proposed.get('native_vlan')
         if native_check and proposed.get('native_vlan'):
@@ -291,7 +294,7 @@ def vlan_range_to_list(vlans):
     result = []
     if vlans:
         for part in vlans.split(','):
-            if part == 'none':
+            if part.lower() == 'none':
                 break
             if '-' in part:
                 start, stop = (int(i) for i in part.split('-'))

diff --git a/lib/ansible/modules/network/nxos/_nxos_switchport.py b/lib/ansible/modules/network/nxos/_nxos_switchport.py
@@ -269,18 +269,23 @@ def remove_switchport_config_commands(interface, existing, proposed, module):
             commands.append(command)
 
     elif mode == 'trunk':
-        tv_check = existing.get('trunk_vlans_list') == proposed.get('trunk_vlans_list')
-
-        if tv_check:
-            existing_vlans = existing.get('trunk_vlans_list')
-            proposed_vlans = proposed.get('trunk_vlans_list')
-            vlans_to_remove = set(proposed_vlans).intersection(existing_vlans)
 
-            if vlans_to_remove:
-                proposed_allowed_vlans = proposed.get('trunk_allowed_vlans')
-                remove_trunk_allowed_vlans = proposed.get('trunk_vlans', proposed_allowed_vlans)
-                command = 'switchport trunk allowed vlan remove {0}'.format(remove_trunk_allowed_vlans)
-                commands.append(command)
+        # Supported Remove Scenarios for trunk_vlans_list
+        # 1) Existing: 1,2,3 Proposed: 1,2,3 - Remove all
+        # 2) Existing: 1,2,3 Proposed: 1,2   - Remove 1,2 Leave 3
+        # 3) Existing: 1,2,3 Proposed: 2,3   - Remove 2,3 Leave 1
+        # 4) Existing: 1,2,3 Proposed: 4,5,6 - None removed.
+        # 5) Existing: None  Proposed: 1,2,3 - None removed.
+
+        existing_vlans = existing.get('trunk_vlans_list')
+        proposed_vlans = proposed.get('trunk_vlans_list')
+        vlans_to_remove = set(proposed_vlans).intersection(existing_vlans)
+
+        if vlans_to_remove:
+            proposed_allowed_vlans = proposed.get('trunk_allowed_vlans')
+            remove_trunk_allowed_vlans = proposed.get('trunk_vlans', proposed_allowed_vlans)
+            command = 'switchport trunk allowed vlan remove {0}'.format(remove_trunk_allowed_vlans)
+            commands.append(command)
 
         native_check = existing.get('native_vlan') == proposed.get('native_vlan')
         if native_check and proposed.get('native_vlan'):

diff --git a/lib/ansible/modules/network/nxos/nxos_bgp_af.py b/lib/ansible/modules/network/nxos/nxos_bgp_af.py
@@ -543,14 +543,16 @@ def get_network_command(existing, key, value):
                 command = '{0} {1}'.format(key, inet[0])
             elif len(inet) == 2:
                 command = '{0} {1} route-map {2}'.format(key, inet[0], inet[1])
-            commands.append(command)
+            if command:
+                commands.append(command)
     for enet in existing_networks:
         if enet not in value:
             if len(enet) == 1:
                 command = 'no {0} {1}'.format(key, enet[0])
             elif len(enet) == 2:
                 command = 'no {0} {1} route-map {2}'.format(key, enet[0], enet[1])
-            commands.append(command)
+            if command:
+                commands.append(command)
     return commands
 
 
@@ -568,7 +570,8 @@ def get_inject_map_command(existing, key, value):
                 command = ('inject-map {0} exist-map {1} '
                            'copy-attributes'.format(maps[0],
                                                     maps[1]))
-            commands.append(command)
+            if command:
+                commands.append(command)
     for emaps in existing_maps:
         if emaps not in value:
             if len(emaps) == 2:
@@ -578,7 +581,8 @@ def get_inject_map_command(existing, key, value):
                 command = ('no inject-map {0} exist-map {1} '
                            'copy-attributes'.format(emaps[0],
                                                     emaps[1]))
-            commands.append(command)
+            if command:
+                commands.append(command)
     return commands
 
 

diff --git a/lib/ansible/modules/network/nxos/nxos_l2_interface.py b/lib/ansible/modules/network/nxos/nxos_l2_interface.py
@@ -252,18 +252,15 @@ def remove_switchport_config_commands(name, existing, proposed, module):
             commands.append(command)
 
     elif mode == 'trunk':
-        tv_check = existing.get('trunk_vlans_list') == proposed.get('trunk_vlans_list')
-
-        if tv_check:
-            existing_vlans = existing.get('trunk_vlans_list')
-            proposed_vlans = proposed.get('trunk_vlans_list')
-            vlans_to_remove = set(proposed_vlans).intersection(existing_vlans)
-
-            if vlans_to_remove:
-                proposed_allowed_vlans = proposed.get('trunk_allowed_vlans')
-                remove_trunk_allowed_vlans = proposed.get('trunk_vlans', proposed_allowed_vlans)
-                command = 'switchport trunk allowed vlan remove {0}'.format(remove_trunk_allowed_vlans)
-                commands.append(command)
+        existing_vlans = existing.get('trunk_vlans_list')
+        proposed_vlans = proposed.get('trunk_vlans_list')
+        vlans_to_remove = set(proposed_vlans).intersection(existing_vlans)
+
+        if vlans_to_remove:
+            proposed_allowed_vlans = proposed.get('trunk_allowed_vlans')
+            remove_trunk_allowed_vlans = proposed.get('trunk_vlans', proposed_allowed_vlans)
+            command = 'switchport trunk allowed vlan remove {0}'.format(remove_trunk_allowed_vlans)
+            commands.append(command)
 
         native_check = existing.get('native_vlan') == proposed.get('native_vlan')
         if native_check and proposed.get('native_vlan'):

diff --git a/lib/ansible/modules/network/nxos/nxos_snapshot.py b/lib/ansible/modules/network/nxos/nxos_snapshot.py
@@ -69,7 +69,7 @@
         default: null
     comparison_results_file:
         description:
-            - Name of the file where snapshots comparison will be store.
+            - Name of the file where snapshots comparison will be stored when C(action=compare).
         required: false
         default: null
     compare_option:
@@ -354,7 +354,13 @@ def main():
 
     argument_spec.update(nxos_argument_spec)
 
+    required_if = [("action", "compare", ["snapshot1", "snapshot2", "comparison_results_file"]),
+                   ("action", "create", ["snapshot_name", "description"]),
+                   ("action", "add", ["section", "show_command", "row_id", "element_key1"]),
+                   ("action", "delete", ["snapshot_name"])]
+
     module = AnsibleModule(argument_spec=argument_spec,
+                           required_if=required_if,
                            supports_check_mode=True)
 
     warnings = list()
@@ -363,33 +369,10 @@ def main():
     action = module.params['action']
     comparison_results_file = module.params['comparison_results_file']
 
-    CREATE_PARAMS = ['snapshot_name', 'description']
-    ADD_PARAMS = ['section', 'show_command', 'row_id', 'element_key1']
-    COMPARE_PARAMS = ['snapshot1', 'snapshot2', 'comparison_results_file']
-
     if not os.path.isdir(module.params['path']):
         module.fail_json(msg='{0} is not a valid directory name.'.format(
             module.params['path']))
 
-    if action == 'create':
-        for param in CREATE_PARAMS:
-            if not module.params[param]:
-                module.fail_json(msg='snapshot_name and description are '
-                                     'required when action=create')
-    elif action == 'add':
-        for param in ADD_PARAMS:
-            if not module.params[param]:
-                module.fail_json(msg='section, show_command, row_id '
-                                     'and element_key1 are required '
-                                     'when action=add')
-    elif action == 'compare':
-        for param in COMPARE_PARAMS:
-            if not module.params[param]:
-                module.fail_json(msg='snapshot1 and snapshot2 are required '
-                                     'when action=create')
-    elif action == 'delete' and not module.params['snapshot_name']:
-        module.fail_json(msg='snapshot_name is required when action=delete')
-
     existing_snapshots = invoke('get_existing', module)
     action_results = invoke('action_%s' % action, module, existing_snapshots)
 

diff --git a/lib/ansible/modules/network/nxos/nxos_udld.py b/lib/ansible/modules/network/nxos/nxos_udld.py
@@ -34,8 +34,6 @@
     - 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 @@
         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.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)