nxos_vlan refactor to support non structured output (#43805)

* nxos_vlan refactor to support non structured output

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* unittest fix

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* minor fixes

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* use check_rc

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* address review comment

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

* remove additional return statement

* address Nate's review

Signed-off-by: Trishna Guha <trishnaguha17@gmail.com>

lib/ansible/module_utils/network/nxos/nxos.py

@@ -154,7 +154,18 @@ class Cli:
         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))
 

lib/ansible/modules/network/nxos/nxos_facts.py

@@ -194,7 +194,7 @@ class FactsBase(object):
             '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 @@ class Hardware(FactsBase):
             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 @@ class Interfaces(FactsBase):
         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 @@ class Interfaces(FactsBase):
                 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 @@ class Interfaces(FactsBase):
         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 @@ class Legacy(FactsBase):
                 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)

lib/ansible/modules/network/nxos/nxos_vlan.py

@@ -74,7 +74,6 @@ options:
     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 @@ import time
 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)
 

lib/ansible/plugins/cliconf/nxos.py

@@ -202,7 +202,7 @@ class Cliconf(CliconfBase):
             try:
                 out = self.send_command(**cmd)
             except AnsibleConnectionFailure as e:
-                if check_rc:
+                if check_rc is True:
                     raise
                 out = getattr(e, 'err', e)
 

lib/ansible/plugins/httpapi/nxos.py

@@ -99,7 +99,7 @@ class HttpApi(HttpApiBase):
         try:
             out = self.send_request(commands)
         except ConnectionError as exc:
-            if check_rc:
+            if check_rc is True:
                 raise
             out = to_text(exc)
 

test/units/modules/network/nxos/fixtures/nxos_vlan/config.cfg

@@ -0,0 +1,4 @@
+vlan 1
+  mode ce
+  state active
+  no shutdown

test/units/modules/network/nxos/fixtures/nxos_vlan/show_vlan.txt

@@ -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"
-    }
-  }
-}

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"
+    }
+  }
+}

test/units/modules/network/nxos/test_nxos_vlan.py

@@ -42,16 +42,11 @@ class TestNxosVlanModule(TestNxosModule):
         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):