Fix ios_vlan to correctly identify unmodified config when having long interface names (#40145)

Change the command to get the interface in a vlan "show vlan" => "show vlan brief"
Change the parsing of the return command of the switch.
The return of the ios command is fixed so i cut with fix number of carracter.
Adding looking for the next line to add the forgeted interfaces.

lib/ansible/modules/network/ios/ios_vlan.py

@@ -129,59 +129,59 @@ def map_obj_to_commands(updates, module):
 
         if state == 'absent':
             if obj_in_have:
-                commands.append('no vlan {}'.format(vlan_id))
+                commands.append('no vlan {0}'.format(vlan_id))
 
         elif state == 'present':
             if not obj_in_have:
-                commands.append('vlan {}'.format(vlan_id))
+                commands.append('vlan {0}'.format(vlan_id))
                 if name:
-                    commands.append('name {}'.format(name))
+                    commands.append('name {0}'.format(name))
 
                 if interfaces:
                     for i in interfaces:
-                        commands.append('interface {}'.format(i))
+                        commands.append('interface {0}'.format(i))
                         commands.append('switchport mode access')
-                        commands.append('switchport access vlan {}'.format(vlan_id))
+                        commands.append('switchport access vlan {0}'.format(vlan_id))
 
             else:
                 if name:
                     if name != obj_in_have['name']:
-                        commands.append('vlan {}'.format(vlan_id))
+                        commands.append('vlan {0}'.format(vlan_id))
-                        commands.append('name {}'.format(name))
+                        commands.append('name {0}'.format(name))
 
                 if interfaces:
                     if not obj_in_have['interfaces']:
                         for i in interfaces:
-                            commands.append('vlan {}'.format(vlan_id))
+                            commands.append('vlan {0}'.format(vlan_id))
-                            commands.append('interface {}'.format(i))
+                            commands.append('interface {0}'.format(i))
                             commands.append('switchport mode access')
-                            commands.append('switchport access vlan {}'.format(vlan_id))
+                            commands.append('switchport access vlan {0}'.format(vlan_id))
 
                     elif set(interfaces) != set(obj_in_have['interfaces']):
                         missing_interfaces = list(set(interfaces) - set(obj_in_have['interfaces']))
                         for i in missing_interfaces:
-                            commands.append('vlan {}'.format(vlan_id))
+                            commands.append('vlan {0}'.format(vlan_id))
-                            commands.append('interface {}'.format(i))
+                            commands.append('interface {0}'.format(i))
                             commands.append('switchport mode access')
-                            commands.append('switchport access vlan {}'.format(vlan_id))
+                            commands.append('switchport access vlan {0}'.format(vlan_id))
 
                         superfluous_interfaces = list(set(obj_in_have['interfaces']) - set(interfaces))
                         for i in superfluous_interfaces:
-                            commands.append('vlan {}'.format(vlan_id))
+                            commands.append('vlan {0}'.format(vlan_id))
-                            commands.append('interface {}'.format(i))
+                            commands.append('interface {0}'.format(i))
                             commands.append('switchport mode access')
-                            commands.append('no switchport access vlan {}'.format(vlan_id))
+                            commands.append('no switchport access vlan {0}'.format(vlan_id))
         else:
-            commands.append('vlan {}'.format(vlan_id))
+            commands.append('vlan {0}'.format(vlan_id))
             if name:
-                commands.append('name {}'.format(name))
+                commands.append('name {0}'.format(name))
-            commands.append('state {}'.format(state))
+            commands.append('state {0}'.format(state))
 
     if purge:
         for h in have:
             obj_in_want = search_obj_in_list(h['vlan_id'], want)
             if not obj_in_want and h['vlan_id'] != '1':
-                commands.append('no vlan {}'.format(h['vlan_id']))
+                commands.append('no vlan {0}'.format(h['vlan_id']))
 
     return commands
 
@@ -211,36 +211,46 @@ def map_params_to_obj(module):
     return obj
 
 
-def map_config_to_obj(module):
+def parse_to_logical_rows(out):
-    output = run_commands(module, ['show vlan'])
+    started_yielding = False
-    lines = output[0].strip().splitlines()[2:-1]
+    cur_row = []
+    for l in out.splitlines()[2:]:
+        if not l:
+            """Skip empty lines."""
+            continue
+        if '0' < l[0] < '9':
+            """Line starting with a number."""
+            if started_yielding:
+                yield cur_row
+                cur_row = []  # Reset it to hold a next chunk
+            started_yielding = True
+        cur_row.append(l)
 
-    if not lines:
+    # Return the rest of it:
-        return list()
+    yield cur_row
 
-    objs = list()
 
-    for l in lines:
+def map_ports_str_to_list(ports_str):
-        splitted_line = l.strip().replace(",", "").split()
+    return list(filter(bool, (p.strip().replace('Gi', 'GigabitEthernet') for p in ports_str.split(', '))))
-        if splitted_line == []:
-            break
-        obj = {}
-        obj['vlan_id'] = splitted_line[0]
-        obj['name'] = splitted_line[1]
-        obj['state'] = splitted_line[2]
 
+
+def parse_to_obj(logical_rows):
+    first_row = logical_rows[0]
+    rest_rows = logical_rows[1:]
+    obj = re.match(r'(?P<vlan_id>\d+)\s+(?P<name>[^\s]+)\s+(?P<state>[^\s]+)\s*(?P<interfaces>.*)', first_row).groupdict()
     if obj['state'] == 'suspended':
         obj['state'] = 'suspend'
+    obj['interfaces'] = map_ports_str_to_list(obj['interfaces'])
+    obj['interfaces'].extend(prts_r for prts in rest_rows for prts_r in map_ports_str_to_list(prts))
+    return obj
 
-        obj['interfaces'] = []
-        if len(splitted_line) > 3:
-            interface = []
-            for i in range(3, len(splitted_line)):
-                interface.append(splitted_line[i].replace('Gi', 'GigabitEthernet'))
-            obj['interfaces'].extend(interface)
-        objs.append(obj)
 
-    return objs
+def parse_vlan_brief(vlan_out):
+    return [parse_to_obj(r) for r in parse_to_logical_rows(vlan_out)]
+
+
+def map_config_to_obj(module):
+    return parse_vlan_brief(run_commands(module, ['show vlan brief'])[0])
 
 
 def check_declarative_intent_params(want, module, result):

test/sanity/pylint/ignore.txt

@@ -47,7 +47,6 @@ lib/ansible/modules/network/eos/eos_linkagg.py ansible-format-automatic-specific
 lib/ansible/modules/network/eos/eos_logging.py ansible-format-automatic-specification
 lib/ansible/modules/network/eos/eos_static_route.py ansible-format-automatic-specification
 lib/ansible/modules/network/ios/ios_l3_interface.py ansible-format-automatic-specification
-lib/ansible/modules/network/ios/ios_vlan.py ansible-format-automatic-specification
 lib/ansible/modules/network/iosxr/iosxr_banner.py ansible-format-automatic-specification
 lib/ansible/modules/network/iosxr/iosxr_interface.py ansible-format-automatic-specification
 lib/ansible/modules/network/nxos/nxos_logging.py ansible-format-automatic-specification

test/units/modules/network/ios/fixtures/ios_vlan_config.cfg

@@ -0,0 +1,8 @@
+VLAN Name                             Status    Ports
+---- -------------------------------- --------- -------------------------------
+1    default                          active    Gi1/0/4, Gi1/0/5
+                                                Gi1/0/52
+                                                Gi1/0/54
+2    vlan2                            active    Gi1/0/6, Gi1/0/7
+1002 fddi-default                     act/unsup 
+1003 fddo-default                     act/unsup

test/units/modules/network/ios/test_ios_vlan.py

@@ -0,0 +1,137 @@
+# (c) 2018 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
+
+import json
+
+from ansible.compat.tests.mock import patch
+from ansible.modules.network.ios import ios_vlan
+from ansible.modules.network.ios.ios_vlan import parse_vlan_brief
+from units.modules.utils import set_module_args
+from .ios_module import TestIosModule, load_fixture
+
+
+class TestIosVlanModule(TestIosModule):
+
+    module = ios_vlan
+
+    def setUp(self):
+        super(TestIosVlanModule, self).setUp()
+
+        self.mock_run_commands = patch('ansible.modules.network.ios.ios_vlan.run_commands')
+        self.run_commands = self.mock_run_commands.start()
+
+        self.mock_load_config = patch('ansible.modules.network.ios.ios_vlan.load_config')
+        self.load_config = self.mock_load_config.start()
+
+    def tearDown(self):
+        super(TestIosVlanModule, self).tearDown()
+        self.mock_run_commands.stop()
+        self.mock_load_config.stop()
+
+    def load_fixtures(self, commands=None, transport='cli'):
+        self.run_commands.return_value = [load_fixture('ios_vlan_config.cfg')]
+        self.load_config.return_value = {'diff': None, 'session': 'session'}
+
+    def test_ios_vlan_create(self):
+        set_module_args({'vlan_id': '3', 'name': 'test', 'state': 'present'})
+        result = self.execute_module(changed=True)
+        expected_commands = [
+            'vlan 3',
+            'name test',
+        ]
+        self.assertEqual(result['commands'], expected_commands)
+
+    def test_ios_vlan_rename(self):
+        set_module_args({'vlan_id': '2', 'name': 'test', 'state': 'present'})
+        result = self.execute_module(changed=True)
+        expected_commands = [
+            'vlan 2',
+            'name test',
+        ]
+        self.assertEqual(result['commands'], expected_commands)
+
+    def test_ios_vlan_with_interfaces(self):
+        set_module_args({'vlan_id': '2', 'name': 'vlan2', 'state': 'present', 'interfaces': ['GigabitEthernet1/0/8', 'GigabitEthernet1/0/7']})
+        result = self.execute_module(changed=True)
+        expected_commands = [
+            'vlan 2',
+            'interface GigabitEthernet1/0/8',
+            'switchport mode access',
+            'switchport access vlan 2',
+            'vlan 2',
+            'interface GigabitEthernet1/0/6',
+            'switchport mode access',
+            'no switchport access vlan 2',
+        ]
+        self.assertEqual(result['commands'], expected_commands)
+
+    def test_ios_vlan_with_interfaces_and_newvlan(self):
+        set_module_args({'vlan_id': '3', 'name': 'vlan3', 'state': 'present', 'interfaces': ['GigabitEthernet1/0/8', 'GigabitEthernet1/0/7']})
+        result = self.execute_module(changed=True)
+        expected_commands = [
+            'vlan 3',
+            'name vlan3',
+            'interface GigabitEthernet1/0/8',
+            'switchport mode access',
+            'switchport access vlan 3',
+            'interface GigabitEthernet1/0/7',
+            'switchport mode access',
+            'switchport access vlan 3',
+        ]
+        self.assertEqual(result['commands'], expected_commands)
+
+    def test_parse_vlan_brief(self):
+        result = parse_vlan_brief(load_fixture('ios_vlan_config.cfg'))
+        obj = [
+            {
+                'name': 'default',
+                'interfaces': [
+                    'GigabitEthernet1/0/4',
+                    'GigabitEthernet1/0/5',
+                    'GigabitEthernet1/0/52',
+                    'GigabitEthernet1/0/54',
+                ],
+                'state': 'active',
+                'vlan_id': '1',
+            },
+            {
+                'name': 'vlan2',
+                'interfaces': [
+                    'GigabitEthernet1/0/6',
+                    'GigabitEthernet1/0/7',
+                ],
+                'state': 'active',
+                'vlan_id': '2',
+            },
+            {
+                'name': 'fddi-default',
+                'interfaces': [],
+                'state': 'act/unsup',
+                'vlan_id': '1002',
+            },
+            {
+                'name': 'fddo-default',
+                'interfaces': [],
+                'state': 'act/unsup',
+                'vlan_id': '1003',
+            },
+        ]
+        self.assertEqual(result, obj)