From 3903ca5c8ebc9cd49b689c6d8488df8fde6faf11 Mon Sep 17 00:00:00 2001 From: pierremahot Date: Wed, 23 May 2018 16:15:54 +0200 Subject: [PATCH] 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 | 108 +++++++------- test/sanity/pylint/ignore.txt | 1 - .../network/ios/fixtures/ios_vlan_config.cfg | 8 + .../modules/network/ios/test_ios_vlan.py | 137 ++++++++++++++++++ 4 files changed, 204 insertions(+), 50 deletions(-) create mode 100644 test/units/modules/network/ios/fixtures/ios_vlan_config.cfg create mode 100644 test/units/modules/network/ios/test_ios_vlan.py diff --git a/lib/ansible/modules/network/ios/ios_vlan.py b/lib/ansible/modules/network/ios/ios_vlan.py index cd3617e9bf9..fece8876f4a 100644 --- a/lib/ansible/modules/network/ios/ios_vlan.py +++ b/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('name {}'.format(name)) + commands.append('vlan {0}'.format(vlan_id)) + 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('interface {}'.format(i)) + commands.append('vlan {0}'.format(vlan_id)) + 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('interface {}'.format(i)) + commands.append('vlan {0}'.format(vlan_id)) + 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('interface {}'.format(i)) + commands.append('vlan {0}'.format(vlan_id)) + 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('state {}'.format(state)) + commands.append('name {0}'.format(name)) + 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 parse_to_logical_rows(out): + started_yielding = False + 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) + + # Return the rest of it: + yield cur_row + + +def map_ports_str_to_list(ports_str): + return list(filter(bool, (p.strip().replace('Gi', 'GigabitEthernet') for p in ports_str.split(', ')))) + + +def parse_to_obj(logical_rows): + first_row = logical_rows[0] + rest_rows = logical_rows[1:] + obj = re.match(r'(?P\d+)\s+(?P[^\s]+)\s+(?P[^\s]+)\s*(?P.*)', 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 + + +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): - output = run_commands(module, ['show vlan']) - lines = output[0].strip().splitlines()[2:-1] - - if not lines: - return list() - - objs = list() - - for l in lines: - splitted_line = l.strip().replace(",", "").split() - if splitted_line == []: - break - obj = {} - obj['vlan_id'] = splitted_line[0] - obj['name'] = splitted_line[1] - obj['state'] = splitted_line[2] - - if obj['state'] == 'suspended': - obj['state'] = 'suspend' - - 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 + return parse_vlan_brief(run_commands(module, ['show vlan brief'])[0]) def check_declarative_intent_params(want, module, result): diff --git a/test/sanity/pylint/ignore.txt b/test/sanity/pylint/ignore.txt index 612a703d312..266e40feb7e 100644 --- a/test/sanity/pylint/ignore.txt +++ b/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 diff --git a/test/units/modules/network/ios/fixtures/ios_vlan_config.cfg b/test/units/modules/network/ios/fixtures/ios_vlan_config.cfg new file mode 100644 index 00000000000..945b8b4aa2b --- /dev/null +++ b/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 \ No newline at end of file diff --git a/test/units/modules/network/ios/test_ios_vlan.py b/test/units/modules/network/ios/test_ios_vlan.py new file mode 100644 index 00000000000..8315e8343e6 --- /dev/null +++ b/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 . + +# 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)