Repository: ansible/ansible
Pull request: nxos_vlans: fix rmb behaviors and tests
##### SUMMARY
`nxos_vlans` had several failed behaviors and did not work properly on all nxos platforms.

- facts detection was not working correctly; e.g. when the device cli showed `vlan 1,3-5`  the module was only reporting information for vlans 3 and 5.
- The `deleted`, `replaced`, and `overridden` states were not handled correctly. Some objects were ignored and others were changed when they should not have been.
- The vlan cli is quite different on N5K/N6K so the current approach of collected data from `show running` won't work. I rewrote the facts collector to use structured data, which has most of the state and is consistent across all platforms. The `mapped_vni` attribute is the only state that is not included in the structured data so I used `show running` output for that attribute only.
- I added a fairly exhaustive Unit Test for this module. Please review this UT, it demonstrates a wide variety of test scenarios required for the nxos_vlans module.

##### ISSUE TYPE
- Bugfix Pull Request

##### COMPONENT NAME
`nxos_vlans`

##### ADDITIONAL INFORMATION
Tests are now passing on ALL of our regression platforms, which includes a variety of different images: (internal id's: `dt-n9k5-1, n6k-77, n9k-109, n3k-105, n7k-99, n7k-j, n9k-108, n9k-109, n9k-100, n3k-173`)

Unit test coverage:
```
lib/ansible/module_utils/network/nxos/argspec/vlans/vlans.py    86%
lib/ansible/module_utils/network/nxos/config/vlans/vlans.py     99%
lib/ansible/module_utils/network/nxos/facts/vlans/vlans.py      93%
lib/ansible/modules/network/nxos/nxos_vlans.py                  89%
```

##### ISSUES
This patch also resolves #63817.

Problem statement:
nxos_vlans dies with TypeError: 'NoneType' object has no attribute '__getitem__'
##### SUMMARY
Attempting to rewrite playbooks with nxos_vlan*s*, in 2.9 "preview" RPM, but failing instantly.

##### ISSUE TYPE
- Bug Report

##### COMPONENT NAME
nxos_vlans

##### ANSIBLE VERSION
```
ansible 2.9.0rc4
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 2.7.5 (default, Aug  7 2019, 00:51:29) [GCC 4.8.5 20150623 (Red Hat 4.8.5-39)]
```

##### CONFIGURATION
```
ANSIBLE_PIPELINING(/etc/ansible/ansible.cfg) = True
DEFAULT_FORKS(/etc/ansible/ansible.cfg) = 99
DEFAULT_GATHERING(/etc/ansible/ansible.cfg) = smart
DEFAULT_LOG_PATH(/etc/ansible/ansible.cfg) = /var/log/ansible.log
DISPLAY_ARGS_TO_STDOUT(/etc/ansible/ansible.cfg) = True
RETRY_FILES_ENABLED(/etc/ansible/ansible.cfg) = False
SHOW_CUSTOM_STATS(/etc/ansible/ansible.cfg) = True
```

##### OS / ENVIRONMENT
CentOS 7.7 controller
NX-OS versions vary, but we aren't even getting there so irrelevant for now.

##### STEPS TO REPRODUCE
```yaml
---
- name: Apply VLAN changes to Cisco Nexus
  hosts: cisco_nexus
  gather_facts: false
  tasks:

  - name: Remove VLANs
    nxos_vlans:
      config:
        - vlan_id: "{{ item }}"
      state: "deleted"
    loop:
      - 20
    register: commands

  - debug:
      var: commands
      verbosity: 1
```

/etc/ansible/hosts (partially obscured, sorry)
```inifile

[network]

[network:children]
switches
routers

[network:vars]
ansible_user=haha,no
ansible_password=er,no,again
ansible_connection=network_cli

[switches:children]
cisco_nexus
ruckus_icx
cisco_catalyst
extreme_exos
arista

[routers:children]
extreme_ironware
arista

[cisco:children]
cisco_nexus
cisco_catalyst

[cisco:vars]

[cisco_nexus]
aria            ansible_host=x
paris           ansible_host=x
n5ktop          ansible_host=x
n5kbottom       ansible_host=x
cdcn5ktop       ansible_host=x
cdcn5kbottom    ansible_host=x

[cisco_nexus:vars]
ansible_network_os="nxos"

[cisco_catalyst]
x ansible_host=x

[cisco_catalyst:vars]
ansible_network_os="ios"

[ruckus_icx]
xansible_host=x

[ruckus_icx:vars]
ansible_network_os="ironware"

[extreme_ironware]
x ansible_host=x

[extreme_ironware:vars]
ansible_network_os="ironware"

[extreme_exos]
x ansible_host=x

[extreme_exos:vars]
ansible_network_os="exos"

[arista]
x ansible_host=x

[arista:vars]
ansible_network_os="eos"
```

##### EXPECTED RESULTS
Some other error message telling me how I screwed up, most likely :-)


##### ACTUAL RESULTS
```paste below
[root@ansible vlan]# ansible-playbook --check -vvvv vlans-nexus.yml 
ansible-playbook 2.9.0rc4
  config file = /etc/ansible/ansible.cfg
  configured module search path = [u'/root/.ansible/plugins/modules', u'/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/lib/python2.7/site-packages/ansible
  executable location = /usr/bin/ansible-playbook
  python version = 2.7.5 (default, Aug  7 2019, 00:51:29) [GCC 4.8.5 20150623 (Red Hat 4.8.5-39)]
Using /etc/ansible/ansible.cfg as config file
setting up inventory plugins
host_list declined parsing /etc/ansible/hosts as it did not pass its verify_file() method
script declined parsing /etc/ansible/hosts as it did not pass its verify_file() method
auto declined parsing /etc/ansible/hosts as it did not pass its verify_file() method
Parsed /etc/ansible/hosts inventory source with ini plugin
/usr/lib/python2.7/site-packages/requests/__init__.py:91: RequestsDependencyWarning: urllib3 (1.24.1) or chardet (2.2.1) doesn't match a supported version!
  RequestsDependencyWarning)
Loading callback plugin default of type stdout, v2.0 from /usr/lib/python2.7/site-packages/ansible/plugins/callback/default.pyc

PLAYBOOK: vlans-nexus.yml *************************************************************************************************************************************************************************
Positional arguments: vlans-nexus.yml
check: True
become_method: sudo
inventory: (u'/etc/ansible/hosts',)
forks: 99
tags: (u'all',)
verbosity: 4
connection: smart
timeout: 10
1 plays in vlans-nexus.yml

PLAY [Apply VLAN changes to Cisco Nexus] **********************************************************************************************************************************************************
META: ran handlers

TASK [Remove VLANs state=deleted, config=[{u'vlan_id': u'{{ item }}'}]] ***************************************************************************************************************************
task path: /root/vlan/vlans-nexus.yml:7
<10.0.2.237> attempting to start connection
<10.0.2.237> using connection plugin network_cli
<10.0.2.238> attempting to start connection
<10.0.2.238> using connection plugin network_cli
<10.0.2.202> attempting to start connection
<10.0.2.202> using connection plugin network_cli
<10.0.2.201> attempting to start connection
<10.0.2.201> using connection plugin network_cli
<10.0.2.11> attempting to start connection
<10.0.2.11> using connection plugin network_cli
<10.0.2.12> attempting to start connection
<10.0.2.12> using connection plugin network_cli
<10.0.2.238> local domain socket does not exist, starting it
<10.0.2.238> control socket path is /root/.ansible/pc/8d396364e4
<10.0.2.238> local domain socket listeners started successfully
<10.0.2.238> loaded cliconf plugin for network_os nxos
<10.0.2.238> 
<10.0.2.238> local domain socket path is /root/.ansible/pc/8d396364e4
<10.0.2.11> local domain socket does not exist, starting it
<10.0.2.11> control socket path is /root/.ansible/pc/da6da5a9f1
<10.0.2.11> local domain socket listeners started successfully
<10.0.2.11> loaded cliconf plugin for network_os nxos
<10.0.2.11> 
<10.0.2.11> local domain socket path is /root/.ansible/pc/da6da5a9f1
<10.0.2.202> local domain socket does not exist, starting it
<10.0.2.202> control socket path is /root/.ansible/pc/870a4db6ba
<10.0.2.202> local domain socket listeners started successfully
<10.0.2.202> loaded cliconf plugin for network_os nxos
<10.0.2.202> 
<10.0.2.202> local domain socket path is /root/.ansible/pc/870a4db6ba
<10.0.2.237> local domain socket does not exist, starting it
<10.0.2.237> control socket path is /root/.ansible/pc/a3e4e489de
<10.0.2.237> local domain socket listeners started successfully
<10.0.2.237> loaded cliconf plugin for network_os nxos
<10.0.2.237> 
<10.0.2.237> local domain socket path is /root/.ansible/pc/a3e4e489de
<10.0.2.12> local domain socket does not exist, starting it
<10.0.2.12> control socket path is /root/.ansible/pc/1dca186e56
<10.0.2.12> local domain socket listeners started successfully
<10.0.2.12> loaded cliconf plugin for network_os nxos
<10.0.2.12> 
<10.0.2.12> local domain socket path is /root/.ansible/pc/1dca186e56
<10.0.2.201> local domain socket does not exist, starting it
<10.0.2.201> control socket path is /root/.ansible/pc/f50c5d0caf
<10.0.2.201> local domain socket listeners started successfully
<10.0.2.201> loaded cliconf plugin for network_os nxos
<10.0.2.201> 
<10.0.2.201> local domain socket path is /root/.ansible/pc/f50c5d0caf
<cdcn5ktop> Attempting python interpreter discovery
<aria> Attempting python interpreter discovery
<10.0.2.11> ESTABLISH LOCAL CONNECTION FOR USER: root
<10.0.2.237> ESTABLISH LOCAL CONNECTION FOR USER: root
<10.0.2.11> EXEC /bin/sh -c 'echo PLATFORM; uname; echo FOUND; command -v '"'"'/usr/bin/python'"'"'; command -v '"'"'python3.7'"'"'; command -v '"'"'python3.6'"'"'; command -v '"'"'python3.5'"'"'; command -v '"'"'python2.7'"'"'; command -v '"'"'python2.6'"'"'; command -v '"'"'/usr/libexec/platform-python'"'"'; command -v '"'"'/usr/bin/python3'"'"'; command -v '"'"'python'"'"'; echo ENDFOUND && sleep 0'
<10.0.2.237> EXEC /bin/sh -c 'echo PLATFORM; uname; echo FOUND; command -v '"'"'/usr/bin/python'"'"'; command -v '"'"'python3.7'"'"'; command -v '"'"'python3.6'"'"'; command -v '"'"'python3.5'"'"'; command -v '"'"'python2.7'"'"'; command -v '"'"'python2.6'"'"'; command -v '"'"'/usr/libexec/platform-python'"'"'; command -v '"'"'/usr/bin/python3'"'"'; command -v '"'"'python'"'"'; echo ENDFOUND && sleep 0'
<paris> Attempting python interpreter discovery
<10.0.2.238> ESTABLISH LOCAL CONNECTION FOR USER: root
<n5kbottom> Attempting python interpreter discovery
<10.0.2.238> EXEC /bin/sh -c 'echo PLATFORM; uname; echo FOUND; command -v '"'"'/usr/bin/python'"'"'; command -v '"'"'python3.7'"'"'; command -v '"'"'python3.6'"'"'; command -v '"'"'python3.5'"'"'; command -v '"'"'python2.7'"'"'; command -v '"'"'python2.6'"'"'; command -v '"'"'/usr/libexec/platform-python'"'"'; command -v '"'"'/usr/bin/python3'"'"'; command -v '"'"'python'"'"'; echo ENDFOUND && sleep 0'
<10.0.2.201> ESTABLISH LOCAL CONNECTION FOR USER: root
<10.0.2.201> EXEC /bin/sh -c 'echo PLATFORM; uname; echo FOUND; command -v '"'"'/usr/bin/python'"'"'; command -v '"'"'python3.7'"'"'; command -v '"'"'python3.6'"'"'; command -v '"'"'python3.5'"'"'; command -v '"'"'python2.7'"'"'; command -v '"'"'python2.6'"'"'; command -v '"'"'/usr/libexec/platform-python'"'"'; command -v '"'"'/usr/bin/python3'"'"'; command -v '"'"'python'"'"'; echo ENDFOUND && sleep 0'
<n5ktop> Attempting python interpreter discovery
<10.0.2.202> ESTABLISH LOCAL CONNECTION FOR USER: root
<10.0.2.202> EXEC /bin/sh -c 'echo PLATFORM; uname; echo FOUND; command -v '"'"'/usr/bin/python'"'"'; command -v '"'"'python3.7'"'"'; command -v '"'"'python3.6'"'"'; command -v '"'"'python3.5'"'"'; command -v '"'"'python2.7'"'"'; command -v '"'"'python2.6'"'"'; command -v '"'"'/usr/libexec/platform-python'"'"'; command -v '"'"'/usr/bin/python3'"'"'; command -v '"'"'python'"'"'; echo ENDFOUND && sleep 0'
<cdcn5kbottom> Attempting python interpreter discovery
<10.0.2.12> ESTABLISH LOCAL CONNECTION FOR USER: root
<10.0.2.12> EXEC /bin/sh -c 'echo PLATFORM; uname; echo FOUND; command -v '"'"'/usr/bin/python'"'"'; command -v '"'"'python3.7'"'"'; command -v '"'"'python3.6'"'"'; command -v '"'"'python3.5'"'"'; command -v '"'"'python2.7'"'"'; command -v '"'"'python2.6'"'"'; command -v '"'"'/usr/libexec/platform-python'"'"'; command -v '"'"'/usr/bin/python3'"'"'; command -v '"'"'python'"'"'; echo ENDFOUND && sleep 0'
<10.0.2.11> EXEC /bin/sh -c '/usr/bin/python && sleep 0'
<10.0.2.237> EXEC /bin/sh -c '/usr/bin/python && sleep 0'
<10.0.2.238> EXEC /bin/sh -c '/usr/bin/python && sleep 0'
<10.0.2.202> EXEC /bin/sh -c '/usr/bin/python && sleep 0'
<10.0.2.201> EXEC /bin/sh -c '/usr/bin/python && sleep 0'
<10.0.2.12> EXEC /bin/sh -c '/usr/bin/python && sleep 0'
Using module file /usr/lib/python2.7/site-packages/ansible/modules/network/nxos/nxos_vlans.py
Pipelining is enabled.
<10.0.2.11> EXEC /bin/sh -c '/usr/bin/python && sleep 0'
Using module file /usr/lib/python2.7/site-packages/ansible/modules/network/nxos/nxos_vlans.py
Pipelining is enabled.
<10.0.2.237> EXEC /bin/sh -c '/usr/bin/python && sleep 0'
Using module file /usr/lib/python2.7/site-packages/ansible/modules/network/nxos/nxos_vlans.py
Using module file /usr/lib/python2.7/site-packages/ansible/modules/network/nxos/nxos_vlans.py
Pipelining is enabled.
Pipelining is enabled.
<10.0.2.238> EXEC /bin/sh -c '/usr/bin/python && sleep 0'
<10.0.2.12> EXEC /bin/sh -c '/usr/bin/python && sleep 0'
Using module file /usr/lib/python2.7/site-packages/ansible/modules/network/nxos/nxos_vlans.py
Pipelining is enabled.
<10.0.2.201> EXEC /bin/sh -c '/usr/bin/python && sleep 0'
Using module file /usr/lib/python2.7/site-packages/ansible/modules/network/nxos/nxos_vlans.py
Pipelining is enabled.
<10.0.2.202> EXEC /bin/sh -c '/usr/bin/python && sleep 0'
The full traceback is:
Traceback (most recent call last):
  File "<stdin>", line 102, in <module>
  File "<stdin>", line 94, in _ansiballz_main
  File "<stdin>", line 40, in invoke_module
  File "/usr/lib64/python2.7/runpy.py", line 180, in run_module
    fname, loader, pkg_name)
  File "/usr/lib64/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/tmp/ansible_nxos_vlans_payload_mDCyj4/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py", line 247, in <module>
  File "/tmp/ansible_nxos_vlans_payload_mDCyj4/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py", line 242, in main
  File "/tmp/ansible_nxos_vlans_payload_mDCyj4/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 65, in execute_module
  File "/tmp/ansible_nxos_vlans_payload_mDCyj4/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 95, in set_config
  File "/tmp/ansible_nxos_vlans_payload_mDCyj4/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 115, in set_state
  File "/tmp/ansible_nxos_vlans_payload_mDCyj4/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 196, in _state_deleted
TypeError: 'NoneType' object has no attribute '__getitem__'

failed: [n5ktop] (item=20) => {
    "ansible_facts": {
        "discovered_interpreter_python": "/usr/bin/python"
    }, 
    "ansible_loop_var": "item", 
    "changed": false, 
    "item": 20, 
    "module_stderr": "Traceback (most recent call last):\n  File \"<stdin>\", line 102, in <module>\n  File \"<stdin>\", line 94, in _ansiballz_main\n  File \"<stdin>\", line 40, in invoke_module\n  File \"/usr/lib64/python2.7/runpy.py\", line 180, in run_module\n    fname, loader, pkg_name)\n  File \"/usr/lib64/python2.7/runpy.py\", line 72, in _run_code\n    exec code in run_globals\n  File \"/tmp/ansible_nxos_vlans_payload_mDCyj4/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py\", line 247, in <module>\n  File \"/tmp/ansible_nxos_vlans_payload_mDCyj4/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py\", line 242, in main\n  File \"/tmp/ansible_nxos_vlans_payload_mDCyj4/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 65, in execute_module\n  File \"/tmp/ansible_nxos_vlans_payload_mDCyj4/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 95, in set_config\n  File \"/tmp/ansible_nxos_vlans_payload_mDCyj4/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 115, in set_state\n  File \"/tmp/ansible_nxos_vlans_payload_mDCyj4/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 196, in _state_deleted\nTypeError: 'NoneType' object has no attribute '__getitem__'\n", 
    "module_stdout": "", 
    "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error", 
    "rc": 1
}
The full traceback is:
Traceback (most recent call last):
  File "<stdin>", line 102, in <module>
  File "<stdin>", line 94, in _ansiballz_main
  File "<stdin>", line 40, in invoke_module
  File "/usr/lib64/python2.7/runpy.py", line 180, in run_module
    fname, loader, pkg_name)
  File "/usr/lib64/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/tmp/ansible_nxos_vlans_payload_r3ruAN/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py", line 247, in <module>
  File "/tmp/ansible_nxos_vlans_payload_r3ruAN/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py", line 242, in main
  File "/tmp/ansible_nxos_vlans_payload_r3ruAN/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 65, in execute_module
  File "/tmp/ansible_nxos_vlans_payload_r3ruAN/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 95, in set_config
  File "/tmp/ansible_nxos_vlans_payload_r3ruAN/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 115, in set_state
  File "/tmp/ansible_nxos_vlans_payload_r3ruAN/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 196, in _state_deleted
TypeError: 'NoneType' object has no attribute '__getitem__'

failed: [n5kbottom] (item=20) => {
    "ansible_facts": {
        "discovered_interpreter_python": "/usr/bin/python"
    }, 
    "ansible_loop_var": "item", 
    "changed": false, 
    "item": 20, 
    "module_stderr": "Traceback (most recent call last):\n  File \"<stdin>\", line 102, in <module>\n  File \"<stdin>\", line 94, in _ansiballz_main\n  File \"<stdin>\", line 40, in invoke_module\n  File \"/usr/lib64/python2.7/runpy.py\", line 180, in run_module\n    fname, loader, pkg_name)\n  File \"/usr/lib64/python2.7/runpy.py\", line 72, in _run_code\n    exec code in run_globals\n  File \"/tmp/ansible_nxos_vlans_payload_r3ruAN/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py\", line 247, in <module>\n  File \"/tmp/ansible_nxos_vlans_payload_r3ruAN/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py\", line 242, in main\n  File \"/tmp/ansible_nxos_vlans_payload_r3ruAN/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 65, in execute_module\n  File \"/tmp/ansible_nxos_vlans_payload_r3ruAN/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 95, in set_config\n  File \"/tmp/ansible_nxos_vlans_payload_r3ruAN/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 115, in set_state\n  File \"/tmp/ansible_nxos_vlans_payload_r3ruAN/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 196, in _state_deleted\nTypeError: 'NoneType' object has no attribute '__getitem__'\n", 
    "module_stdout": "", 
    "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error", 
    "rc": 1
}
The full traceback is:
Traceback (most recent call last):
  File "<stdin>", line 102, in <module>
  File "<stdin>", line 94, in _ansiballz_main
  File "<stdin>", line 40, in invoke_module
  File "/usr/lib64/python2.7/runpy.py", line 180, in run_module
    fname, loader, pkg_name)
  File "/usr/lib64/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/tmp/ansible_nxos_vlans_payload_KKt_HZ/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py", line 247, in <module>
  File "/tmp/ansible_nxos_vlans_payload_KKt_HZ/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py", line 242, in main
  File "/tmp/ansible_nxos_vlans_payload_KKt_HZ/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 65, in execute_module
  File "/tmp/ansible_nxos_vlans_payload_KKt_HZ/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 95, in set_config
  File "/tmp/ansible_nxos_vlans_payload_KKt_HZ/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 115, in set_state
  File "/tmp/ansible_nxos_vlans_payload_KKt_HZ/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 196, in _state_deleted
TypeError: 'NoneType' object has no attribute '__getitem__'

failed: [cdcn5ktop] (item=20) => {
    "ansible_facts": {
        "discovered_interpreter_python": "/usr/bin/python"
    }, 
    "ansible_loop_var": "item", 
    "changed": false, 
    "item": 20, 
    "module_stderr": "Traceback (most recent call last):\n  File \"<stdin>\", line 102, in <module>\n  File \"<stdin>\", line 94, in _ansiballz_main\n  File \"<stdin>\", line 40, in invoke_module\n  File \"/usr/lib64/python2.7/runpy.py\", line 180, in run_module\n    fname, loader, pkg_name)\n  File \"/usr/lib64/python2.7/runpy.py\", line 72, in _run_code\n    exec code in run_globals\n  File \"/tmp/ansible_nxos_vlans_payload_KKt_HZ/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py\", line 247, in <module>\n  File \"/tmp/ansible_nxos_vlans_payload_KKt_HZ/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py\", line 242, in main\n  File \"/tmp/ansible_nxos_vlans_payload_KKt_HZ/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 65, in execute_module\n  File \"/tmp/ansible_nxos_vlans_payload_KKt_HZ/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 95, in set_config\n  File \"/tmp/ansible_nxos_vlans_payload_KKt_HZ/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 115, in set_state\n  File \"/tmp/ansible_nxos_vlans_payload_KKt_HZ/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 196, in _state_deleted\nTypeError: 'NoneType' object has no attribute '__getitem__'\n", 
    "module_stdout": "", 
    "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error", 
    "rc": 1
}
The full traceback is:
Traceback (most recent call last):
  File "<stdin>", line 102, in <module>
  File "<stdin>", line 94, in _ansiballz_main
  File "<stdin>", line 40, in invoke_module
  File "/usr/lib64/python2.7/runpy.py", line 180, in run_module
    fname, loader, pkg_name)
  File "/usr/lib64/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/tmp/ansible_nxos_vlans_payload_BVCetb/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py", line 247, in <module>
  File "/tmp/ansible_nxos_vlans_payload_BVCetb/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py", line 242, in main
  File "/tmp/ansible_nxos_vlans_payload_BVCetb/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 65, in execute_module
  File "/tmp/ansible_nxos_vlans_payload_BVCetb/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 95, in set_config
  File "/tmp/ansible_nxos_vlans_payload_BVCetb/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 115, in set_state
  File "/tmp/ansible_nxos_vlans_payload_BVCetb/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 196, in _state_deleted
TypeError: 'NoneType' object has no attribute '__getitem__'

failed: [cdcn5kbottom] (item=20) => {
    "ansible_facts": {
        "discovered_interpreter_python": "/usr/bin/python"
    }, 
    "ansible_loop_var": "item", 
    "changed": false, 
    "item": 20, 
    "module_stderr": "Traceback (most recent call last):\n  File \"<stdin>\", line 102, in <module>\n  File \"<stdin>\", line 94, in _ansiballz_main\n  File \"<stdin>\", line 40, in invoke_module\n  File \"/usr/lib64/python2.7/runpy.py\", line 180, in run_module\n    fname, loader, pkg_name)\n  File \"/usr/lib64/python2.7/runpy.py\", line 72, in _run_code\n    exec code in run_globals\n  File \"/tmp/ansible_nxos_vlans_payload_BVCetb/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py\", line 247, in <module>\n  File \"/tmp/ansible_nxos_vlans_payload_BVCetb/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py\", line 242, in main\n  File \"/tmp/ansible_nxos_vlans_payload_BVCetb/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 65, in execute_module\n  File \"/tmp/ansible_nxos_vlans_payload_BVCetb/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 95, in set_config\n  File \"/tmp/ansible_nxos_vlans_payload_BVCetb/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 115, in set_state\n  File \"/tmp/ansible_nxos_vlans_payload_BVCetb/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 196, in _state_deleted\nTypeError: 'NoneType' object has no attribute '__getitem__'\n", 
    "module_stdout": "", 
    "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error", 
    "rc": 1
}
The full traceback is:
Traceback (most recent call last):
  File "<stdin>", line 102, in <module>
  File "<stdin>", line 94, in _ansiballz_main
  File "<stdin>", line 40, in invoke_module
  File "/usr/lib64/python2.7/runpy.py", line 180, in run_module
    fname, loader, pkg_name)
  File "/usr/lib64/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/tmp/ansible_nxos_vlans_payload_iLGlVA/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py", line 247, in <module>
  File "/tmp/ansible_nxos_vlans_payload_iLGlVA/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py", line 242, in main
  File "/tmp/ansible_nxos_vlans_payload_iLGlVA/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 65, in execute_module
  File "/tmp/ansible_nxos_vlans_payload_iLGlVA/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 95, in set_config
  File "/tmp/ansible_nxos_vlans_payload_iLGlVA/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 115, in set_state
  File "/tmp/ansible_nxos_vlans_payload_iLGlVA/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 196, in _state_deleted
TypeError: 'NoneType' object has no attribute '__getitem__'

failed: [paris] (item=20) => {
    "ansible_facts": {
        "discovered_interpreter_python": "/usr/bin/python"
    }, 
    "ansible_loop_var": "item", 
    "changed": false, 
    "item": 20, 
    "module_stderr": "Traceback (most recent call last):\n  File \"<stdin>\", line 102, in <module>\n  File \"<stdin>\", line 94, in _ansiballz_main\n  File \"<stdin>\", line 40, in invoke_module\n  File \"/usr/lib64/python2.7/runpy.py\", line 180, in run_module\n    fname, loader, pkg_name)\n  File \"/usr/lib64/python2.7/runpy.py\", line 72, in _run_code\n    exec code in run_globals\n  File \"/tmp/ansible_nxos_vlans_payload_iLGlVA/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py\", line 247, in <module>\n  File \"/tmp/ansible_nxos_vlans_payload_iLGlVA/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py\", line 242, in main\n  File \"/tmp/ansible_nxos_vlans_payload_iLGlVA/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 65, in execute_module\n  File \"/tmp/ansible_nxos_vlans_payload_iLGlVA/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 95, in set_config\n  File \"/tmp/ansible_nxos_vlans_payload_iLGlVA/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 115, in set_state\n  File \"/tmp/ansible_nxos_vlans_payload_iLGlVA/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 196, in _state_deleted\nTypeError: 'NoneType' object has no attribute '__getitem__'\n", 
    "module_stdout": "", 
    "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error", 
    "rc": 1
}
The full traceback is:
Traceback (most recent call last):
  File "<stdin>", line 102, in <module>
  File "<stdin>", line 94, in _ansiballz_main
  File "<stdin>", line 40, in invoke_module
  File "/usr/lib64/python2.7/runpy.py", line 180, in run_module
    fname, loader, pkg_name)
  File "/usr/lib64/python2.7/runpy.py", line 72, in _run_code
    exec code in run_globals
  File "/tmp/ansible_nxos_vlans_payload_C_1OAm/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py", line 247, in <module>
  File "/tmp/ansible_nxos_vlans_payload_C_1OAm/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py", line 242, in main
  File "/tmp/ansible_nxos_vlans_payload_C_1OAm/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 65, in execute_module
  File "/tmp/ansible_nxos_vlans_payload_C_1OAm/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 95, in set_config
  File "/tmp/ansible_nxos_vlans_payload_C_1OAm/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 115, in set_state
  File "/tmp/ansible_nxos_vlans_payload_C_1OAm/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py", line 196, in _state_deleted
TypeError: 'NoneType' object has no attribute '__getitem__'

failed: [aria] (item=20) => {
    "ansible_facts": {
        "discovered_interpreter_python": "/usr/bin/python"
    }, 
    "ansible_loop_var": "item", 
    "changed": false, 
    "item": 20, 
    "module_stderr": "Traceback (most recent call last):\n  File \"<stdin>\", line 102, in <module>\n  File \"<stdin>\", line 94, in _ansiballz_main\n  File \"<stdin>\", line 40, in invoke_module\n  File \"/usr/lib64/python2.7/runpy.py\", line 180, in run_module\n    fname, loader, pkg_name)\n  File \"/usr/lib64/python2.7/runpy.py\", line 72, in _run_code\n    exec code in run_globals\n  File \"/tmp/ansible_nxos_vlans_payload_C_1OAm/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py\", line 247, in <module>\n  File \"/tmp/ansible_nxos_vlans_payload_C_1OAm/ansible_nxos_vlans_payload.zip/ansible/modules/network/nxos/nxos_vlans.py\", line 242, in main\n  File \"/tmp/ansible_nxos_vlans_payload_C_1OAm/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 65, in execute_module\n  File \"/tmp/ansible_nxos_vlans_payload_C_1OAm/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 95, in set_config\n  File \"/tmp/ansible_nxos_vlans_payload_C_1OAm/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 115, in set_state\n  File \"/tmp/ansible_nxos_vlans_payload_C_1OAm/ansible_nxos_vlans_payload.zip/ansible/module_utils/network/nxos/config/vlans/vlans.py\", line 196, in _state_deleted\nTypeError: 'NoneType' object has no attribute '__getitem__'\n", 
    "module_stdout": "", 
    "msg": "MODULE FAILURE\nSee stdout/stderr for the exact error", 
    "rc": 1
}

PLAY RECAP ****************************************************************************************************************************************************************************************
aria                       : ok=0    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0   
cdcn5kbottom               : ok=0    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0   
cdcn5ktop                  : ok=0    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0   
n5kbottom                  : ok=0    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0   
n5ktop                     : ok=0    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0   
paris                      : ok=0    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0   
```

Hints from issue discussion:

Commit message: whitespace fixes 2

Patch to review:
diff --git a/lib/ansible/module_utils/network/nxos/config/vlans/vlans.py b/lib/ansible/module_utils/network/nxos/config/vlans/vlans.py
index 5bd423c01039a4..20774770f0a501 100644
--- a/lib/ansible/module_utils/network/nxos/config/vlans/vlans.py
+++ b/lib/ansible/module_utils/network/nxos/config/vlans/vlans.py
@@ -34,8 +34,6 @@ class Vlans(ConfigBase):
         'vlans',
     ]
 
-    exclude_params = ['name', 'state']
-
     def __init__(self, module):
         super(Vlans, self).__init__(module)
 
@@ -49,8 +47,17 @@ def get_vlans_facts(self):
         vlans_facts = facts['ansible_network_resources'].get('vlans')
         if not vlans_facts:
             return []
+
+        # Remove vlan 1 from facts list
+        vlans_facts = [i for i in vlans_facts if (i['vlan_id']) != 1]
         return vlans_facts
 
+    def edit_config(self, commands):
+        """Wrapper method for `_connection.edit_config()`
+        This exists solely to allow the unit test framework to mock device connection calls.
+        """
+        return self._connection.edit_config(commands)
+
     def execute_module(self):
         """ Execute the module
 
@@ -65,7 +72,7 @@ def execute_module(self):
         commands.extend(self.set_config(existing_vlans_facts))
         if commands:
             if not self._module.check_mode:
-                self._connection.edit_config(commands)
+                self.edit_config(commands)
             result['changed'] = True
         result['commands'] = commands
 
@@ -90,6 +97,8 @@ def set_config(self, existing_vlans_facts):
         want = []
         if config:
             for w in config:
+                if w.get('vlan_id') == '1':
+                    self._module.fail_json(msg="Vlan 1 is not allowed to be managed by this module")
                 want.append(remove_empties(w))
         have = existing_vlans_facts
         resp = self.set_state(want, have)
@@ -121,57 +130,78 @@ def set_state(self, want, have):
                     commands.extend(self._state_replaced(w, have))
         return commands
 
-    def _state_replaced(self, w, have):
-        """ The command generator when state is replaced
+    def remove_default_states(self, obj):
+        """Removes non-empty but default states from the obj.
+        """
+        default_states = {
+            'enabled': True,
+            'state': 'active',
+            'mode': 'ce',
+        }
+        for k in default_states.keys():
+            if obj[k] == default_states[k]:
+                obj.pop(k, None)
+        return obj
 
+    def _state_replaced(self, w, have):
+        """ The command generator when state is replaced.
+        Scope is limited to vlan objects defined in the playbook.
         :rtype: A list
-        :returns: the commands necessary to migrate the current configuration
-                  to the desired configuration
+        :returns: The minimum command set required to migrate the current
+                  configuration to the desired configuration.
         """
-        commands = []
         obj_in_have = search_obj_in_list(w['vlan_id'], have, 'vlan_id')
-        diff = dict_diff(w, obj_in_have)
-        merged_commands = self.set_commands(w, have)
-        if 'vlan_id' not in diff:
-            diff['vlan_id'] = w['vlan_id']
-        wkeys = w.keys()
-        dkeys = diff.keys()
-        for k in wkeys:
-            if k in self.exclude_params and k in dkeys:
-                del diff[k]
-        replaced_commands = self.del_attribs(diff)
-
-        if merged_commands:
-            cmds = set(replaced_commands).intersection(set(merged_commands))
-            for cmd in cmds:
-                merged_commands.remove(cmd)
-            commands.extend(replaced_commands)
-            commands.extend(merged_commands)
-        return commands
+        if obj_in_have:
+            # ignore states that are already reset, then diff what's left
+            obj_in_have = self.remove_default_states(obj_in_have)
+            diff = dict_diff(w, obj_in_have)
+            # Remove merge items from diff; what's left will be used to
+            # remove states not specified in the playbook
+            for k in dict(set(w.items()) - set(obj_in_have.items())).keys():
+                diff.pop(k, None)
+        else:
+            diff = w
+
+        # merged_cmds: 'want' cmds to update 'have' states that don't match
+        # replaced_cmds: remaining 'have' cmds that need to be reset to default
+        merged_cmds = self.set_commands(w, have)
+        replaced_cmds = []
+        if obj_in_have:
+            # Remaining diff items are used to reset states to default
+            replaced_cmds = self.del_attribs(diff)
+        cmds = []
+        if replaced_cmds or merged_cmds:
+            cmds += ['vlan %s' % str(w['vlan_id'])]
+            cmds += merged_cmds + replaced_cmds
+        return cmds
 
     def _state_overridden(self, want, have):
-        """ The command generator when state is overridden
-
+        """ The command generator when state is overridden.
+        Scope includes all vlan objects on the device.
         :rtype: A list
-        :returns: the commands necessary to migrate the current configuration
-                  to the desired configuration
+        :returns: the minimum command set required to migrate the current
+                  configuration to the desired configuration.
         """
-        commands = []
+        # overridden behavior is the same as replaced except for scope.
+        cmds = []
+        existing_vlans = []
         for h in have:
+            existing_vlans.append(h['vlan_id'])
             obj_in_want = search_obj_in_list(h['vlan_id'], want, 'vlan_id')
-            if h == obj_in_want:
-                continue
-            for w in want:
-                if h['vlan_id'] == w['vlan_id']:
-                    wkeys = w.keys()
-                    hkeys = h.keys()
-                    for k in wkeys:
-                        if k in self.exclude_params and k in hkeys:
-                            del h[k]
-            commands.extend(self.del_attribs(h))
+            if obj_in_want:
+                if h != obj_in_want:
+                    replaced_cmds = self._state_replaced(obj_in_want, [h])
+                    if replaced_cmds:
+                        cmds.extend(replaced_cmds)
+            else:
+                cmds.append('no vlan %s' % h['vlan_id'])
+
+        # Add wanted vlans that don't exist on the device yet
         for w in want:
-            commands.extend(self.set_commands(w, have))
-        return commands
+            if w['vlan_id'] not in existing_vlans:
+                new_vlan = ['vlan %s' % w['vlan_id']]
+                cmds.extend(new_vlan + self.add_commands(w))
+        return cmds
 
     def _state_merged(self, w, have):
         """ The command generator when state is merged
@@ -180,7 +210,10 @@ def _state_merged(self, w, have):
         :returns: the commands necessary to merge the provided into
                   the current configuration
         """
-        return self.set_commands(w, have)
+        cmds = self.set_commands(w, have)
+        if cmds:
+            cmds.insert(0, 'vlan %s' % str(w['vlan_id']))
+        return(cmds)
 
     def _state_deleted(self, want, have):
         """ The command generator when state is deleted
@@ -193,7 +226,8 @@ def _state_deleted(self, want, have):
         if want:
             for w in want:
                 obj_in_have = search_obj_in_list(w['vlan_id'], have, 'vlan_id')
-                commands.append('no vlan ' + str(obj_in_have['vlan_id']))
+                if obj_in_have:
+                    commands.append('no vlan ' + str(obj_in_have['vlan_id']))
         else:
             if not have:
                 return commands
@@ -202,20 +236,20 @@ def _state_deleted(self, want, have):
         return commands
 
     def del_attribs(self, obj):
+        """Returns a list of commands to reset states to default
+        """
         commands = []
-        if not obj or len(obj.keys()) == 1:
+        if not obj:
             return commands
-        commands.append('vlan ' + str(obj['vlan_id']))
-        if 'name' in obj:
-            commands.append('no' + ' ' + 'name')
-        if 'state' in obj:
-            commands.append('no state')
-        if 'enabled' in obj:
-            commands.append('no shutdown')
-        if 'mode' in obj:
-            commands.append('mode ce')
-        if 'mapped_vni' in obj:
-            commands.append('no vn-segment')
+        default_cmds = {
+            'name': 'no name',
+            'state': 'no state',
+            'enabled': 'no shutdown',
+            'mode': 'mode ce',
+            'mapped_vni': 'no vn-segment',
+        }
+        for k in obj:
+            commands.append(default_cmds[k])
         return commands
 
     def diff_of_dicts(self, w, obj):
@@ -229,20 +263,19 @@ def add_commands(self, d):
         commands = []
         if not d:
             return commands
-        commands.append('vlan' + ' ' + str(d['vlan_id']))
         if 'name' in d:
             commands.append('name ' + d['name'])
         if 'state' in d:
             commands.append('state ' + d['state'])
         if 'enabled' in d:
-            if d['enabled'] == 'True':
+            if d['enabled'] is True:
                 commands.append('no shutdown')
             else:
                 commands.append('shutdown')
         if 'mode' in d:
             commands.append('mode ' + d['mode'])
         if 'mapped_vni' in d:
-            commands.append('vn-segment ' + d['mapped_vni'])
+            commands.append('vn-segment %s' % d['mapped_vni'])
 
         return commands
 
diff --git a/lib/ansible/module_utils/network/nxos/facts/vlans/vlans.py b/lib/ansible/module_utils/network/nxos/facts/vlans/vlans.py
index 924a20f9a48eb6..db4dfef094d055 100644
--- a/lib/ansible/module_utils/network/nxos/facts/vlans/vlans.py
+++ b/lib/ansible/module_utils/network/nxos/facts/vlans/vlans.py
@@ -13,6 +13,7 @@
 __metaclass__ = type
 
 import re
+import ast
 from copy import deepcopy
 
 from ansible.module_utils.network.common import utils
@@ -38,6 +39,12 @@ def __init__(self, module, subspec='config', options='options'):
 
         self.generated_spec = utils.generate_dict(facts_argument_spec)
 
+    def get_device_data(self, connection, show_cmd):
+        """Wrapper method for `connection.get()`
+        This exists solely to allow the unit test framework to mock device connection calls.
+        """
+        return connection.get(show_cmd)
+
     def populate_facts(self, connection, ansible_facts, data=None):
         """ Populate the facts for vlans
         :param connection: the device connection
@@ -47,24 +54,18 @@ def populate_facts(self, connection, ansible_facts, data=None):
         """
         objs = []
         if not data:
-            data = connection.get('show running-config | section ^vlan')
-        vlans = re.split(r'(,|-)', data.split()[1])
-        for v in vlans:
-            if not v.isdigit():
-                vlans.remove(v)
-
-        config = re.split(r'(^|\n)vlan', data)
-        for conf in config:
-            conf = conf.strip()
-            if conf:
-                if conf[0] in vlans:
-                    vlans.remove(conf[0])
-                    obj = self.render_config(self.generated_spec, conf)
-                    if obj and len(obj.keys()) > 1:
-                        objs.append(obj)
-
-        for v in vlans:
-            obj = self.render_config(self.generated_spec, v)
+            # Use structured for most of the vlan parameter states.
+            # This data is consistent across the supported nxos platforms.
+            structured = self.get_device_data(connection, 'show vlan | json')
+
+            # Raw cli config is needed for mapped_vni, which is not included in structured.
+            run_cfg_output = self.get_device_data(connection, 'show running-config | section ^vlan')
+
+            # Create a single dictionary from all data sources
+            data = self.normalize_table_data(structured, run_cfg_output)
+
+        for vlan in data:
+            obj = self.render_config(self.generated_spec, vlan)
             if obj:
                 objs.append(obj)
 
@@ -75,36 +76,86 @@ def populate_facts(self, connection, ansible_facts, data=None):
             params = utils.validate_config(self.argument_spec, {'config': objs})
             for cfg in params['config']:
                 facts['vlans'].append(utils.remove_empties(cfg))
-
         ansible_facts['ansible_network_resources'].update(facts)
         return ansible_facts
 
-    def render_config(self, spec, conf):
+    def render_config(self, spec, vlan):
         """
         Render config as dictionary structure and delete keys
           from spec for null values
         :param spec: The facts tree, generated from the argspec
-        :param conf: The configuration
+        :param vlan: structured data vlan settings (dict) and raw cfg from device
         :rtype: dictionary
         :returns: The generated config
+        Sample inputs: test/units/modules/network/nxos/fixtures/nxos_vlans/show_vlan
         """
-        config = deepcopy(spec)
-        if len(conf) == 1:
-            return utils.remove_empties({'vlan_id': conf})
-
-        match = re.search(r'^(\S+)?', conf, re.M)
-        if match:
-            if len(match.group(1)) == 1:
-                config['vlan_id'] = match.group(1)
-                config['name'] = parse_conf_arg(conf, 'name')
-                config['mode'] = parse_conf_arg(conf, 'mode')
-                config['mapped_vni'] = parse_conf_arg(conf, 'vn-segment')
-                config['state'] = parse_conf_arg(conf, 'state')
-                admin_state = parse_conf_cmd_arg(conf, 'shutdown', 'down', 'up')
-                if admin_state == 'up':
-                    config['enabled'] = True
-                elif admin_state == 'down':
-                    config['enabled'] = False
-
-        vlans_cfg = utils.remove_empties(config)
-        return vlans_cfg
+        obj = deepcopy(spec)
+
+        obj['vlan_id'] = vlan['vlan_id']
+
+        # name: 'VLAN000x' (default name) or custom name
+        name = vlan['vlanshowbr-vlanname']
+        if name and re.match("VLAN%04d" % int(vlan['vlan_id']), name):
+            name = None
+        obj['name'] = name
+
+        # mode: 'ce-vlan' or 'fabricpath-vlan'
+        obj['mode'] = vlan['vlanshowinfo-vlanmode'].replace('-vlan', '')
+
+        # enabled: shutdown, noshutdown
+        obj['enabled'] = True if 'noshutdown' in vlan['vlanshowbr-shutstate'] else False
+
+        # state: active, suspend
+        obj['state'] = vlan['vlanshowbr-vlanstate']
+
+        # non-structured data
+        obj['mapped_vni'] = parse_conf_arg(vlan['run_cfg'], 'vn-segment')
+
+        return utils.remove_empties(obj)
+
+    def normalize_table_data(self, structured, run_cfg_output):
+        """Normalize structured output and raw running-config output into
+        a single dict to simplify render_config usage.
+        This is needed because:
+        - The NXOS devices report most of the vlan settings within two
+          structured data keys: 'vlanbrief' and 'mtuinfo', but the output is
+          incomplete and therefore raw running-config data is also needed.
+        - running-config by itself is insufficient because of major differences
+          in the cli config syntax across platforms.
+        - Thus a helper method combines settings from the separate top-level keys,
+          and adds a 'run_cfg' key containing raw cli from the device.
+        """
+        # device output may be string, convert to list
+        structured = ast.literal_eval(str(structured))
+
+        # SAMPLE: {"TABLE_vlanbriefid": {"ROW_vlanbriefid": {
+        #   "vlanshowbr-vlanid": "4", "vlanshowbr-vlanid-utf": "4",
+        #   "vlanshowbr-vlanname": "VLAN0004", "vlanshowbr-vlanstate": "active",
+        #   "vlanshowbr-shutstate": "noshutdown"}},
+        vlanbrief = structured['TABLE_vlanbrief']['ROW_vlanbrief']
+
+        # SAMPLE: "TABLE_mtuinfoid": {"ROW_mtuinfoid": {
+        #   "vlanshowinfo-vlanid": "4", "vlanshowinfo-media-type": "enet",
+        #   "vlanshowinfo-vlanmode": "ce-vlan"}}
+        mtuinfo = structured['TABLE_mtuinfo']['ROW_mtuinfo']
+
+        if type(vlanbrief) is not list:
+            vlanbrief = [vlanbrief]
+            mtuinfo = [mtuinfo]
+
+        # split out any per-vlan cli config
+        run_cfg_list = re.split(r'[\n^]vlan ', run_cfg_output)
+
+        # Create a list of vlan dicts where each dict contains vlanbrief,
+        # mtuinfo, and non-structured running-config data for one vlan.
+        vlans = []
+        for index, v in enumerate(vlanbrief):
+            v['vlan_id'] = v.get('vlanshowbr-vlanid-utf')
+            vlan = {}
+            vlan.update(v)
+            vlan.update(mtuinfo[index])
+
+            run_cfg = [i for i in run_cfg_list if "%s\n" % v['vlan_id'] in i] or ['']
+            vlan['run_cfg'] = run_cfg.pop()
+            vlans.append(vlan)
+        return vlans
diff --git a/test/integration/targets/nxos_vlans/tests/cli/deleted.yaml b/test/integration/targets/nxos_vlans/tests/cli/deleted.yaml
index e346a5eb2c1c63..e3e8de7d068e25 100644
--- a/test/integration/targets/nxos_vlans/tests/cli/deleted.yaml
+++ b/test/integration/targets/nxos_vlans/tests/cli/deleted.yaml
@@ -5,6 +5,7 @@
 - name: setup
   cli_config:
     config: |
+      no vlan 2-100
       vlan 5
       vlan 6
 
@@ -23,7 +24,7 @@
 
   - assert:
       that:
-        - "ansible_facts.network_resources.vlans|symmetric_difference(result.before)|length == 0"
+        - "result.before|length == (ansible_facts.network_resources.vlans|length - 1)"
         - "result.after|length == 0"
         - "result.changed == true"
         - "'no vlan 5' in result.commands"
diff --git a/test/integration/targets/nxos_vlans/tests/cli/merged.yaml b/test/integration/targets/nxos_vlans/tests/cli/merged.yaml
index fd3ad7a011e180..88000fbca74a6a 100644
--- a/test/integration/targets/nxos_vlans/tests/cli/merged.yaml
+++ b/test/integration/targets/nxos_vlans/tests/cli/merged.yaml
@@ -5,8 +5,7 @@
 - name: setup
   cli_config: &cleanup
     config: |
-      no vlan 5
-      no vlan 6
+      no vlan 2-100
 
 - block:
   - name: Merged
@@ -40,7 +39,7 @@
 
   - assert:
       that:
-        - "ansible_facts.network_resources.vlans|symmetric_difference(result.after)|length == 0"
+        - "result.after|length == (ansible_facts.network_resources.vlans|length - 1)"
 
   - name: Idempotence - Merged
     nxos_vlans: *merged
diff --git a/test/integration/targets/nxos_vlans/tests/cli/overridden.yaml b/test/integration/targets/nxos_vlans/tests/cli/overridden.yaml
index faee7979cfd390..ad0760ea4a9357 100644
--- a/test/integration/targets/nxos_vlans/tests/cli/overridden.yaml
+++ b/test/integration/targets/nxos_vlans/tests/cli/overridden.yaml
@@ -5,18 +5,17 @@
 - name: setup1
   cli_config: &cleanup
     config: |
-      no vlan 5
-      no vlan 6
-      no vlan 9
+      no vlan 2-100
 
 - block:
   - name: setup
     cli_config:
       config: |
         vlan 5
-        name test-vlan5
+          name test-vlan5
         state suspend
-        vlan 6
+          vlan 6
+          exit
 
   - name: Gather vlans facts
     nxos_facts: &facts
@@ -36,22 +35,21 @@
 
   - assert:
       that:
-        - "ansible_facts.network_resources.vlans|symmetric_difference(result.before)|length == 0"
+        - "result.before|length == (ansible_facts.network_resources.vlans|length - 1)"
         - "result.changed == true"
-        - "'vlan 5' in result.commands"
-        - "'no name' in result.commands"
-        - "'no state' in result.commands"
+        - "'no vlan 5' in result.commands"
+        - "'no vlan 6' in result.commands"
         - "'vlan 9' in result.commands"
         - "'name test-vlan9' in result.commands"
         - "'shutdown' in result.commands"
-        - "result.commands|length == 6"
+        - "result.commands|length == 5"
 
   - name: Gather vlans post facts
     nxos_facts: *facts
 
   - assert:
       that:
-        - "ansible_facts.network_resources.vlans|symmetric_difference(result.after)|length == 0"
+        - "result.after|length == (ansible_facts.network_resources.vlans|length - 1)"
 
   - name: Idempotence - Overridden
     nxos_vlans: *overridden
diff --git a/test/integration/targets/nxos_vlans/tests/cli/replaced.yaml b/test/integration/targets/nxos_vlans/tests/cli/replaced.yaml
index af0ef91da452d7..e55c1dd2e1049c 100644
--- a/test/integration/targets/nxos_vlans/tests/cli/replaced.yaml
+++ b/test/integration/targets/nxos_vlans/tests/cli/replaced.yaml
@@ -5,17 +5,17 @@
 - name: setup1
   cli_config: &cleanup
     config: |
-      no vlan 5
-      no vlan 6
+      no vlan 2-100
 
 - block:
   - name: setup2
     cli_config:
       config: |
         vlan 5
-        name test-vlan5
+          name test-vlan5
         vlan 6
-        name test-vlan6
+          name test-vlan6
+          exit
 
   - name: Gather vlans facts
     nxos_facts: &facts
@@ -34,7 +34,7 @@
 
   - assert:
       that:
-        - "ansible_facts.network_resources.vlans|symmetric_difference(result.before)|length == 0"
+        - "result.before|length == (ansible_facts.network_resources.vlans|length - 1)"
         - "result.changed == true"
         - "'vlan 6' in result.commands"
         - "'no name' in result.commands"
@@ -46,7 +46,7 @@
 
   - assert:
       that:
-        - "ansible_facts.network_resources.vlans|symmetric_difference(result.after)|length == 0"
+        - "result.after|length == (ansible_facts.network_resources.vlans|length - 1)"
 
   - name: Idempotence - Replaced
     nxos_vlans: *replaced
diff --git a/test/units/modules/network/nxos/fixtures/nxos_vlans/save b/test/units/modules/network/nxos/fixtures/nxos_vlans/save
new file mode 100644
index 00000000000000..95083afb4f0d72
--- /dev/null
+++ b/test/units/modules/network/nxos/fixtures/nxos_vlans/save
@@ -0,0 +1,3 @@
+            #structured = '{"TABLE_vlanbrief": {"ROW_vlanbrief": [{"vlanshowbr-vlanid": "1", "vlanshowbr-vlanid-utf": "1", "vlanshowbr-vlanname": "default", "vlanshowbr-vlanstate": "active", "vlanshowbr-shutstate": "noshutdown"}, {"vlanshowbr-vlanid": "3", "vlanshowbr-vlanid-utf": "3", "vlanshowbr-vlanname": "VLAN0003", "vlanshowbr-vlanstate": "active", "vlanshowbr-shutstate": "noshutdown"}, {"vlanshowbr-vlanid": "4", "vlanshowbr-vlanid-utf": "4", "vlanshowbr-vlanname": "vlan4", "vlanshowbr-vlanstate": "active", "vlanshowbr-shutstate": "noshutdown"}, {"vlanshowbr-vlanid": "5", "vlanshowbr-vlanid-utf": "5", "vlanshowbr-vlanname": "vlan5", "vlanshowbr-vlanstate": "active", "vlanshowbr-shutstate": "noshutdown"}]}, "TABLE_mtuinfo": {"ROW_mtuinfo": [{"vlanshowinfo-vlanid": "1", "vlanshowinfo-media-type": "enet", "vlanshowinfo-vlanmode": "ce-vlan"}, {"vlanshowinfo-vlanid": "3", "vlanshowinfo-media-type": "enet", "vlanshowinfo-vlanmode": "ce-vlan"}, {"vlanshowinfo-vlanid": "4", "vlanshowinfo-media-type": "enet", "vlanshowinfo-vlanmode": "ce-vlan"}, {"vlanshowinfo-vlanid": "5", "vlanshowinfo-media-type": "enet", "vlanshowinfo-vlanmode": "ce-vlan"}]}}'
+            #run_cfg_output = 'vlan 1,3-5\nvlan 4\n  name vlan4\n  vn-segment 921\nvlan 5\n  name vlan5'
+
diff --git a/test/units/modules/network/nxos/fixtures/nxos_vlans/show_running-config b/test/units/modules/network/nxos/fixtures/nxos_vlans/show_running-config
new file mode 100644
index 00000000000000..7f7c881fe3e29f
--- /dev/null
+++ b/test/units/modules/network/nxos/fixtures/nxos_vlans/show_running-config
@@ -0,0 +1,11 @@
+vlan 1,3-5
+vlan 3
+  name test-vlan3
+vlan 5
+  shutdown
+  name test-changeme
+  mode fabricpath
+  state suspend
+  vn-segment 942
+vlan 8
+  shutdown
diff --git a/test/units/modules/network/nxos/fixtures/nxos_vlans/show_vlan b/test/units/modules/network/nxos/fixtures/nxos_vlans/show_vlan
new file mode 100644
index 00000000000000..2295a67d20ba0f
--- /dev/null
+++ b/test/units/modules/network/nxos/fixtures/nxos_vlans/show_vlan
@@ -0,0 +1,45 @@
+{
+  "TABLE_vlanbrief": {
+    "ROW_vlanbrief": [
+      { "vlanshowbr-vlanid": "1", "vlanshowbr-vlanid-utf": "1",
+        "vlanshowbr-vlanname": "default",
+        "vlanshowbr-vlanstate": "active",
+        "vlanshowbr-shutstate": "noshutdown"
+      },
+      { "vlanshowbr-vlanid": "3", "vlanshowbr-vlanid-utf": "3",
+        "vlanshowbr-vlanname": "test-vlan3",
+        "vlanshowbr-vlanstate": "active",
+        "vlanshowbr-shutstate": "noshutdown"
+      },
+      { "vlanshowbr-vlanid": "4", "vlanshowbr-vlanid-utf": "4",
+        "vlanshowbr-vlanname": "VLAN0004",
+        "vlanshowbr-vlanstate": "active",
+        "vlanshowbr-shutstate": "noshutdown"
+      },
+      { "vlanshowbr-vlanid": "5", "vlanshowbr-vlanid-utf": "5",
+        "vlanshowbr-vlanname": "test-changeme",
+        "vlanshowbr-vlanstate": "suspend",
+        "vlanshowbr-shutstate": "shutdown"
+      },
+      { "vlanshowbr-vlanid": "8", "vlanshowbr-vlanid-utf": "8",
+        "vlanshowbr-vlanname": "VLAN0008",
+        "vlanshowbr-vlanstate": "active",
+        "vlanshowbr-shutstate": "shutdown"
+      }
+    ]
+  },
+  "TABLE_mtuinfo": {
+    "ROW_mtuinfo": [
+      { "vlanshowinfo-vlanid": "1", "vlanshowinfo-media-type": "enet",
+        "vlanshowinfo-vlanmode": "ce-vlan" },
+      { "vlanshowinfo-vlanid": "3", "vlanshowinfo-media-type": "enet",
+        "vlanshowinfo-vlanmode": "ce-vlan" },
+      { "vlanshowinfo-vlanid": "4", "vlanshowinfo-media-type": "enet",
+        "vlanshowinfo-vlanmode": "ce-vlan" },
+      { "vlanshowinfo-vlanid": "5", "vlanshowinfo-media-type": "enet",
+        "vlanshowinfo-vlanmode": "fabricpath-vlan" },
+      { "vlanshowinfo-vlanid": "8", "vlanshowinfo-media-type": "enet",
+        "vlanshowinfo-vlanmode": "ce-vlan" }
+    ]
+  }
+}
diff --git a/test/units/modules/network/nxos/test_nxos_vlans.py b/test/units/modules/network/nxos/test_nxos_vlans.py
new file mode 100644
index 00000000000000..fb63968d08a3de
--- /dev/null
+++ b/test/units/modules/network/nxos/test_nxos_vlans.py
@@ -0,0 +1,138 @@
+# (c) 2019 Red Hat Inc.
+#
+# 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/>.
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+from textwrap import dedent
+from units.compat.mock import patch
+from units.modules.utils import AnsibleFailJson
+from ansible.modules.network.nxos import nxos_vlans
+from ansible.module_utils.network.nxos.config.vlans.vlans import Vlans
+from .nxos_module import TestNxosModule, load_fixture, set_module_args
+
+ignore_provider_arg = True
+
+
+class TestNxosVlansModule(TestNxosModule):
+
+    module = nxos_vlans
+
+    def setUp(self):
+        super(TestNxosVlansModule, self).setUp()
+
+        self.mock_FACT_LEGACY_SUBSETS = patch('ansible.module_utils.network.nxos.facts.facts.FACT_LEGACY_SUBSETS')
+        self.FACT_LEGACY_SUBSETS = self.mock_FACT_LEGACY_SUBSETS.start()
+
+        self.mock_get_resource_connection_config = patch('ansible.module_utils.network.common.cfg.base.get_resource_connection')
+        self.get_resource_connection_config = self.mock_get_resource_connection_config.start()
+
+        self.mock_get_resource_connection_facts = patch('ansible.module_utils.network.common.facts.facts.get_resource_connection')
+        self.get_resource_connection_facts = self.mock_get_resource_connection_facts.start()
+
+        self.mock_edit_config = patch('ansible.module_utils.network.nxos.config.vlans.vlans.Vlans.edit_config')
+        self.edit_config = self.mock_edit_config.start()
+
+        self.mock_get_device_data = patch('ansible.module_utils.network.nxos.facts.vlans.vlans.VlansFacts.get_device_data')
+        self.get_device_data = self.mock_get_device_data.start()
+
+    def tearDown(self):
+        super(TestNxosVlansModule, self).tearDown()
+        self.mock_FACT_LEGACY_SUBSETS.stop()
+        self.mock_get_resource_connection_config.stop()
+        self.mock_get_resource_connection_facts.stop()
+        self.mock_edit_config.stop()
+
+    def load_fixtures(self, commands=None, device=''):
+        self.mock_FACT_LEGACY_SUBSETS.return_value = dict()
+        self.edit_config.return_value = None
+
+        def load_from_file(*args, **kwargs):
+            cmd = args[1]
+            filename = str(cmd).split(' | ')[0].replace(' ', '_')
+            return load_fixture('nxos_vlans', filename)
+        self.get_device_data.side_effect = load_from_file
+
+    def test_1(self):
+        '''
+        vlan 1,3-5
+        vlan 3
+          name test-vlan3
+        !Note:vlan 4 is present with default settings
+        vlan 5
+          shutdown
+          name test-changeme
+          mode fabricpath
+          state suspend
+          vn-segment 942
+        !Note:vlan 7 is not present
+        vlan 8
+          shutdown
+        '''
+        playbook = dict(config=[
+            dict(vlan_id=4),
+            dict(vlan_id=5, mapped_vni=555, mode='ce'),
+            dict(vlan_id=7, mapped_vni=777, name='test-vlan7', enabled=False),
+            dict(vlan_id=8)
+            # vlan 3 is not present in playbook.
+        ])
+
+        merged = [
+            # Update existing device states with any differences in the playbook.
+            'vlan 5', 'vn-segment 555', 'mode ce',
+            'vlan 7', 'vn-segment 777', 'name test-vlan7', 'shutdown'
+        ]
+        playbook['state'] = 'merged'
+        set_module_args(playbook, ignore_provider_arg)
+        self.execute_module(changed=True, commands=merged)
+
+        deleted = [
+            # Reset existing device state to default values. Scope is limited to
+            # objects in the play.For vlans this means deleting each vlan listed
+            # in the playbook. Ignores any play attrs other than 'vlan_id'.
+            'no vlan 4',
+            'no vlan 5',
+            'no vlan 8'
+        ]
+        playbook['state'] = 'deleted'
+        set_module_args(playbook, ignore_provider_arg)
+        self.execute_module(changed=True, commands=deleted)
+
+        overridden = [
+            # The play is the source of truth. Similar to replaced but the scope
+            # includes all objects on the device; i.e. it will also reset state
+            # on objects not found in the play.
+            'no vlan 3',
+            'vlan 5', 'mode ce', 'vn-segment 555', 'no state', 'no shutdown', 'no name',
+            'vlan 8', 'no shutdown',
+            'vlan 7', 'name test-vlan7', 'shutdown', 'vn-segment 777'
+        ]
+        playbook['state'] = 'overridden'
+        set_module_args(playbook, ignore_provider_arg)
+        self.execute_module(changed=True, commands=overridden)
+
+        replaced = [
+            # Scope is limited to objects in the play.
+            # replaced should ignore existing vlan 3.
+            'vlan 5', 'vn-segment 555', 'no name', 'no state', 'no shutdown', 'mode ce',
+            'vlan 7', 'shutdown', 'name test-vlan7', 'vn-segment 777',
+            'vlan 8', 'no shutdown'
+        ]
+        playbook['state'] = 'replaced'
+        set_module_args(playbook, ignore_provider_arg)
+        self.execute_module(changed=True, commands=replaced)


Code review comments:
diff_hunk: @@ -90,6 +97,8 @@ def set_config(self, existing_vlans_facts):
         want = []
         if config:
             for w in config:
+                if w.get('vlan_id') == '1':
path: lib/ansible/module_utils/network/nxos/config/vlans/vlans.py
text: @user1:
Are we sure that vlan_id here will always be a string or could it be an int.

@author:
Nope, I'll fix.

diff_hunk: @@ -0,0 +1,138 @@
+# (c) 2019 Red Hat Inc.
+#
+# 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/>.
+
+# Make coding more python3-ish
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+from textwrap import dedent
+from units.compat.mock import patch
+from units.modules.utils import AnsibleFailJson
+from ansible.modules.network.nxos import nxos_vlans
+from ansible.module_utils.network.nxos.config.vlans.vlans import Vlans
+from .nxos_module import TestNxosModule, load_fixture, set_module_args
+
+ignore_provider_arg = True
+
+
+class TestNxosVlansModule(TestNxosModule):
+
+    module = nxos_vlans
+
+    def setUp(self):
+        super(TestNxosVlansModule, self).setUp()
+
+        self.mock_FACT_LEGACY_SUBSETS = patch('ansible.module_utils.network.nxos.facts.facts.FACT_LEGACY_SUBSETS')
+        self.FACT_LEGACY_SUBSETS = self.mock_FACT_LEGACY_SUBSETS.start()
+
+        self.mock_get_resource_connection_config = patch('ansible.module_utils.network.common.cfg.base.get_resource_connection')
+        self.get_resource_connection_config = self.mock_get_resource_connection_config.start()
+
+        self.mock_get_resource_connection_facts = patch('ansible.module_utils.network.common.facts.facts.get_resource_connection')
+        self.get_resource_connection_facts = self.mock_get_resource_connection_facts.start()
+
+        self.mock_edit_config = patch('ansible.module_utils.network.nxos.config.vlans.vlans.Vlans.edit_config')
+        self.edit_config = self.mock_edit_config.start()
+
+        self.mock_get_device_data = patch('ansible.module_utils.network.nxos.facts.vlans.vlans.VlansFacts.get_device_data')
+        self.get_device_data = self.mock_get_device_data.start()
+
+    def tearDown(self):
+        super(TestNxosVlansModule, self).tearDown()
+        self.mock_FACT_LEGACY_SUBSETS.stop()
+        self.mock_get_resource_connection_config.stop()
+        self.mock_get_resource_connection_facts.stop()
+        self.mock_edit_config.stop()
+
+    def load_fixtures(self, commands=None, device=''):
+        self.mock_FACT_LEGACY_SUBSETS.return_value = dict()
+        self.edit_config.return_value = None
+
+        def load_from_file(*args, **kwargs):
+            cmd = args[1]
+            filename = str(cmd).split(' | ')[0].replace(' ', '_')
+            return load_fixture('nxos_vlans', filename)
+        self.get_device_data.side_effect = load_from_file
+
+    def test_1(self):
+        '''
+        vlan 1,3-5
+        vlan 3
+          name test-vlan3
+        !Note:vlan 4 is present with default settings
+        vlan 5
+          shutdown
+          name test-changeme
+          mode fabricpath
+          state suspend
+          vn-segment 942
+        !Note:vlan 7 is not present
+        vlan 8
+          shutdown
+        '''
+        playbook = dict(config=[
+            dict(vlan_id=4),
+            dict(vlan_id=5, mapped_vni=555, mode='ce'),
+            dict(vlan_id=7, mapped_vni=777, name='test-vlan7', enabled=False),
+            dict(vlan_id=8)
+            # vlan 3 is not present in playbook.
+        ])
+
+        merged = [
+            # Update existing device states with any differences in the playbook.
+            'vlan 5', 'vn-segment 555', 'mode ce',
+            'vlan 7', 'vn-segment 777', 'name test-vlan7', 'shutdown'
+        ]
+        playbook['state'] = 'merged'
+        set_module_args(playbook, ignore_provider_arg)
+        self.execute_module(changed=True, commands=merged)
+
+        deleted = [
+            # Reset existing device state to default values. Scope is limited to
+            # objects in the play.For vlans this means deleting each vlan listed
path: test/units/modules/network/nxos/test_nxos_vlans.py
text: @user1:
*nit* `/play.For/playbook. For /`

@author:
Finally, a real bug. :-)

diff_hunk: @@ -121,57 +130,78 @@ def set_state(self, want, have):
                     commands.extend(self._state_replaced(w, have))
         return commands
 
-    def _state_replaced(self, w, have):
-        """ The command generator when state is replaced
+    def remove_default_states(self, obj):
+        """Removes non-empty but default states from the obj.
+        """
+        default_states = {
+            'enabled': True,
+            'state': 'active',
+            'mode': 'ce',
+        }
+        for k in default_states.keys():
+            if obj[k] == default_states[k]:
+                obj.pop(k, None)
+        return obj
 
+    def _state_replaced(self, w, have):
path: lib/ansible/module_utils/network/nxos/config/vlans/vlans.py
text: @user1:
I would prefer we use `want` instead of `w`.  Makes it more clear.  Also applies to other command generator methods below.

@author:
Changed. 

I'm with you - `w`, along with `d` and `commands` variable names. They wreak havoc with `pdb`.

diff_hunk: @@ -47,24 +54,18 @@ def populate_facts(self, connection, ansible_facts, data=None):
         """
         objs = []
         if not data:
-            data = connection.get('show running-config | section ^vlan')
-        vlans = re.split(r'(,|-)', data.split()[1])
-        for v in vlans:
-            if not v.isdigit():
-                vlans.remove(v)
-
-        config = re.split(r'(^|\n)vlan', data)
-        for conf in config:
-            conf = conf.strip()
-            if conf:
-                if conf[0] in vlans:
-                    vlans.remove(conf[0])
-                    obj = self.render_config(self.generated_spec, conf)
-                    if obj and len(obj.keys()) > 1:
-                        objs.append(obj)
-
-        for v in vlans:
-            obj = self.render_config(self.generated_spec, v)
+            # Use structured for most of the vlan parameter states.
+            # This data is consistent across the supported nxos platforms.
+            structured = self.get_device_data(connection, 'show vlan | json')
path: lib/ansible/module_utils/network/nxos/facts/vlans/vlans.py
text: @user1:
This will not work when we support `httpapi`.  NXAPI does not support `| json`.  We should add a TBD to fix this when support is added.

@user2:
`| json` won't work for the devices that do not have structured output support. We still have to support those devices as they are not End of Support yet according to Cisco https://www.cisco.com/c/en/us/products/collateral/switches/nexus-7000-series-switches/eos-eol-notice-c51-742131.html
We want to make sure that new resource modules work on every device.

@author:
@user2 Hi Trishna. Mike and I are pushing back on this request. This module was very broken except for rudimentary test cases. It now works on _all_ of our regression testbeds and images, with a more comprehensive (and complex) set of test cases, with the exception of the 6.2 image you referenced. We would like to see this PR merged since it fixes so many issues, and open a new issue to address the legacy/EOL limitation separately. I added a TBD to the facts code with a suggested resolution for that. Thanks. 

@user2:
cc @user3 @user4 @user5

diff_hunk: @@ -0,0 +1,3 @@
+            #structured = '{"TABLE_vlanbrief": {"ROW_vlanbrief": [{"vlanshowbr-vlanid": "1", "vlanshowbr-vlanid-utf": "1", "vlanshowbr-vlanname": "default", "vlanshowbr-vlanstate": "active", "vlanshowbr-shutstate": "noshutdown"}, {"vlanshowbr-vlanid": "3", "vlanshowbr-vlanid-utf": "3", "vlanshowbr-vlanname": "VLAN0003", "vlanshowbr-vlanstate": "active", "vlanshowbr-shutstate": "noshutdown"}, {"vlanshowbr-vlanid": "4", "vlanshowbr-vlanid-utf": "4", "vlanshowbr-vlanname": "vlan4", "vlanshowbr-vlanstate": "active", "vlanshowbr-shutstate": "noshutdown"}, {"vlanshowbr-vlanid": "5", "vlanshowbr-vlanid-utf": "5", "vlanshowbr-vlanname": "vlan5", "vlanshowbr-vlanstate": "active", "vlanshowbr-shutstate": "noshutdown"}]}, "TABLE_mtuinfo": {"ROW_mtuinfo": [{"vlanshowinfo-vlanid": "1", "vlanshowinfo-media-type": "enet", "vlanshowinfo-vlanmode": "ce-vlan"}, {"vlanshowinfo-vlanid": "3", "vlanshowinfo-media-type": "enet", "vlanshowinfo-vlanmode": "ce-vlan"}, {"vlanshowinfo-vlanid": "4", "vlanshowinfo-media-type": "enet", "vlanshowinfo-vlanmode": "ce-vlan"}, {"vlanshowinfo-vlanid": "5", "vlanshowinfo-media-type": "enet", "vlanshowinfo-vlanmode": "ce-vlan"}]}}'
path: test/units/modules/network/nxos/fixtures/nxos_vlans/save
text: @user1:
Why do we need this file?

@author:
We do not. It's leftover cruft from testing.

diff_hunk: @@ -49,8 +47,17 @@ def get_vlans_facts(self):
         vlans_facts = facts['ansible_network_resources'].get('vlans')
         if not vlans_facts:
             return []
+
+        # Remove vlan 1 from facts list
+        vlans_facts = [i for i in vlans_facts if (i['vlan_id']) != 1]
path: lib/ansible/module_utils/network/nxos/config/vlans/vlans.py
text: @user1:
Should you use `int(['vlan_id'])` just to be safe when comparing to 1?

@author:
Sure, just to be safe.