refactor iosxr_system for cliconf and netconf (#34749)

* * refactor iosxr_system for cliconf and netconf

* * Fix unit tests and sanity issues

lib/ansible/module_utils/network/common/netconf.py

@@ -84,14 +84,16 @@ class NetconfConnection(Connection):
 
                 warnings = []
                 for error in error_list:
-                    try:
-                        message = error.find('./nc:error-message', NS_MAP).text
-                    except Exception:
-                        message = error.find('./nc:error-info', NS_MAP).text
+                    message_ele = error.find('./nc:error-message', NS_MAP)
+
+                    if message_ele is None:
+                        message_ele = error.find('./nc:error-info', NS_MAP)
+
+                    message = message_ele.text if message_ele is not None else None
 
                     severity = error.find('./nc:error-severity', NS_MAP).text
 
-                    if severity == 'warning' and self.ignore_warning:
+                    if severity == 'warning' and self.ignore_warning and message is not None:
                         warnings.append(message)
                     else:
                         raise ConnectionError(to_text(rpc_error, errors='surrogate_then_replace'))

lib/ansible/module_utils/network/iosxr/iosxr.py

@@ -137,9 +137,7 @@ def build_xml_subtree(container_ele, xmap, param=None, opcode=None):
     meta_subtree = list()
 
     for key, meta in xmap.items():
-
         candidates = meta.get('xpath', "").split("/")
-
         if container_ele.tag == candidates[-2]:
             parent = container_ele
         elif sub_root.tag == candidates[-2]:
@@ -249,14 +247,13 @@ def build_xml(container, xmap=None, params=None, opcode=None):
 
     container_ele = etree.SubElement(root, container, nsmap=NS_DICT[container.upper() + "_NSMAP"])
 
-    if xmap:
-        if not params:
-            build_xml_subtree(container_ele, xmap)
+    if xmap is not None:
+        if params is None:
+            build_xml_subtree(container_ele, xmap, opcode=opcode)
         else:
             subtree_list = list()
-
             for param in to_list(params):
-                subtree_ele = build_xml_subtree(container_ele, xmap, param, opcode=opcode)
+                subtree_ele = build_xml_subtree(container_ele, xmap, param=param, opcode=opcode)
                 if subtree_ele is not None:
                     subtree_list.append(subtree_ele)
 

lib/ansible/modules/network/iosxr/iosxr_system.py

@@ -15,7 +15,9 @@ DOCUMENTATION = """
 ---
 module: iosxr_system
 version_added: "2.3"
-author: "Peter Sprygada (@privateip)"
+author:
+  - "Peter Sprygada (@privateip)"
+  - "Kedar Kekan @kedarX"
 short_description: Manage the system attributes on Cisco IOS XR devices
 description:
   - This module provides declarative management of node system attributes
@@ -24,11 +26,17 @@ description:
     configuration.
 extends_documentation_fragment: iosxr
 notes:
-  - Tested against IOS XR 6.1.2
+  - Tested against IOS XRv 6.1.2
+  - name-servers I(state=absent) operation with C(netconf) transport is a success, but with rpc-error. This is
+    due to XR platform issue. Recommended to use I(ignore_errors) option with the task as a workaround.
 options:
   hostname:
     description:
       - Configure the device hostname parameter. This option takes an ASCII string value.
+  vrf:
+    description:
+      - VRF name for domain services
+    version_added: 2.5
   domain_name:
     description:
       - Configure the IP domain name
@@ -73,7 +81,7 @@ options:
 """
 
 EXAMPLES = """
-- name: configure hostname and domain-name
+- name: configure hostname and domain-name (default vrf=default)
   iosxr_system:
     hostname: iosxr01
     domain_name: test.example.com
@@ -83,11 +91,26 @@ EXAMPLES = """
       - cisco.com
 - name: remove configuration
   iosxr_system:
+    hostname: iosxr01
+    domain_name: test.example.com
+    domain-search:
+      - ansible.com
+      - redhat.com
+      - cisco.com
     state: absent
+- name: configure hostname and domain-name with vrf
+  iosxr_system:
+    hostname: iosxr01
+    vrf: nondefault
+    domain_name: test.example.com
+    domain-search:
+      - ansible.com
+      - redhat.com
+      - cisco.com
 - name: configure DNS lookup sources
   iosxr_system:
     lookup_source: MgmtEth0/0/CPU0/0
-    lookup_enabled: yes
+    lookup_enabled: True
 - name: configure name servers
   iosxr_system:
     name_servers:
@@ -103,12 +126,36 @@ commands:
   sample:
     - hostname iosxr01
     - ip domain-name test.example.com
+xml:
+  description: NetConf rpc xml sent to device with transport C(netconf)
+  returned: always (empty list when no xml rpc to send)
+  type: list
+  version_added: 2.5
+  sample:
+    - '<config xmlns:xc="urn:ietf:params:xml:ns:netconf:base:1.0">
+    <ip-domain xmlns="http://cisco.com/ns/yang/Cisco-IOS-XR-ip-domain-cfg">
+    <vrfs>
+    <vrf>
+    <vrf-name>default</vrf-name>
+    <lists>
+    <list xc:operation="merge">
+    <order>0</order>
+    <list-name>redhat.com</list-name>
+    </list>
+    </lists>
+    </vrf>
+    </vrfs>
+    </ip-domain>
+    </config>'
 """
+
 import re
+import collections
 
 from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.network.iosxr.iosxr import get_config, load_config
-from ansible.module_utils.network.iosxr.iosxr import iosxr_argument_spec
+from ansible.module_utils.network.iosxr.iosxr import get_config, load_config, etree_findall
+from ansible.module_utils.network.iosxr.iosxr import is_cliconf, is_netconf, etree_find
+from ansible.module_utils.network.iosxr.iosxr import iosxr_argument_spec, build_xml
 
 
 def diff_list(want, have):
@@ -117,98 +164,389 @@ def diff_list(want, have):
     return (adds, removes)
 
 
-def map_obj_to_commands(want, have, module):
+class ConfigBase(object):
+    def __init__(self, module):
+        self._module = module
+        self._result = {'changed': False, 'warnings': []}
+        self._want = dict()
+        self._have = dict()
+
+    def map_params_to_obj(self):
+        self._want.update({
+            'hostname': self._module.params['hostname'],
+            'vrf': self._module.params['vrf'],
+            'domain_name': self._module.params['domain_name'],
+            'domain_search': self._module.params['domain_search'],
+            'lookup_source': self._module.params['lookup_source'],
+            'lookup_enabled': self._module.params['lookup_enabled'],
+            'name_servers': self._module.params['name_servers']
+        })
+
+
+class CliConfiguration(ConfigBase):
+    def __init__(self, module):
+        super(CliConfiguration, self).__init__(module)
+
+    def map_obj_to_commands(self):
         commands = list()
-    state = module.params['state']
+        state = self._module.params['state']
 
         def needs_update(x):
-        return want.get(x) and (want.get(x) != have.get(x))
+            return self._want.get(x) and (self._want.get(x) != self._have.get(x))
 
         if state == 'absent':
-        if have['hostname'] != 'ios':
+            if self._have['hostname'] != 'ios':
                 commands.append('no hostname')
-        if have['domain_name']:
+            if self._have['domain_name']:
                 commands.append('no domain name')
-        if have['lookup_source']:
-            commands.append('no domain lookup source-interface %s' % have['lookup_source'])
-        if not have['lookup_enabled']:
+            if self._have['lookup_source']:
+                commands.append('no domain lookup source-interface {0!s}'.format(self._have['lookup_source']))
+            if not self._have['lookup_enabled']:
                 commands.append('no domain lookup disable')
-        for item in have['name_servers']:
-            commands.append('no domain name-server %s' % item)
-        for item in have['domain_search']:
-            commands.append('no domain list %s' % item)
+            for item in self._have['name_servers']:
+                commands.append('no domain name-server {0!s}'.format(item))
+            for item in self._have['domain_search']:
+                commands.append('no domain list {0!s}'.format(item))
 
         elif state == 'present':
             if needs_update('hostname'):
-            commands.append('hostname %s' % want['hostname'])
+                commands.append('hostname {0!s}'.format(self._want['hostname']))
 
             if needs_update('domain_name'):
-            commands.append('domain name %s' % want['domain_name'])
+                commands.append('domain name {0!s}'.format(self._want['domain_name']))
 
             if needs_update('lookup_source'):
-            commands.append('domain lookup source-interface %s' % want['lookup_source'])
+                commands.append('domain lookup source-interface {0!s}'.format(self._want['lookup_source']))
 
-        if needs_update('lookup_enabled'):
+            cmd = None
+            if not self._want['lookup_enabled'] and self._have['lookup_enabled']:
                 cmd = 'domain lookup disable'
-            if want['lookup_enabled']:
-                cmd = 'no %s' % cmd
+            elif self._want['lookup_enabled'] and not self._have['lookup_enabled']:
+                cmd = 'no domain lookup disable'
+            if cmd is not None:
                 commands.append(cmd)
 
-        if want['name_servers'] is not None:
-            adds, removes = diff_list(want['name_servers'], have['name_servers'])
+            if self._want['name_servers'] is not None:
+                adds, removes = diff_list(self._want['name_servers'], self._have['name_servers'])
                 for item in adds:
-                commands.append('domain name-server %s' % item)
+                    commands.append('domain name-server {0!s}'.format(item))
                 for item in removes:
-                commands.append('no domain name-server %s' % item)
+                    commands.append('no domain name-server {0!s}'.format(item))
 
-        if want['domain_search'] is not None:
-            adds, removes = diff_list(want['domain_search'], have['domain_search'])
+            if self._want['domain_search'] is not None:
+                adds, removes = diff_list(self._want['domain_search'], self._have['domain_search'])
                 for item in adds:
-                commands.append('domain list %s' % item)
+                    commands.append('domain list {0!s}'.format(item))
                 for item in removes:
-                commands.append('no domain list %s' % item)
+                    commands.append('no domain list {0!s}'.format(item))
 
-    return commands
+        self._result['commands'] = []
+        if commands:
+            commit = not self._module.check_mode
+            diff = load_config(self._module, commands, commit=commit)
+            if diff:
+                self._result['diff'] = dict(prepared=diff)
 
+            self._result['commands'] = commands
+            self._result['changed'] = True
 
-def parse_hostname(config):
+    def parse_hostname(self, config):
         match = re.search(r'^hostname (\S+)', config, re.M)
         return match.group(1)
 
-
-def parse_domain_name(config):
+    def parse_domain_name(self, config):
         match = re.search(r'^domain name (\S+)', config, re.M)
         if match:
             return match.group(1)
 
-
-def parse_lookup_source(config):
+    def parse_lookup_source(self, config):
         match = re.search(r'^domain lookup source-interface (\S+)', config, re.M)
         if match:
             return match.group(1)
 
-
-def map_config_to_obj(module):
-    config = get_config(module)
-    return {
-        'hostname': parse_hostname(config),
-        'domain_name': parse_domain_name(config),
+    def map_config_to_obj(self):
+        config = get_config(self._module)
+        self._have.update({
+            'hostname': self.parse_hostname(config),
+            'domain_name': self.parse_domain_name(config),
             'domain_search': re.findall(r'^domain list (\S+)', config, re.M),
-        'lookup_source': parse_lookup_source(config),
+            'lookup_source': self.parse_lookup_source(config),
             'lookup_enabled': 'domain lookup disable' not in config,
             'name_servers': re.findall(r'^domain name-server (\S+)', config, re.M)
-    }
+        })
+
+    def run(self):
+        self.map_params_to_obj()
+        self.map_config_to_obj()
+        self.map_obj_to_commands()
+
+        return self._result
+
+
+class NCConfiguration(ConfigBase):
+    def __init__(self, module):
+        super(NCConfiguration, self).__init__(module)
+        self._system_meta = collections.OrderedDict()
+        self._system_domain_meta = collections.OrderedDict()
+        self._system_server_meta = collections.OrderedDict()
+        self._hostname_meta = collections.OrderedDict()
+        self._lookup_source_meta = collections.OrderedDict()
+        self._lookup_meta = collections.OrderedDict()
+
+    def map_obj_to_xml_rpc(self):
+        self._system_meta.update([
+            ('vrfs', {'xpath': 'ip-domain/vrfs', 'tag': True, 'operation': 'edit'}),
+            ('vrf', {'xpath': 'ip-domain/vrfs/vrf', 'tag': True, 'operation': 'edit'}),
+            ('a:vrf', {'xpath': 'ip-domain/vrfs/vrf/vrf-name', 'operation': 'edit'}),
+            ('a:domain_name', {'xpath': 'ip-domain/vrfs/vrf/name', 'operation': 'edit', 'attrib': "operation"}),
+        ])
+
+        self._system_domain_meta.update([
+            ('vrfs', {'xpath': 'ip-domain/vrfs', 'tag': True, 'operation': 'edit'}),
+            ('vrf', {'xpath': 'ip-domain/vrfs/vrf', 'tag': True, 'operation': 'edit'}),
+            ('a:vrf', {'xpath': 'ip-domain/vrfs/vrf/vrf-name', 'operation': 'edit'}),
+            ('lists', {'xpath': 'ip-domain/vrfs/vrf/lists', 'tag': True, 'operation': 'edit'}),
+            ('list', {'xpath': 'ip-domain/vrfs/vrf/lists/list', 'tag': True, 'operation': 'edit', 'attrib': "operation"}),
+            ('a:order', {'xpath': 'ip-domain/vrfs/vrf/lists/list/order', 'operation': 'edit'}),
+            ('a:domain_search', {'xpath': 'ip-domain/vrfs/vrf/lists/list/list-name', 'operation': 'edit'}),
+        ])
+
+        self._system_server_meta.update([
+            ('vrfs', {'xpath': 'ip-domain/vrfs', 'tag': True, 'operation': 'edit'}),
+            ('vrf', {'xpath': 'ip-domain/vrfs/vrf', 'tag': True, 'operation': 'edit'}),
+            ('a:vrf', {'xpath': 'ip-domain/vrfs/vrf/vrf-name', 'operation': 'edit'}),
+            ('servers', {'xpath': 'ip-domain/vrfs/vrf/servers', 'tag': True, 'operation': 'edit'}),
+            ('server', {'xpath': 'ip-domain/vrfs/vrf/servers/server', 'tag': True, 'operation': 'edit', 'attrib': "operation"}),
+            ('a:order', {'xpath': 'ip-domain/vrfs/vrf/servers/server/order', 'operation': 'edit'}),
+            ('a:name_servers', {'xpath': 'ip-domain/vrfs/vrf/servers/server/server-address', 'operation': 'edit'}),
+        ])
+
+        self._hostname_meta.update([
+            ('a:hostname', {'xpath': 'host-names/host-name', 'operation': 'edit', 'attrib': "operation"}),
+        ])
+
+        self._lookup_source_meta.update([
+            ('vrfs', {'xpath': 'ip-domain/vrfs', 'tag': True, 'operation': 'edit'}),
+            ('vrf', {'xpath': 'ip-domain/vrfs/vrf', 'tag': True, 'operation': 'edit'}),
+            ('a:vrf', {'xpath': 'ip-domain/vrfs/vrf/vrf-name', 'operation': 'edit'}),
+            ('a:lookup_source', {'xpath': 'ip-domain/vrfs/vrf/source-interface', 'operation': 'edit', 'attrib': "operation"}),
+        ])
+
+        self._lookup_meta.update([
+            ('vrfs', {'xpath': 'ip-domain/vrfs', 'tag': True, 'operation': 'edit'}),
+            ('vrf', {'xpath': 'ip-domain/vrfs/vrf', 'tag': True, 'operation': 'edit'}),
+            ('a:vrf', {'xpath': 'ip-domain/vrfs/vrf/vrf-name', 'operation': 'edit'}),
+            ('lookup', {'xpath': 'ip-domain/vrfs/vrf/lookup', 'tag': True, 'operation': 'edit', 'attrib': "operation"}),
+        ])
+
+        state = self._module.params['state']
+        _get_filter = build_xml('ip-domain', opcode="filter")
+        running = get_config(self._module, source='running', config_filter=_get_filter)
+        _get_filter = build_xml('host-names', opcode="filter")
+        hostname_runn = get_config(self._module, source='running', config_filter=_get_filter)
+
+        hostname_ele = etree_find(hostname_runn, 'host-name')
+        hostname = hostname_ele.text if hostname_ele is not None else None
+
+        vrf_ele = etree_findall(running, 'vrf')
+        vrf_map = {}
+        for vrf in vrf_ele:
+            name_server_list = list()
+            domain_list = list()
+            vrf_name_ele = etree_find(vrf, 'vrf-name')
+            vrf_name = vrf_name_ele.text if vrf_name_ele is not None else None
+
+            domain_name_ele = etree_find(vrf, 'name')
+            domain_name = domain_name_ele.text if domain_name_ele is not None else None
+
+            domain_ele = etree_findall(vrf, 'list-name')
+            for domain in domain_ele:
+                domain_list.append(domain.text)
+
+            server_ele = etree_findall(vrf, 'server-address')
+            for server in server_ele:
+                name_server_list.append(server.text)
+
+            lookup_source_ele = etree_find(vrf, 'source-interface')
+            lookup_source = lookup_source_ele.text if lookup_source_ele is not None else None
+
+            lookup_enabled = False if etree_find(vrf, 'lookup') is not None else True
+
+            vrf_map[vrf_name] = {'domain_name': domain_name,
+                                 'domain_search': domain_list,
+                                 'name_servers': name_server_list,
+                                 'lookup_source': lookup_source,
+                                 'lookup_enabled': lookup_enabled}
+
+        opcode = None
+        hostname_param = {}
+        lookup_param = {}
+        system_param = {}
+        sys_server_params = list()
+        sys_domain_params = list()
+        add_domain_params = list()
+        del_domain_params = list()
+        add_server_params = list()
+        del_server_params = list()
+        lookup_source_params = {}
+
+        try:
+            sys_node = vrf_map[self._want['vrf']]
+        except KeyError:
+            sys_node = {'domain_name': None,
+                        'domain_search': [],
+                        'name_servers': [],
+                        'lookup_source': None,
+                        'lookup_enabled': True}
 
+        if state == 'absent':
+            opcode = "delete"
 
-def map_params_to_obj(module):
-    return {
-        'hostname': module.params['hostname'],
-        'domain_name': module.params['domain_name'],
-        'domain_search': module.params['domain_search'],
-        'lookup_source': module.params['lookup_source'],
-        'lookup_enabled': module.params['lookup_enabled'],
-        'name_servers': module.params['name_servers']
-    }
+            def needs_update(x):
+                return self._want[x] is not None and self._want[x] == sys_node[x]
+
+            if needs_update('domain_name'):
+                system_param = {'vrf': self._want['vrf'], 'domain_name': self._want['domain_name']}
+
+            if needs_update('hostname'):
+                hostname_param = {'hostname': hostname}
+
+            if not self._want['lookup_enabled'] and not sys_node['lookup_enabled']:
+                lookup_param['vrf'] = self._want['vrf']
+
+            if needs_update('lookup_source'):
+                lookup_source_params['vrf'] = self._want['vrf']
+                lookup_source_params['lookup_source'] = self._want['lookup_source']
+
+            if self._want['domain_search']:
+                domain_param = {}
+                domain_param['domain_name'] = self._want['domain_name']
+                domain_param['vrf'] = self._want['vrf']
+                domain_param['order'] = '0'
+                for domain in self._want['domain_search']:
+                    if domain in sys_node['domain_search']:
+                        domain_param['domain_search'] = domain
+                        sys_domain_params.append(domain_param.copy())
+
+            if self._want['name_servers']:
+                server_param = {}
+                server_param['vrf'] = self._want['vrf']
+                server_param['order'] = '1'
+                for server in self._want['name_servers']:
+                    if server in sys_node['name_servers']:
+                        server_param['name_servers'] = server
+                        sys_server_params.append(server_param.copy())
+
+        elif state == 'present':
+            opcode = "merge"
+
+            def needs_update(x):
+                return self._want[x] is not None and (sys_node[x] is None or
+                                                      (sys_node[x] is not None and self._want[x] != sys_node[x]))
+
+            if needs_update('domain_name'):
+                system_param = {'vrf': self._want['vrf'], 'domain_name': self._want['domain_name']}
+
+            if self._want['hostname'] is not None and self._want['hostname'] != hostname:
+                hostname_param = {'hostname': self._want['hostname']}
+
+            if not self._want['lookup_enabled'] and sys_node['lookup_enabled']:
+                lookup_param['vrf'] = self._want['vrf']
+
+            if needs_update('lookup_source'):
+                lookup_source_params['vrf'] = self._want['vrf']
+                lookup_source_params['lookup_source'] = self._want['lookup_source']
+
+            if self._want['domain_search']:
+                domain_adds, domain_removes = diff_list(self._want['domain_search'], sys_node['domain_search'])
+                domain_param = {}
+                domain_param['domain_name'] = self._want['domain_name']
+                domain_param['vrf'] = self._want['vrf']
+                domain_param['order'] = '0'
+                for domain in domain_adds:
+                    if domain not in sys_node['domain_search']:
+                        domain_param['domain_search'] = domain
+                        add_domain_params.append(domain_param.copy())
+                for domain in domain_removes:
+                    if domain in sys_node['domain_search']:
+                        domain_param['domain_search'] = domain
+                        del_domain_params.append(domain_param.copy())
+
+            if self._want['name_servers']:
+                server_adds, server_removes = diff_list(self._want['name_servers'], sys_node['name_servers'])
+                server_param = {}
+                server_param['vrf'] = self._want['vrf']
+                server_param['order'] = '1'
+                for domain in server_adds:
+                    if domain not in sys_node['name_servers']:
+                        server_param['name_servers'] = domain
+                        add_server_params.append(server_param.copy())
+                for domain in server_removes:
+                    if domain in sys_node['name_servers']:
+                        server_param['name_servers'] = domain
+                        del_server_params.append(server_param.copy())
+
+        self._result['xml'] = []
+        _edit_filter_list = list()
+        if opcode:
+            if hostname_param:
+                _edit_filter_list.append(build_xml('host-names', xmap=self._hostname_meta,
+                                                   params=hostname_param, opcode=opcode))
+
+            if system_param:
+                _edit_filter_list.append(build_xml('ip-domain', xmap=self._system_meta,
+                                                   params=system_param, opcode=opcode))
+
+            if lookup_source_params:
+                _edit_filter_list.append(build_xml('ip-domain', xmap=self._lookup_source_meta,
+                                                   params=lookup_source_params, opcode=opcode))
+            if lookup_param:
+                _edit_filter_list.append(build_xml('ip-domain', xmap=self._lookup_meta,
+                                                   params=lookup_param, opcode=opcode))
+
+            if opcode == 'delete':
+                if sys_domain_params:
+                    _edit_filter_list.append(build_xml('ip-domain', xmap=self._system_domain_meta,
+                                                       params=sys_domain_params, opcode=opcode))
+                if sys_server_params:
+                    _edit_filter_list.append(build_xml('ip-domain', xmap=self._system_server_meta,
+                                                       params=sys_server_params, opcode=opcode))
+                    if self._want['vrf'] != 'default':
+                        self._result['warnings'] = ["name-servers delete operation with non-default vrf is a success, "
+                                                    "but with rpc-error. Recommended to use 'ignore_errors' option with the task as a workaround"]
+            elif opcode == 'merge':
+                if add_domain_params:
+                    _edit_filter_list.append(build_xml('ip-domain', xmap=self._system_domain_meta,
+                                                       params=add_domain_params, opcode=opcode))
+                if del_domain_params:
+                    _edit_filter_list.append(build_xml('ip-domain', xmap=self._system_domain_meta,
+                                                       params=del_domain_params, opcode="delete"))
+
+                if add_server_params:
+                    _edit_filter_list.append(build_xml('ip-domain', xmap=self._system_server_meta,
+                                                       params=add_server_params, opcode=opcode))
+                if del_server_params:
+                    _edit_filter_list.append(build_xml('ip-domain', xmap=self._system_server_meta,
+                                                       params=del_server_params, opcode="delete"))
+
+        diff = None
+        if _edit_filter_list:
+            commit = not self._module.check_mode
+            diff = load_config(self._module, _edit_filter_list, commit=commit, running=running,
+                               nc_get_filter=_get_filter)
+
+        if diff:
+            if self._module._diff:
+                self._result['diff'] = dict(prepared=diff)
+
+            self._result['xml'] = _edit_filter_list
+            self._result['changed'] = True
+
+    def run(self):
+        self.map_params_to_obj()
+        self.map_obj_to_xml_rpc()
+
+        return self._result
 
 
 def main():
@@ -216,12 +554,13 @@ def main():
     """
     argument_spec = dict(
         hostname=dict(),
+        vrf=dict(type='str', default='default'),
         domain_name=dict(),
         domain_search=dict(type='list'),
 
         name_servers=dict(type='list'),
         lookup_source=dict(),
-        lookup_enabled=dict(type='bool'),
+        lookup_enabled=dict(type='bool', default=True),
 
         state=dict(choices=['present', 'absent'], default='present')
     )
@@ -231,23 +570,17 @@ def main():
     module = AnsibleModule(argument_spec=argument_spec,
                            supports_check_mode=True)
 
-    warnings = list()
-
-    result = {'changed': False, 'warnings': warnings}
-
-    want = map_params_to_obj(module)
-    have = map_config_to_obj(module)
-
-    commands = map_obj_to_commands(want, have, module)
-    result['commands'] = commands
-
-    if commands:
-        commit = not module.check_mode
-        diff = load_config(module, commands, commit=commit)
-        if diff:
-            result['diff'] = dict(prepared=diff)
-        result['changed'] = True
-
+    config_object = None
+    if is_cliconf(module):
+        module.deprecate(msg="cli support for 'iosxr_system' is deprecated. Use transport netconf instead",
+                         version="4 releases from v2.5")
+        config_object = CliConfiguration(module)
+    elif is_netconf(module):
+        config_object = NCConfiguration(module)
+
+    result = None
+    if config_object:
+        result = config_object.run()
     module.exit_json(**result)
 
 

lib/ansible/plugins/netconf/iosxr.py

@@ -107,17 +107,18 @@ class Netconf(NetconfBase):
 
         reply = self.get(install_filter)
         ele_boot_variable = etree.fromstring(reply).find('.//boot-variable/boot-variable')
-        if ele_boot_variable:
+        if ele_boot_variable is not None:
             device_info['network_os_image'] = re.split('[:|,]', ele_boot_variable.text)[1]
         ele_package_name = etree.fromstring(reply).find('.//package-name')
-        if ele_package_name:
+        if ele_package_name is not None:
             device_info['network_os_package'] = ele_package_name.text
             device_info['network_os_version'] = re.split('-', ele_package_name.text)[-1]
 
         hostname_filter = build_xml('host-names', opcode='filter')
 
         reply = self.get(hostname_filter)
-        device_info['network_os_hostname'] = etree.fromstring(reply).find('.//host-name').text
+        hostname_ele = etree.fromstring(reply).find('.//host-name')
+        device_info['network_os_hostname'] = hostname_ele.text if hostname_ele is not None else None
 
         return device_info
 

test/integration/targets/iosxr_system/tasks/main.yaml

@@ -1,2 +1,3 @@
 ---
 - { include: cli.yaml, tags: ['cli'] }
+- { include: netconf.yaml, tags: ['netconf'] }

test/integration/targets/iosxr_system/tasks/netconf.yaml

@@ -0,0 +1,22 @@
+---
+- name: collect all netconf test cases
+  find:
+    paths: "{{ role_path }}/tests/netconf"
+    patterns: "{{ testcase }}.yaml"
+  register: test_cases
+  delegate_to: localhost
+
+- name: set test_items
+  set_fact: test_items="{{ test_cases.files | map(attribute='path') | list }}"
+
+- name: run test case (connection=local)
+  include: "{{ test_case_to_run }} ansible_connection=local"
+  with_items: "{{ test_items }}"
+  loop_control:
+    loop_var: test_case_to_run
+
+# - name: run test case (connection=netconf)
+#  include: "{{ test_case_to_run }} ansible_connection=netconf"
+#  with_items: "{{ test_items }}"
+#  loop_control:
+#    loop_var: test_case_to_run

test/integration/targets/iosxr_system/tests/cli/set_name_servers.yaml

@@ -81,6 +81,13 @@
       - result.commands|length == 1
       - "'no domain name-server 3.3.3.3' in result.commands"
 
-# FIXME: No teardown
-#
+- name: setup
+  iosxr_config:
+    lines:
+      - no ip name-server 1.1.1.1
+      - no ip name-server 2.2.2.2
+      - no ip name-server 3.3.3.3
+    match: none
+    provider: "{{ cli }}"
+
 - debug: msg="END cli/set_name_servers.yaml on connection={{ ansible_connection }}"

test/integration/targets/iosxr_system/tests/netconf/set_domain_list.yaml

@@ -0,0 +1,162 @@
+---
+- debug:
+    msg: "START netconf/set_domain_search.yaml on connection={{ ansible_connection }}"
+
+- block:
+  - name: setup
+    iosxr_config:
+      lines:
+        - no domain list ansible.com
+        - no domain list redhat.com
+      match: none
+      provider: "{{ cli }}"
+
+  - name: configure domain_search
+    iosxr_system:
+      domain_search:
+        - ansible.com
+        - redhat.com
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - result.changed == true
+        - "'ansible.com' in result.xml[0]"
+        - "'redhat.com' in result.xml[0]"
+
+  - name: configure domain_search with vrf
+    iosxr_system: &domainvrf
+      vrf: ansiblevrf
+      domain_search:
+        - redhat.com
+        - ansible.com
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - result.changed == true
+        - "'ansiblevrf' in result.xml[0]"
+        - "'ansible.com' in result.xml[0]"
+        - "'redhat.com' in result.xml[0]"
+
+  - name: verify domain_search with vrf
+    iosxr_system: *domainvrf
+    register: result
+
+  - assert:
+      that:
+        - result.changed == false
+
+  - name: delete domain_search with vrf
+    iosxr_system: &deldomainvrf
+      vrf: ansiblevrf
+      domain_search:
+        - redhat.com
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - result.changed == true
+        - "'ansiblevrf' in result.xml[0]"
+        - "'ansible.com' in result.xml[0]"
+
+  - name: verify delete domain_search with vrf
+    iosxr_system: *deldomainvrf
+    register: result
+
+  - assert:
+      that:
+        - result.changed == false
+
+  - name: remove one entry
+    iosxr_system:
+      domain_search:
+        - ansible.com
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - result.changed == true
+        - "'redhat.com' in result.xml[0]"
+
+  - name: verify remove one entry
+    iosxr_system:
+      domain_search:
+        - ansible.com
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - result.changed == false
+
+  - name: add one entry
+    iosxr_system:
+      domain_search:
+        - ansible.com
+        - redhat.com
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - result.changed == true
+        - "'redhat.com' in result.xml[0]"
+
+  - name: verify add one entry
+    iosxr_system:
+      domain_search:
+        - ansible.com
+        - redhat.com
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - result.changed == false
+
+  - name: add and remove one entry
+    iosxr_system:
+      domain_search:
+        - ansible.com
+        - eng.ansible.com
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - result.changed == true
+        - "'redhat.com' in result.xml[1]"
+        - "'eng.ansible.com' in result.xml[0]"
+        - result.xml|length == 2
+
+  - name: verify add and remove one entry
+    iosxr_system:
+      domain_search:
+        - ansible.com
+        - eng.ansible.com
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - result.changed == false
+
+- always:
+  - name: teardown
+    iosxr_config:
+      lines:
+        - no domain list ansible.com
+        - no domain list redhat.com
+        - no domain list eng.ansible.com
+        - no domain vrf ansiblevrf list redhat.com
+        - no domain vrf ansiblevrf list ansible.com
+      match: none
+      provider: "{{ netconf }}"
+
+  - debug:
+      msg: "END netconf/set_domain_search.yaml on connection={{ ansible_connection }}"

test/integration/targets/iosxr_system/tests/netconf/set_domain_name.yaml

@@ -0,0 +1,79 @@
+---
+- debug:
+    msg: "START netconf/set_domain_name.yaml on connection={{ ansible_connection }}"
+- block:
+  - name: setup
+    iosxr_config:
+      lines:
+        - no domain name
+        - no domain vrf ansiblevrf name
+      match: none
+      provider: "{{ cli }}"
+
+  - name: configure domain_name
+    iosxr_system: &domain
+      domain_name: eng.ansible.com
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - "result.changed == true"
+
+  - name: verify domain_name
+    iosxr_system: *domain
+    register: result
+
+  - assert:
+      that:
+        - "result.changed == false"
+
+  - name: configure domain_name
+    iosxr_system: &deldomain
+      domain_name: eng.ansible.com
+      provider: "{{ netconf }}"
+      state: absent
+    register: result
+
+  - assert:
+      that:
+        - "result.changed == true"
+
+  - name: verify domain_name
+    iosxr_system: *deldomain
+    register: result
+
+  - assert:
+      that:
+        - "result.changed == false"
+
+  - name: configure domain_name with vrf
+    iosxr_system: &domainvrf
+      domain_name: eng.ansible.com
+      vrf: ansiblevrf
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - "result.changed == true"
+
+  - name: verify domain_name with vrf
+    iosxr_system: *domainvrf
+    register: result
+
+  - assert:
+      that:
+        - "result.changed == false"
+
+- always:
+  - name: teardown
+    iosxr_config:
+      lines:
+        - no domain name
+        - no domain vrf ansiblevrf name
+      match: none
+      provider: "{{ cli }}"
+
+  - debug:
+      msg: "END netconf/set_domain_name.yaml on connection={{ ansible_connection }}"

test/integration/targets/iosxr_system/tests/netconf/set_hostname.yaml

@@ -0,0 +1,39 @@
+---
+- debug:
+    msg: "START netconf/set_hostname.yaml on connection={{ ansible_connection }}"
+
+- block:
+  - name: setup
+    iosxr_config:
+      lines: hostname switch
+      match: none
+      provider: "{{ cli }}"
+
+  - name: configure hostname
+    iosxr_system:
+      hostname: foo
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - "result.changed == true"
+
+  - name: verify hostname
+    iosxr_system:
+      hostname: foo
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - "result.changed == false"
+- always:
+  - name: teardown
+    iosxr_config:
+      lines: "hostname {{ inventory_hostname }}"
+      match: none
+      provider: "{{ cli }}"
+
+- debug:
+    msg: "END netconf/set_hostname.yaml on connection={{ ansible_connection }}"

test/integration/targets/iosxr_system/tests/netconf/set_lookup_source.yaml

@@ -0,0 +1,150 @@
+---
+- debug:
+    msg: "START netconf/set_lookup_source.yaml on connection={{ ansible_connection }}"
+
+- block:
+  - name: setup
+    iosxr_config:
+      lines:
+        - no domain lookup source-interface Loopback10
+        - no domain vrf ansiblevrf lookup source-interface Loopback10
+        - no domain lookup disable
+        - no domain vrf ansiblevrf lookup disable
+      match: none
+      provider: "{{ cli }}"
+
+  - name: configure lookup_source
+    iosxr_system: &lookup
+      lookup_source: Loopback10
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - result.changed == true
+        - "'Loopback10' in result.xml[0]"
+
+  - name: verify lookup_source
+    iosxr_system: *lookup
+    register: result
+
+  - assert:
+      that:
+        - result.changed == false
+
+  - name: disable lookup
+    iosxr_system: &disable
+      lookup_enabled: False 
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - result.changed == true
+        - "'lookup' in result.xml[0]"
+
+  - name: verify disable lookup
+    iosxr_system: *disable
+    register: result
+
+  - assert:
+      that:
+        - result.changed == false
+
+  - name: delete lookup_source
+    iosxr_system: &dellookup
+      lookup_source: Loopback10
+      provider: "{{ netconf }}"
+      state: absent
+    register: result
+
+  - assert:
+      that:
+        - result.changed == true
+        - "'Loopback10' in result.xml[0]"
+
+  - name: verify lookup_source
+    iosxr_system: *dellookup
+    register: result
+
+  - assert:
+      that:
+        - result.changed == false
+
+  - name: configure lookup_source with vrf
+    iosxr_system: &lookupvrf
+      lookup_source: Loopback10
+      vrf: ansiblevrf
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - result.changed == true
+        - "'Loopback10' in result.xml[0]"
+        - "'ansiblevrf' in result.xml[0]"
+
+  - name: verify lookup_source
+    iosxr_system: *lookupvrf
+    register: result
+
+  - assert:
+      that:
+        - result.changed == false
+
+  - name: disable lookup
+    iosxr_system: &disablevrf
+      lookup_enabled: False 
+      vrf: ansiblevrf
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - result.changed == true
+        - "'lookup' in result.xml[0]"
+        - "'ansiblevrf' in result.xml[0]"
+
+  - name: verify disable lookup
+    iosxr_system: *disablevrf
+    register: result
+
+  - assert:
+      that:
+        - result.changed == false
+
+  - name: delete lookup_source
+    iosxr_system: &dellookupvrf
+      lookup_source: Loopback10
+      vrf: ansiblevrf
+      provider: "{{ netconf }}"
+      state: absent
+    register: result
+
+  - assert:
+      that:
+        - result.changed == true
+        - "'Loopback10' in result.xml[0]"
+        - "'ansiblevrf' in result.xml[0]"
+
+  - name: verify lookup_source
+    iosxr_system: *dellookupvrf
+    register: result
+
+  - assert:
+      that:
+        - result.changed == false
+
+- always:
+  - name: teardown
+    iosxr_config:
+      lines:
+        - no domain lookup source-interface Loopback10
+        - no domain vrf ansiblevrf lookup source-interface Loopback10
+        - no domain lookup disable
+        - no domain vrf ansiblevrf lookup disable
+      match: none
+      provider: "{{ cli }}"
+
+  - debug:
+      msg: "END netconf/set_lookup_source.yaml on connection={{ ansible_connection }}"

test/integration/targets/iosxr_system/tests/netconf/set_name_servers.yaml

@@ -0,0 +1,120 @@
+---
+- debug: 
+    msg: "START netconf/set_name_servers.yaml on connection={{ ansible_connection }}"
+- block:
+  - name: configure name_servers
+    iosxr_system:
+      name_servers:
+        - 1.1.1.1
+        - 2.2.2.2
+        - 3.3.3.3
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - result.changed == true
+        - result.xml|length == 1
+        - "'1.1.1.1' in result.xml[0]"
+        - "'2.2.2.2' in result.xml[0]"
+        - "'3.3.3.3' in result.xml[0]"
+
+  - name: verify name_servers
+    iosxr_system:
+      name_servers:
+        - 1.1.1.1
+        - 2.2.2.2
+        - 3.3.3.3
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - result.changed == false
+
+  - name: add with to vrf
+    iosxr_system: &addvrf
+      vrf: ansible
+      name_servers:
+        - 1.1.1.1
+        - 2.2.2.2
+        - 3.3.3.3
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - result.changed == true
+        - result.xml|length == 1
+        - "'ansible' in result.xml[0]"
+        - "'1.1.1.1' in result.xml[0]"
+        - "'2.2.2.2' in result.xml[0]"
+        - "'3.3.3.3' in result.xml[0]"
+
+  - name: verify change to vrf
+    iosxr_system: *addvrf
+    register: result
+
+  - assert:
+      that:
+        - result.changed == false
+
+  - name: remove one
+    iosxr_system:
+      name_servers:
+        - 1.1.1.1
+        - 2.2.2.2
+      provider: "{{ netconf }}"
+    register: result
+
+  - assert:
+      that:
+        - result.changed == true
+        - result.xml|length == 1
+        - "'3.3.3.3' in result.xml[0]"
+
+## multiple name-servers deletion commands doesn't work in single 
+# config session (only the 1st one takes effect). May or may not be 
+# a VIRL software issue.
+- always:
+  - name: setup
+    iosxr_config:
+      lines: no domain name-server 1.1.1.1
+      match: none
+      provider: "{{ cli }}"
+
+  - name: setup
+    iosxr_config:
+      lines: no domain name-server 2.2.2.2
+      match: none
+      provider: "{{ cli }}"
+
+  - name: setup
+    iosxr_config:
+      lines: no domain name-server 3.3.3.3
+      match: none
+      provider: "{{ cli }}"
+
+  - name: setup
+    iosxr_config:
+      lines: no domain vrf ansible name-server 1.1.1.1
+      match: none
+      provider: "{{ cli }}"
+    ignore_errors: true
+
+  - name: setup
+    iosxr_config:
+      lines: no domain vrf ansible name-server 2.2.2.2
+      match: none
+      provider: "{{ cli }}"
+    ignore_errors: true
+
+  - name: setup
+    iosxr_config:
+      lines: no domain vrf ansible name-server 3.3.3.3
+      match: none
+      provider: "{{ cli }}"
+    ignore_errors: true
+
+  - debug: 
+      msg: "END netconf/set_name_servers.yaml on connection={{ ansible_connection }}"

test/units/modules/network/iosxr/test_iosxr_system.py

@@ -38,6 +38,9 @@ class TestIosxrSystemModule(TestIosxrModule):
         self.mock_load_config = patch('ansible.modules.network.iosxr.iosxr_system.load_config')
         self.load_config = self.mock_load_config.start()
 
+        self.mock_is_cliconf = patch('ansible.modules.network.iosxr.iosxr_system.is_cliconf')
+        self.is_cliconf = self.mock_is_cliconf.start()
+
     def tearDown(self):
         super(TestIosxrSystemModule, self).tearDown()
 
@@ -47,25 +50,26 @@ class TestIosxrSystemModule(TestIosxrModule):
     def load_fixtures(self, commands=None):
         self.get_config.return_value = load_fixture('iosxr_system_config.cfg')
         self.load_config.return_value = dict(diff=None, session='session')
+        self.is_cliconf.return_value = True
 
     def test_iosxr_system_hostname_changed(self):
         set_module_args(dict(hostname='foo'))
-        commands = ['hostname foo']
+        commands = ['hostname foo', 'no domain lookup disable']
         self.execute_module(changed=True, commands=commands)
 
     def test_iosxr_system_domain_name(self):
         set_module_args(dict(domain_name='test.com'))
-        commands = ['domain name test.com']
+        commands = ['domain name test.com', 'no domain lookup disable']
         self.execute_module(changed=True, commands=commands)
 
     def test_iosxr_system_domain_search(self):
         set_module_args(dict(domain_search=['ansible.com', 'redhat.com']))
-        commands = ['domain list ansible.com', 'no domain list cisco.com']
+        commands = ['domain list ansible.com', 'no domain list cisco.com', 'no domain lookup disable']
         self.execute_module(changed=True, commands=commands)
 
     def test_iosxr_system_lookup_source(self):
         set_module_args(dict(lookup_source='Ethernet1'))
-        commands = ['domain lookup source-interface Ethernet1']
+        commands = ['domain lookup source-interface Ethernet1', 'no domain lookup disable']
         self.execute_module(changed=True, commands=commands)
 
     def test_iosxr_system_lookup_enabled(self):
@@ -76,7 +80,7 @@ class TestIosxrSystemModule(TestIosxrModule):
     def test_iosxr_system_name_servers(self):
         name_servers = ['8.8.8.8', '8.8.4.4', '1.1.1.1']
         set_module_args(dict(name_servers=name_servers))
-        commands = ['domain name-server 1.1.1.1', 'no domain name-server 8.8.4.4']
+        commands = ['domain name-server 1.1.1.1', 'no domain name-server 8.8.4.4', 'no domain lookup disable']
         self.execute_module(changed=True)
 
     def test_iosxr_system_state_absent(self):
@@ -94,5 +98,5 @@ class TestIosxrSystemModule(TestIosxrModule):
         self.execute_module(changed=True, commands=commands)
 
     def test_iosxr_system_no_change(self):
-        set_module_args(dict(hostname='iosxr01', domain_name='eng.ansible.com'))
+        set_module_args(dict(hostname='iosxr01', domain_name='eng.ansible.com', lookup_enabled=False))
         self.execute_module()