From 96346938eef9594f4d1ceeff1d48f13807e36e21 Mon Sep 17 00:00:00 2001 From: Trishna Guha Date: Fri, 10 Aug 2018 21:03:56 +0530 Subject: [PATCH] nxos_vlan refactor to support non structured output (#43805) * nxos_vlan refactor to support non structured output Signed-off-by: Trishna Guha * unittest fix Signed-off-by: Trishna Guha * minor fixes Signed-off-by: Trishna Guha * use check_rc Signed-off-by: Trishna Guha * address review comment Signed-off-by: Trishna Guha * remove additional return statement * address Nate's review Signed-off-by: Trishna Guha --- lib/ansible/module_utils/network/nxos/nxos.py | 13 +- .../modules/network/nxos/nxos_facts.py | 60 ++--- lib/ansible/modules/network/nxos/nxos_vlan.py | 233 ++++++++++++------ lib/ansible/plugins/cliconf/nxos.py | 2 +- lib/ansible/plugins/httpapi/nxos.py | 2 +- .../nxos/fixtures/nxos_vlan/config.cfg | 4 + .../nxos/fixtures/nxos_vlan/show_vlan.txt | 18 -- .../fixtures/nxos_vlan/show_vlan_brief.txt | 11 + .../modules/network/nxos/test_nxos_vlan.py | 5 - 9 files changed, 199 insertions(+), 149 deletions(-) delete mode 100644 test/units/modules/network/nxos/fixtures/nxos_vlan/show_vlan.txt create mode 100644 test/units/modules/network/nxos/fixtures/nxos_vlan/show_vlan_brief.txt diff --git a/lib/ansible/module_utils/network/nxos/nxos.py b/lib/ansible/module_utils/network/nxos/nxos.py index 50f8b2fbb113c8..1b25113971464b 100644 --- a/lib/ansible/module_utils/network/nxos/nxos.py +++ b/lib/ansible/module_utils/network/nxos/nxos.py @@ -154,7 +154,18 @@ def run_commands(self, commands, check_rc=True): connection = self._get_connection() try: - return connection.run_commands(commands, check_rc) + out = connection.run_commands(commands, check_rc) + if check_rc == 'retry_json': + capabilities = self.get_capabilities() + network_api = capabilities.get('network_api') + + if network_api == 'cliconf' and out: + for index, resp in enumerate(out): + if 'Invalid command at' in resp and 'json' in resp: + if commands[index]['output'] == 'json': + commands[index]['output'] = 'text' + out = connection.run_commands(commands, check_rc) + return out except ConnectionError as exc: self._module.fail_json(msg=to_text(exc)) diff --git a/lib/ansible/modules/network/nxos/nxos_facts.py b/lib/ansible/modules/network/nxos/nxos_facts.py index 593ca6d0b49f16..4ecb1ba7aa64ac 100644 --- a/lib/ansible/modules/network/nxos/nxos_facts.py +++ b/lib/ansible/modules/network/nxos/nxos_facts.py @@ -194,7 +194,7 @@ def run(self, command, output='text'): 'command': command, 'output': output } - resp = run_commands(self.module, [command], check_rc=False) + resp = run_commands(self.module, [command], check_rc='retry_json') try: return resp[0] except IndexError: @@ -234,10 +234,8 @@ class Default(FactsBase): def populate(self): data = None - try: - data = self.run('show version', output='json') - except ConnectionError: - data = self.run('show version') + data = self.run('show version', output='json') + if data: if isinstance(data, dict): if data.get('sys_ver_str'): @@ -300,10 +298,8 @@ def populate(self): self.facts['filesystems'] = self.parse_filesystems(data) data = None - try: - data = self.run('show system resources', output='json') - except ConnectionError: - data = self.run('show system resources') + data = self.run('show system resources', output='json') + if data: if isinstance(data, dict): self.facts['memtotal_mb'] = int(data['memory_usage_total']) / 1024 @@ -380,10 +376,8 @@ def populate(self): self.facts['all_ipv6_addresses'] = list() data = None - try: - data = self.run('show interface', output='json') - except ConnectionError: - data = self.run('show interface') + data = self.run('show interface', output='json') + if data: if isinstance(data, dict): self.facts['interfaces'] = self.populate_structured_interfaces(data) @@ -392,10 +386,7 @@ def populate(self): self.facts['interfaces'] = self.populate_interfaces(interfaces) if self.ipv6_structure_op_supported(): - try: - data = self.run('show ipv6 interface', output='json') - except ConnectionError: - data = self.run('show ipv6 interface') + data = self.run('show ipv6 interface', output='json') else: data = None if data: @@ -409,10 +400,7 @@ def populate(self): if data: self.facts['neighbors'] = self.populate_neighbors(data) - try: - data = self.run('show cdp neighbors detail', output='json') - except ConnectionError: - data = self.run('show cdp neighbors detail') + data = self.run('show cdp neighbors detail', output='json') if data: if isinstance(data, dict): self.facts['neighbors'] = self.populate_structured_neighbors_cdp(data) @@ -718,10 +706,7 @@ class Legacy(FactsBase): def populate(self): data = None - try: - data = self.run('show version') - except ConnectionError: - data = self.run('show version', output='json') + data = self.run('show version', output='json') if data: if isinstance(data, dict): self.facts.update(self.transform_dict(data, self.VERSION_MAP)) @@ -730,50 +715,35 @@ def populate(self): self.facts['_os'] = self.parse_os(data) self.facts['_platform'] = self.parse_platform(data) - try: - data = self.run('show interface', output='json') - except ConnectionError: - data = self.run('show interface') + data = self.run('show interface', output='json') if data: if isinstance(data, dict): self.facts['_interfaces_list'] = self.parse_structured_interfaces(data) else: self.facts['_interfaces_list'] = self.parse_interfaces(data) - try: - data = self.run('show vlan brief', output='json') - except ConnectionError: - data = self.run('show vlan brief') + data = self.run('show vlan brief', output='json') if data: if isinstance(data, dict): self.facts['_vlan_list'] = self.parse_structured_vlans(data) else: self.facts['_vlan_list'] = self.parse_vlans(data) - try: - data = self.run('show module', output='json') - except ConnectionError: - data = self.run('show module') + data = self.run('show module', output='json') if data: if isinstance(data, dict): self.facts['_module'] = self.parse_structured_module(data) else: self.facts['_module'] = self.parse_module(data) - try: - data = self.run('show environment fan', output='json') - except ConnectionError: - data = self.run('show environment fan') + data = self.run('show environment fan', output='json') if data: if isinstance(data, dict): self.facts['_fan_info'] = self.parse_structured_fan_info(data) else: self.facts['_fan_info'] = self.parse_fan_info(data) - try: - data = self.run('show environment power', output='json') - except ConnectionError: - data = self.run('show environment power') + data = self.run('show environment power', output='json') if data: if isinstance(data, dict): self.facts['_power_supply_info'] = self.parse_structured_power_supply_info(data) diff --git a/lib/ansible/modules/network/nxos/nxos_vlan.py b/lib/ansible/modules/network/nxos/nxos_vlan.py index 152c9c0958907b..cb497eb535bee1 100644 --- a/lib/ansible/modules/network/nxos/nxos_vlan.py +++ b/lib/ansible/modules/network/nxos/nxos_vlan.py @@ -74,7 +74,6 @@ description: - Set VLAN mode to classical ethernet or fabricpath. This is a valid option for Nexus 5000 and 7000 series. - default: ce choices: ['ce','fabricpath'] version_added: "2.4" aggregate: @@ -156,8 +155,10 @@ from copy import deepcopy from ansible.module_utils.network.nxos.nxos import get_config, load_config, run_commands -from ansible.module_utils.network.nxos.nxos import get_capabilities, nxos_argument_spec +from ansible.module_utils.network.nxos.nxos import normalize_interface, nxos_argument_spec from ansible.module_utils.basic import AnsibleModule +from ansible.module_utils.connection import ConnectionError +from ansible.module_utils.network.common.config import CustomNetworkConfig from ansible.module_utils.network.common.utils import remove_default_spec @@ -191,7 +192,7 @@ def is_default_name(obj, vlan_id): return False -def map_obj_to_commands(updates, module, os_platform): +def map_obj_to_commands(updates, module): commands = list() purge = module.params['purge'] want, have = updates @@ -201,15 +202,11 @@ def map_obj_to_commands(updates, module, os_platform): name = w['name'] interfaces = w.get('interfaces') or [] mapped_vni = w['mapped_vni'] + mode = w['mode'] vlan_state = w['vlan_state'] admin_state = w['admin_state'] state = w['state'] del w['state'] - if any(i in os_platform for i in ['5K', '7K']): - mode = w['mode'] - else: - w['mode'] = None - mode = w['mode'] obj_in_have = search_obj_in_list(vlan_id, have) @@ -258,7 +255,7 @@ def map_obj_to_commands(updates, module, os_platform): else: if not is_default_name(obj_in_have, vlan_id): commands.append('no name') - if key == 'vlan_state': + if key == 'vlan_state' and value: commands.append('state {0}'.format(value)) if key == 'mapped_vni': if value == 'default': @@ -271,7 +268,7 @@ def map_obj_to_commands(updates, module, os_platform): commands.append('no shutdown') elif value == 'down': commands.append('shutdown') - if key == 'mode': + if key == 'mode' and value: commands.append('mode {0}'.format(value)) if len(commands) > 1: commands.append('exit') @@ -367,6 +364,13 @@ def vlan_range_commands(module, have): return commands +def normalize(interfaces): + normalized = None + if interfaces: + normalized = [normalize_interface(i) for i in interfaces] + return normalized + + def map_params_to_obj(module): obj = [] if module.params['vlan_range']: @@ -382,19 +386,24 @@ def map_params_to_obj(module): d = item.copy() d['vlan_id'] = str(d['vlan_id']) d['mapped_vni'] = str(d['mapped_vni']) + d['interfaces'] = normalize(d['interfaces']) + d['associated_interfaces'] = normalize(d['associated_interfaces']) obj.append(d) else: + interfaces = normalize(module.params['interfaces']) + associated_interfaces = normalize(module.params['associated_interfaces']) + obj.append({ 'vlan_id': str(module.params['vlan_id']), 'name': module.params['name'], - 'interfaces': module.params['interfaces'], + 'interfaces': interfaces, 'vlan_state': module.params['vlan_state'], 'mapped_vni': str(module.params['mapped_vni']), 'state': module.params['state'], 'admin_state': module.params['admin_state'], 'mode': module.params['mode'], - 'associated_interfaces': module.params['associated_interfaces'] + 'associated_interfaces': associated_interfaces }) return obj @@ -408,49 +417,23 @@ def parse_admin_state(vlan): return 'down' -def parse_mode(os_platform, output, vlan_id): - if not any(i in os_platform for i in ['5K', '7K']): - return None - - try: - mtus = output['TABLE_mtuinfo']['ROW_mtuinfo'] - except KeyError: - return None - - if mtus: - if isinstance(mtus, list): - for mtu in mtus: - if mtu['vlanshowinfo-vlanid'] == vlan_id: - mode = mtu.get('vlanshowinfo-vlanmode') - if mode == 'ce-vlan': - return 'ce' - elif mode == 'fabricpath-vlan': - return 'fabricpath' - return None - - elif isinstance(mtus, dict): - if mtus['vlanshowinfo-vlanid'] == vlan_id: - mode = mtus.get('vlanshowinfo-vlanmode') - if mode == 'ce-vlan': - return 'ce' - elif mode == 'fabricpath-vlan': - return 'fabricpath' - return None +def parse_mode(config): + mode = None - else: - return None - else: - return None + if config: + match = re.search(r'mode (\S+)', config) + if match: + mode = match.group(1) + return mode -def parse_vni(module, vlan_id): +def parse_vni(config): vni = None - flags = ['| section vlan.{0}'.format(vlan_id)] - cfg = get_config(module, flags=flags) - match = re.search(r'vn-segment (\S+)', cfg, re.M) - if match: - vni = match.group(1) + if config: + match = re.search(r'vn-segment (\S+)', config) + if match: + vni = match.group(1) return str(vni) @@ -482,41 +465,138 @@ def parse_interfaces(module, vlan): return vlan_int -def parse_vlan_options(module, os_platform, output, vlan): +def parse_vlan_config(netcfg, vlan_id): + parents = ['vlan {0}'.format(vlan_id)] + config = netcfg.get_section(parents) + return config + + +def parse_vlan_options(module, netcfg, output, vlan): obj = {} vlan_id = vlan['vlanshowbr-vlanid-utf'] + config = parse_vlan_config(netcfg, vlan_id) + obj['vlan_id'] = str(vlan_id) obj['name'] = vlan.get('vlanshowbr-vlanname') obj['vlan_state'] = vlan.get('vlanshowbr-vlanstate') obj['admin_state'] = parse_admin_state(vlan) - obj['mode'] = parse_mode(os_platform, output, vlan_id) - obj['mapped_vni'] = parse_vni(module, vlan_id) + obj['mode'] = parse_mode(config) + obj['mapped_vni'] = parse_vni(config) obj['interfaces'] = parse_interfaces(module, vlan) return obj -def map_config_to_obj(module, os_platform): +def parse_vlan_non_structured(module, netcfg, vlans): + objs = list() + + for vlan in vlans: + vlan_match = re.search(r'(\d+)', vlan, re.M) + if vlan_match: + obj = {} + vlan_id = vlan_match.group(1) + obj['vlan_id'] = str(vlan_id) + + name_match = re.search(r'{0}\s*(\S+)'.format(vlan_id), vlan, re.M) + if name_match: + name = name_match.group(1) + obj['name'] = name + + state_match = re.search(r'{0}\s*{1}\s*(\S+)'.format(vlan_id, name), vlan, re.M) + if state_match: + vlan_state_match = state_match.group(1) + if vlan_state_match == 'suspended': + vlan_state = 'suspend' + admin_state = 'up' + elif vlan_state_match == 'sus/lshut': + vlan_state = 'suspend' + admin_state = 'down' + if vlan_state_match == 'active': + vlan_state = 'active' + admin_state = 'up' + if vlan_state_match == 'act/lshut': + vlan_state = 'active' + admin_state = 'down' + + obj['vlan_state'] = vlan_state + obj['admin_state'] = admin_state + + vlan = ','.join(vlan.splitlines()) + interfaces = list() + intfs_match = re.search(r'{0}\s*{1}\s*{2}\s*(.*)'.format(vlan_id, name, vlan_state_match), + vlan, re.M) + if intfs_match: + intfs = intfs_match.group(1) + intfs = intfs.split() + for i in intfs: + intf = normalize_interface(i.strip(',')) + interfaces.append(intf) + + if interfaces: + obj['interfaces'] = interfaces + else: + obj['interfaces'] = None + + config = parse_vlan_config(netcfg, vlan_id) + obj['mode'] = parse_mode(config) + obj['mapped_vni'] = parse_vni(config) + + objs.append(obj) + + return objs + + +def map_config_to_obj(module): objs = list() - output = run_commands(module, ['show vlan | json'])[0] - try: - vlans = output['TABLE_vlanbrief']['ROW_vlanbrief'] - except KeyError: - return objs - - if vlans: - if isinstance(vlans, list): - for vlan in vlans: - obj = parse_vlan_options(module, os_platform, output, vlan) - objs.append(obj) - - elif isinstance(vlans, dict): - obj = parse_vlan_options(module, os_platform, output, vlans) - objs.append(obj) + output = None + + command = ['show vlan brief | json'] + output = run_commands(module, command, check_rc='retry_json')[0] + if output: + netcfg = CustomNetworkConfig(indent=2, + contents=get_config(module, flags=['all'])) + + if isinstance(output, dict): + vlans = None + try: + vlans = output['TABLE_vlanbriefxbrief']['ROW_vlanbriefxbrief'] + except KeyError: + return objs + + if vlans: + if isinstance(vlans, list): + for vlan in vlans: + obj = parse_vlan_options(module, netcfg, output, vlan) + objs.append(obj) + elif isinstance(vlans, dict): + obj = parse_vlan_options(module, netcfg, output, vlans) + objs.append(obj) + else: + vlans = list() + splitted_line = re.split(r'\n(\d+)|\n{2}', output.strip()) + + for line in splitted_line: + if not line: + continue + if len(line) > 0: + line = line.strip() + if line[0].isdigit(): + match = re.search(r'(\d+)', line, re.M) + if match: + v = match.group(1) + pos1 = splitted_line.index(v) + pos2 = pos1 + 1 + vlaninfo = ''.join(splitted_line[pos1:pos2 + 1]) + vlans.append(vlaninfo) + + if vlans: + objs = parse_vlan_non_structured(module, netcfg, vlans) + else: + return objs return objs -def check_declarative_intent_params(want, module, os_platform, result): +def check_declarative_intent_params(want, module, result): have = None is_delay = False @@ -530,7 +610,7 @@ def check_declarative_intent_params(want, module, os_platform, result): is_delay = True if have is None: - have = map_config_to_obj(module, os_platform) + have = map_config_to_obj(module) for i in w['associated_interfaces']: obj_in_have = search_obj_in_list(w['vlan_id'], have) @@ -552,7 +632,7 @@ def main(): delay=dict(default=10, type='int'), state=dict(choices=['present', 'absent'], default='present', required=False), admin_state=dict(choices=['up', 'down'], required=False, default='up'), - mode=dict(choices=['ce', 'fabricpath'], required=False, default='ce'), + mode=dict(choices=['ce', 'fabricpath'], required=False), ) aggregate_spec = deepcopy(element_spec) @@ -579,22 +659,19 @@ def main(): mutually_exclusive=mutually_exclusive, supports_check_mode=True) - info = get_capabilities(module).get('device_info', {}) - os_platform = info.get('network_os_platform', '') - warnings = list() result = {'changed': False} if warnings: result['warnings'] = warnings - have = map_config_to_obj(module, os_platform) + have = map_config_to_obj(module) want = map_params_to_obj(module) if module.params['vlan_range']: commands = vlan_range_commands(module, have) result['commands'] = commands else: - commands = map_obj_to_commands((want, have), module, os_platform) + commands = map_obj_to_commands((want, have), module) result['commands'] = commands if commands: @@ -603,7 +680,7 @@ def main(): result['changed'] = True if want: - check_declarative_intent_params(want, module, os_platform, result) + check_declarative_intent_params(want, module, result) module.exit_json(**result) diff --git a/lib/ansible/plugins/cliconf/nxos.py b/lib/ansible/plugins/cliconf/nxos.py index 64839ce9555064..0ba2dbf4997cf0 100644 --- a/lib/ansible/plugins/cliconf/nxos.py +++ b/lib/ansible/plugins/cliconf/nxos.py @@ -202,7 +202,7 @@ def run_commands(self, commands=None, check_rc=True): try: out = self.send_command(**cmd) except AnsibleConnectionFailure as e: - if check_rc: + if check_rc is True: raise out = getattr(e, 'err', e) diff --git a/lib/ansible/plugins/httpapi/nxos.py b/lib/ansible/plugins/httpapi/nxos.py index e5fdf5ec5677af..3bb5de812d5683 100644 --- a/lib/ansible/plugins/httpapi/nxos.py +++ b/lib/ansible/plugins/httpapi/nxos.py @@ -99,7 +99,7 @@ def run_commands(self, commands, check_rc=True): try: out = self.send_request(commands) except ConnectionError as exc: - if check_rc: + if check_rc is True: raise out = to_text(exc) diff --git a/test/units/modules/network/nxos/fixtures/nxos_vlan/config.cfg b/test/units/modules/network/nxos/fixtures/nxos_vlan/config.cfg index e69de29bb2d1d6..905d309ff09e93 100644 --- a/test/units/modules/network/nxos/fixtures/nxos_vlan/config.cfg +++ b/test/units/modules/network/nxos/fixtures/nxos_vlan/config.cfg @@ -0,0 +1,4 @@ +vlan 1 + mode ce + state active + no shutdown diff --git a/test/units/modules/network/nxos/fixtures/nxos_vlan/show_vlan.txt b/test/units/modules/network/nxos/fixtures/nxos_vlan/show_vlan.txt deleted file mode 100644 index 58bcedda8f327b..00000000000000 --- a/test/units/modules/network/nxos/fixtures/nxos_vlan/show_vlan.txt +++ /dev/null @@ -1,18 +0,0 @@ -{ - "TABLE_vlanbrief": { - "ROW_vlanbrief": { - "vlanshowbr-vlanid": 16777216, - "vlanshowbr-vlanid-utf": 1, - "vlanshowbr-vlanname": "default", - "vlanshowbr-vlanstate": "active", - "vlanshowbr-shutstate": "noshutdown" - } - }, - "TABLE_mtuinfo": { - "ROW_mtuinfo": { - "vlanshowinfo-vlanid": 1, - "vlanshowinfo-media-type": "enet", - "vlanshowinfo-vlanmode": "ce-vlan" - } - } -} diff --git a/test/units/modules/network/nxos/fixtures/nxos_vlan/show_vlan_brief.txt b/test/units/modules/network/nxos/fixtures/nxos_vlan/show_vlan_brief.txt new file mode 100644 index 00000000000000..ca90e5d4944076 --- /dev/null +++ b/test/units/modules/network/nxos/fixtures/nxos_vlan/show_vlan_brief.txt @@ -0,0 +1,11 @@ +{ + "TABLE_vlanbriefxbrief": { + "ROW_vlanbriefxbrief": { + "vlanshowbr-vlanid": 16777216, + "vlanshowbr-vlanid-utf": 1, + "vlanshowbr-vlanname": "default", + "vlanshowbr-vlanstate": "active", + "vlanshowbr-shutstate": "noshutdown" + } + } +} diff --git a/test/units/modules/network/nxos/test_nxos_vlan.py b/test/units/modules/network/nxos/test_nxos_vlan.py index ba2469eebdb9cb..dcc9f131b5e3b7 100644 --- a/test/units/modules/network/nxos/test_nxos_vlan.py +++ b/test/units/modules/network/nxos/test_nxos_vlan.py @@ -42,16 +42,11 @@ def setUp(self): self.mock_get_config = patch('ansible.modules.network.nxos.nxos_vlan.get_config') self.get_config = self.mock_get_config.start() - self.mock_get_capabilities = patch('ansible.modules.network.nxos.nxos_vlan.get_capabilities') - self.get_capabilities = self.mock_get_capabilities.start() - self.get_capabilities.return_value = {'network_api': 'cliconf'} - def tearDown(self): super(TestNxosVlanModule, self).tearDown() self.mock_run_commands.stop() self.mock_load_config.stop() self.mock_get_config.stop() - self.mock_get_capabilities.stop() def load_fixtures(self, commands=None, device=''): def load_from_file(*args, **kwargs):