Repository: ansible/ansible
Pull request: nxos_interface: Disable switchport for loopback/svi
##### SUMMARY
Fixes #38559

This PR fixes a regression introduced in v2.5 that if you have an interface type of "loopback" or "svi", and have "mode" set, causes the module to thrown an exception because a switchport command is issued to an interface that does not support it.

We probably want to fix this by testing if the "mode" parameter is allowed for these interface types, but that's a breaking change and probably requires a deprecation process and warnings before making that switch.

At least we want to fix this regression first.

##### ISSUE TYPE
 - Bugfix Pull Request

##### COMPONENT NAME
nxos_interface

##### ANSIBLE VERSION
v2.5 and older

Problem statement:
nxos_interface: Invalid command at '^' marker
##### ISSUE TYPE
 - Bug Report

##### COMPONENT NAME
nxos_interface

##### ANSIBLE VERSION
v2.5+

##### CONFIGURATION
```
ANSIBLE_PIPELINING(/home/dwieers/backend/ansible.cfg) = True
DEFAULT_CALLBACK_WHITELIST(/home/dwieers/backend/ansible.cfg) = ['mail']
DEFAULT_FORKS(/home/dwieers/backend/ansible.cfg) = 15
DEFAULT_TRANSPORT(/home/dwieers/backend/ansible.cfg) = ssh
```

##### OS / ENVIRONMENT
CentOS 7.4

##### SUMMARY
When running the below task with devel branch, the task fails. This did not happen with Ansible v2.4.

```yaml
  - name: Create VLAN interface
    nxos_interface:
      interface: vlan {{ item.group }}
      admin_state: up
      mode: layer3
      state: present
    with_items: '{{ port_channels }}'
```

The error we get is:

```
TASK [Create VLAN interface] **********************************************************************************************************************
task path: /home/dwieers/backend/task041-create_nxos_aci_topo_scenario01.yaml:109
failed: [bdsol-aci12-n3k.cisco.com] (item={u'router_id': u'6.6.6.3', u'group': 106, u'loopback_addr': u'8.8.8.8', u'vlan_mask': 24, u'vlan_addr': u'7.7.7.3', u'loopback_mask': 32, u'trunk_allowed_vlans': 106, u'pcmode': u'active', u'mode': u'trunk', u'members': [u'eth 1/6', u'eth 1/14']}) => {"changed": false, "item": {"group": 106, "loopback_addr": "8.8.8.8", "loopback_mask": 32, "members": ["eth 1/6", "eth 1/14"], "mode": "trunk", "pcmode": "active", "router_id": "6.6.6.3", "trunk_allowed_vlans": 106, "vlan_addr": "7.7.7.3", "vlan_mask": 24}, "msg": "no switchport\r\r\n                                               ^\r\n% Invalid command at '^' marker.\r\n\rn3k.foo.bar(config-if)# "}
```

Tested using connection parameters, provider and nxos/nxapi. All show the same behavior.

```
no switchport
                                               ^
% Invalid command at '^' marker.

n3k.foo.bar(config-if)# 
```

After dumping the exact commands being run:
```
interface Vlan106
no switchport
```

It appears it is doing a "no switchport" on a VLAN interface, where it did not do that before.

Hints from issue discussion:
Files identified in the description:
* [lib/ansible/modules/network/nxos/nxos_interface.py](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/network/nxos/nxos_interface.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 @jedelman8 @mikewiebe @privateip @rahushen @rcarrillocruz @trishnaguha
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
@dagwieers What happens when you execute the command manually on the device?
I think this because lack of support of the command for vlan interface.
I am using NX-OS 9K and 7K. Both doesn't support switchport in vlan interface config.
@trishnaguha Correct, same for loopback interfaces. I have the code fixes and will make a PR shortly.
The Doc for the mode param in nxos_interface states that

`Manage Layer 2 or Layer 3 state of the interface. This option is supported for ethernet and portchannel interface. Applicable for ethernet and portchannel interface only.` http://docs.ansible.com/ansible/latest/modules/nxos_interface_module.html

This does not look like a bug to me as it is documented.

Commit message: nxos_interface: Disable switchport for loopback/svi

Patch to review:
diff --git a/lib/ansible/modules/network/nxos/nxos_interface.py b/lib/ansible/modules/network/nxos/nxos_interface.py
index ca471d9553ace2..079698613796c0 100644
--- a/lib/ansible/modules/network/nxos/nxos_interface.py
+++ b/lib/ansible/modules/network/nxos/nxos_interface.py
@@ -1,20 +1,7 @@
 #!/usr/bin/python
-#
-# This file is part of Ansible
-#
-# Ansible is free software: you can redistribute it and/or modify
-# it under the terms of the GNU General Public License as published by
-# the Free Software Foundation, either version 3 of the License, or
-# (at your option) any later version.
-#
-# Ansible is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with Ansible.  If not, see <http://www.gnu.org/licenses/>.
-#
+# -*- coding: utf-8 -*-
+
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 ANSIBLE_METADATA = {'metadata_version': '1.1',
                     'status': ['preview'],
@@ -444,10 +431,12 @@ def map_obj_to_commands(updates, module):
 
         elif state == 'present':
             if obj_in_have:
-                if mode == 'layer2' and mode != obj_in_have.get('mode'):
-                    add_command_to_interface(interface, 'switchport', commands)
-                elif mode == 'layer3' and mode != obj_in_have.get('mode'):
-                    add_command_to_interface(interface, 'no switchport', commands)
+                # Don't run switchport command for loopback and svi interfaces
+                if get_interface_type(name) in ('ethernet', 'portchannel'):
+                    if mode == 'layer2' and mode != obj_in_have.get('mode'):
+                        add_command_to_interface(interface, 'switchport', commands)
+                    elif mode == 'layer3' and mode != obj_in_have.get('mode'):
+                        add_command_to_interface(interface, 'no switchport', commands)
 
                 if admin_state == 'up' and admin_state != obj_in_have.get('admin_state'):
                     add_command_to_interface(interface, 'no shutdown', commands)
@@ -484,10 +473,12 @@ def map_obj_to_commands(updates, module):
 
             else:
                 commands.append(interface)
-                if mode == 'layer2':
-                    commands.append('switchport')
-                elif mode == 'layer3':
-                    commands.append('no switchport')
+                # Don't run switchport command for loopback and svi interfaces
+                if interface_type in ('ethernet', 'portchannel'):
+                    if mode == 'layer2':
+                        commands.append('switchport')
+                    elif mode == 'layer3':
+                        commands.append('no switchport')
 
                 if admin_state == 'up':
                     commands.append('no shutdown')


Code review comments:
diff_hunk: @@ -484,10 +473,12 @@ def map_obj_to_commands(updates, module):
 
             else:
                 commands.append(interface)
-                if mode == 'layer2':
-                    commands.append('switchport')
-                elif mode == 'layer3':
-                    commands.append('no switchport')
+                # Don't run switchport command for loopback and svi interfaces
+                if interface_type in ('ethernet', 'portchannel'):
path: lib/ansible/modules/network/nxos/nxos_interface.py
text: @user1:
Please change this to `get_interface_type(name)` here. `interface_type` is one of the module parameters which is mutually exclusive to module param `name` or `interface`. `get_interface_type` function returns type of the interface.