From 93acd7c651bc8c5135a89de6d29f02927048117e Mon Sep 17 00:00:00 2001 From: Kedar Kekan <4506537+kedarX@users.noreply.github.com> Date: Wed, 17 Jan 2018 19:59:28 +0530 Subject: [PATCH] refactor iosxr_system for cliconf and netconf (#34749) * * refactor iosxr_system for cliconf and netconf * * Fix unit tests and sanity issues --- .../module_utils/network/common/netconf.py | 12 +- .../module_utils/network/iosxr/iosxr.py | 11 +- .../modules/network/iosxr/iosxr_system.py | 541 ++++++++++++++---- lib/ansible/plugins/netconf/iosxr.py | 7 +- .../targets/iosxr_system/tasks/main.yaml | 1 + .../targets/iosxr_system/tasks/netconf.yaml | 22 + .../tests/cli/set_name_servers.yaml | 11 +- .../tests/netconf/set_domain_list.yaml | 162 ++++++ .../tests/netconf/set_domain_name.yaml | 79 +++ .../tests/netconf/set_hostname.yaml | 39 ++ .../tests/netconf/set_lookup_source.yaml | 150 +++++ .../tests/netconf/set_name_servers.yaml | 120 ++++ .../network/iosxr/test_iosxr_system.py | 16 +- 13 files changed, 1044 insertions(+), 127 deletions(-) create mode 100644 test/integration/targets/iosxr_system/tasks/netconf.yaml create mode 100644 test/integration/targets/iosxr_system/tests/netconf/set_domain_list.yaml create mode 100644 test/integration/targets/iosxr_system/tests/netconf/set_domain_name.yaml create mode 100644 test/integration/targets/iosxr_system/tests/netconf/set_hostname.yaml create mode 100644 test/integration/targets/iosxr_system/tests/netconf/set_lookup_source.yaml create mode 100644 test/integration/targets/iosxr_system/tests/netconf/set_name_servers.yaml diff --git a/lib/ansible/module_utils/network/common/netconf.py b/lib/ansible/module_utils/network/common/netconf.py index 2cec50a8da2..a2d6b5427a5 100644 --- a/lib/ansible/module_utils/network/common/netconf.py +++ b/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')) diff --git a/lib/ansible/module_utils/network/iosxr/iosxr.py b/lib/ansible/module_utils/network/iosxr/iosxr.py index 8021ba598d8..126dc1928d1 100644 --- a/lib/ansible/module_utils/network/iosxr/iosxr.py +++ b/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) diff --git a/lib/ansible/modules/network/iosxr/iosxr_system.py b/lib/ansible/modules/network/iosxr/iosxr_system.py index 25245f230cd..c13292d98de 100644 --- a/lib/ansible/modules/network/iosxr/iosxr_system.py +++ b/lib/ansible/modules/network/iosxr/iosxr_system.py @@ -15,20 +15,28 @@ 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 - on Cisco IOS XR devices. It provides an option to configure host system + on Cisco IOS XR devices. It provides an option to configure host system parameters or remove those parameters from the device active 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: + - ' + + + + default + + + 0 + redhat.com + + + + + + ' """ + 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): - commands = list() - state = module.params['state'] +class ConfigBase(object): + def __init__(self, module): + self._module = module + self._result = {'changed': False, 'warnings': []} + self._want = dict() + self._have = dict() - def needs_update(x): - return want.get(x) and (want.get(x) != have.get(x)) - - if state == 'absent': - if have['hostname'] != 'ios': - commands.append('no hostname') - if 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']: - 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) - - elif state == 'present': - if needs_update('hostname'): - commands.append('hostname %s' % want['hostname']) - - if needs_update('domain_name'): - commands.append('domain name %s' % want['domain_name']) - - if needs_update('lookup_source'): - commands.append('domain lookup source-interface %s' % want['lookup_source']) - - if needs_update('lookup_enabled'): - cmd = 'domain lookup disable' - if want['lookup_enabled']: - cmd = 'no %s' % cmd - commands.append(cmd) - - if want['name_servers'] is not None: - adds, removes = diff_list(want['name_servers'], have['name_servers']) - for item in adds: - commands.append('domain name-server %s' % item) - for item in removes: - commands.append('no domain name-server %s' % item) - - if want['domain_search'] is not None: - adds, removes = diff_list(want['domain_search'], have['domain_search']) - for item in adds: - commands.append('domain list %s' % item) - for item in removes: - commands.append('no domain list %s' % item) - - return commands + 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'] + }) -def parse_hostname(config): - match = re.search(r'^hostname (\S+)', config, re.M) - return match.group(1) +class CliConfiguration(ConfigBase): + def __init__(self, module): + super(CliConfiguration, self).__init__(module) + def map_obj_to_commands(self): + commands = list() + state = self._module.params['state'] -def parse_domain_name(config): - match = re.search(r'^domain name (\S+)', config, re.M) - if match: + def needs_update(x): + return self._want.get(x) and (self._want.get(x) != self._have.get(x)) + + if state == 'absent': + if self._have['hostname'] != 'ios': + commands.append('no hostname') + if self._have['domain_name']: + commands.append('no domain name') + 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 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 {0!s}'.format(self._want['hostname'])) + + if needs_update('domain_name'): + commands.append('domain name {0!s}'.format(self._want['domain_name'])) + + if needs_update('lookup_source'): + commands.append('domain lookup source-interface {0!s}'.format(self._want['lookup_source'])) + + cmd = None + if not self._want['lookup_enabled'] and self._have['lookup_enabled']: + cmd = 'domain lookup disable' + 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 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 {0!s}'.format(item)) + for item in removes: + commands.append('no domain name-server {0!s}'.format(item)) + + 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 {0!s}'.format(item)) + for item in removes: + commands.append('no domain list {0!s}'.format(item)) + + 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(self, config): + match = re.search(r'^hostname (\S+)', config, re.M) return match.group(1) + 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): - match = re.search(r'^domain lookup source-interface (\S+)', config, re.M) - if match: - return match.group(1) + 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(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': 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 -def map_config_to_obj(module): - config = get_config(module) - return { - 'hostname': parse_hostname(config), - 'domain_name': parse_domain_name(config), - 'domain_search': re.findall(r'^domain list (\S+)', config, re.M), - 'lookup_source': parse_lookup_source(config), - 'lookup_enabled': 'domain lookup disable' not in config, - 'name_servers': re.findall(r'^domain name-server (\S+)', config, re.M) - } +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"}), + ]) -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'] - } + 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 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) diff --git a/lib/ansible/plugins/netconf/iosxr.py b/lib/ansible/plugins/netconf/iosxr.py index eee3eade142..13216bba2ac 100644 --- a/lib/ansible/plugins/netconf/iosxr.py +++ b/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 diff --git a/test/integration/targets/iosxr_system/tasks/main.yaml b/test/integration/targets/iosxr_system/tasks/main.yaml index 415c99d8b12..af08869c922 100644 --- a/test/integration/targets/iosxr_system/tasks/main.yaml +++ b/test/integration/targets/iosxr_system/tasks/main.yaml @@ -1,2 +1,3 @@ --- - { include: cli.yaml, tags: ['cli'] } +- { include: netconf.yaml, tags: ['netconf'] } diff --git a/test/integration/targets/iosxr_system/tasks/netconf.yaml b/test/integration/targets/iosxr_system/tasks/netconf.yaml new file mode 100644 index 00000000000..986e2639534 --- /dev/null +++ b/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 diff --git a/test/integration/targets/iosxr_system/tests/cli/set_name_servers.yaml b/test/integration/targets/iosxr_system/tests/cli/set_name_servers.yaml index 22b186e7a50..cb6e4a82e4b 100644 --- a/test/integration/targets/iosxr_system/tests/cli/set_name_servers.yaml +++ b/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 }}" diff --git a/test/integration/targets/iosxr_system/tests/netconf/set_domain_list.yaml b/test/integration/targets/iosxr_system/tests/netconf/set_domain_list.yaml new file mode 100644 index 00000000000..ce382e856ae --- /dev/null +++ b/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 }}" diff --git a/test/integration/targets/iosxr_system/tests/netconf/set_domain_name.yaml b/test/integration/targets/iosxr_system/tests/netconf/set_domain_name.yaml new file mode 100644 index 00000000000..98561e29765 --- /dev/null +++ b/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 }}" diff --git a/test/integration/targets/iosxr_system/tests/netconf/set_hostname.yaml b/test/integration/targets/iosxr_system/tests/netconf/set_hostname.yaml new file mode 100644 index 00000000000..7da79253e79 --- /dev/null +++ b/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 }}" diff --git a/test/integration/targets/iosxr_system/tests/netconf/set_lookup_source.yaml b/test/integration/targets/iosxr_system/tests/netconf/set_lookup_source.yaml new file mode 100644 index 00000000000..9f855ac27b3 --- /dev/null +++ b/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 }}" diff --git a/test/integration/targets/iosxr_system/tests/netconf/set_name_servers.yaml b/test/integration/targets/iosxr_system/tests/netconf/set_name_servers.yaml new file mode 100644 index 00000000000..66a484ef204 --- /dev/null +++ b/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 }}" diff --git a/test/units/modules/network/iosxr/test_iosxr_system.py b/test/units/modules/network/iosxr/test_iosxr_system.py index de5029eb8fe..6cf95bdfb70 100644 --- a/test/units/modules/network/iosxr/test_iosxr_system.py +++ b/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()