From 2b2072a8c9df1d90e9d2a7fcfd7a62e650055ed9 Mon Sep 17 00:00:00 2001 From: Peter Sprygada Date: Sun, 12 Mar 2017 11:45:00 -0500 Subject: [PATCH] roll up of fixes and updates for junos modules (#22543) * removes cli functions * adds comment and confirm to arguments * implements zeroize argument * fixes get_diff function in junos shared lib to return diff * lots of minor bug fixes in junos_config * minor syntax fixes in junos_netconf * updates netconf integration tests --- lib/ansible/module_utils/junos.py | 16 +-- .../modules/network/junos/junos_config.py | 107 +++++++++--------- .../modules/network/junos/junos_netconf.py | 23 ++-- .../junos_config/tests/netconf/src_basic.yaml | 4 - .../junos_netconf/tests/cli/changeport.yaml | 15 +-- .../junos_netconf/tests/cli/netconf.yaml | 13 ++- 6 files changed, 86 insertions(+), 92 deletions(-) diff --git a/lib/ansible/module_utils/junos.py b/lib/ansible/module_utils/junos.py index 17873b39373..accec56fc30 100644 --- a/lib/ansible/module_utils/junos.py +++ b/lib/ansible/module_utils/junos.py @@ -21,7 +21,7 @@ from contextlib import contextmanager from xml.etree.ElementTree import Element, SubElement, tostring from ansible.module_utils.basic import env_fallback -from ansible.module_utils.netconf import send_request +from ansible.module_utils.netconf import send_request, children from ansible.module_utils.netconf import discard_changes, validate from ansible.module_utils.network_common import to_list from ansible.module_utils.connection import exec_command @@ -91,7 +91,7 @@ def load_configuration(module, candidate=None, action='merge', rollback=None, fo cfg.text = '\n'.join(candidate) else: cfg = SubElement(obj, lookup[format]) - cfg.append(candidate) + cfg.text = '\n'.join(candidate) return send_request(module, obj) @@ -112,9 +112,11 @@ def commit_configuration(module, confirm=False, check=False, comment=None, confi if check: SubElement(obj, 'check') if comment: - children(obj, ('log', str(comment))) + subele = SubElement(obj, 'log') + subele.text = str(comment) if confirm_timeout: - children(obj, ('confirm-timeout', int(confirm_timeout))) + subele = SubElement(obj, 'confirm-timeout') + subele.text = int(confirm_timeout) return send_request(module, obj) def command(module, command, format='text', rpc_only=False): @@ -138,14 +140,14 @@ def locked_config(module): def get_diff(module): reply = get_configuration(module, compare=True, format='text') output = reply.find('.//configuration-output') - if output: - return output[0].text + if output is not None: + return output.text def load_config(module, candidate, action='merge', commit=False, format='xml', comment=None, confirm=False, confirm_timeout=None): with locked_config(module): - resp = load_configuration(module, candidate, action=action, format=format) + reply = load_configuration(module, candidate, action=action, format=format) validate(module) diff = get_diff(module) diff --git a/lib/ansible/modules/network/junos/junos_config.py b/lib/ansible/modules/network/junos/junos_config.py index 66a7ec6a589..73f4815420b 100644 --- a/lib/ansible/modules/network/junos/junos_config.py +++ b/lib/ansible/modules/network/junos/junos_config.py @@ -180,31 +180,24 @@ import json from xml.etree import ElementTree from ansible.module_utils.basic import AnsibleModule -from ansible.module_utils.junos import get_config, get_diff, load_config +from ansible.module_utils.junos import get_diff, load_config, get_configuration from ansible.module_utils.junos import junos_argument_spec from ansible.module_utils.junos import check_args as junos_check_args -from ansible.module_utils.netcfg import NetworkConfig +from ansible.module_utils.netconf import send_request from ansible.module_utils.six import string_types USE_PERSISTENT_CONNECTION = True DEFAULT_COMMENT = 'configured by junos_config' -def check_transport(module): - transport = (module.params['provider'] or {}).get('transport') - - if transport == 'netconf': - module.fail_json(msg='junos_config module is only supported over cli transport') - - def check_args(module, warnings): junos_check_args(module, warnings) - if module.params['zeroize']: - module.fail_json(msg='argument zeroize is deprecated and no longer ' - 'supported, use junos_command instead') if module.params['replace'] is not None: module.fail_json(msg='argument replace is deprecated, use update') +zeroize = lambda x: send_request(x, Element('request-system-zeroize')) +rollback = lambda x: get_diff(x) + def guess_format(config): try: json.loads(config) @@ -223,27 +216,15 @@ def guess_format(config): return 'text' -def config_to_commands(config): - set_format = config.startswith('set') or config.startswith('delete') - candidate = NetworkConfig(indent=4, contents=config) - if not set_format: - candidate = [c.line for c in candidate.items] - commands = list() - # this filters out less specific lines - for item in candidate: - for index, entry in enumerate(commands): - if item.startswith(entry): - del commands[index] - break - commands.append(item) - - else: - commands = str(candidate).split('\n') - - return commands - def filter_delete_statements(module, candidate): - config = get_config(module) + reply = get_configuration(module, format='set') + match = reply.find('.//configuration-set') + if match is None: + module.fail_json(msg='unable to retrieve device configuration') + config = str(match.text) + + #if 'delete interfaces lo0' in candidate: + # raise ValueError(config) modified_candidate = candidate[:] for index, line in enumerate(candidate): @@ -251,39 +232,43 @@ def filter_delete_statements(module, candidate): newline = re.sub('^delete', 'set', line) if newline not in config: del modified_candidate[index] + return modified_candidate -def load(module): +def configure_device(module): candidate = module.params['lines'] or module.params['src'] if isinstance(candidate, string_types): candidate = candidate.split('\n') kwargs = { - 'confirm': module.params['confirm'] is not None, - 'confirm_timeout': module.params['confirm'], 'comment': module.params['comment'], - 'commit': not module.check_mode, + 'commit': not module.check_mode } + if module.params['confirm'] > 0: + kwargs.update({ + 'confirm': True, + 'confirm_timeout': module.params['confirm'] + }) + + config_format = None + if module.params['src']: config_format = module.params['src_format'] or guess_format(str(candidate)) - kwargs.update({'format': config_format, 'action': module.params['update']}) + if config_format == 'set': + kwargs.update({'format': 'text', 'action': 'set'}) + else: + kwargs.update({'format': config_format, 'action': module.params['update']}) # this is done to filter out `delete ...` statements which map to # nothing in the config as that will cause an exception to be raised - if module.params['lines']: + if any((module.params['lines'], config_format == 'set')): candidate = filter_delete_statements(module, candidate) + kwargs['format'] = 'text' + kwargs['action'] = 'set' return load_config(module, candidate, **kwargs) -def update_result(module, result, diff=None): - if diff == '': - diff = None - result['changed'] = diff is not None - if module._diff: - result['diff'] = {'prepared': diff} - - def main(): """ main entry point for module execution """ @@ -306,35 +291,47 @@ def main(): backup=dict(type='bool', default=False), rollback=dict(type='int'), - # deprecated zeroize in Ansible 2.3 zeroize=dict(default=False, type='bool'), ) argument_spec.update(junos_argument_spec) - mutually_exclusive = [('lines', 'src', 'rollback')] + mutually_exclusive = [('lines', 'src', 'rollback', 'zeroize')] module = AnsibleModule(argument_spec=argument_spec, mutually_exclusive=mutually_exclusive, supports_check_mode=True) - check_transport(module) - warnings = list() check_args(module, warnings) result = {'changed': False, 'warnings': warnings} if module.params['backup']: - result['__backup__'] = get_config(module) + reply = get_configuration(module, format='set') + match = reply.find('.//configuration-set') + if match is None: + module.fail_json(msg='unable to retrieve device configuration') + result['__backup__'] = str(match.text).strip() if module.params['rollback']: - diff = get_diff(module) - update_result(module, result, diff) + if not module.check_mode: + diff = rollback(module) + if module._diff: + result['diff'] = {'prepared': diff} + result['changed'] = True + + elif module.params['zeroize']: + if not module.check_mode: + zeroize(module) + result['changed'] = True else: - diff = load(module) - update_result(module, result, diff) + diff = configure_device(module) + if diff: + if module._diff: + result['diff'] = {'prepared': diff} + result['changed'] = True module.exit_json(**result) diff --git a/lib/ansible/modules/network/junos/junos_netconf.py b/lib/ansible/modules/network/junos/junos_netconf.py index acf860ed2fe..292c339d799 100644 --- a/lib/ansible/modules/network/junos/junos_netconf.py +++ b/lib/ansible/modules/network/junos/junos_netconf.py @@ -80,6 +80,7 @@ import re from ansible.module_utils.junos import junos_argument_spec, check_args from ansible.module_utils.basic import AnsibleModule from ansible.module_utils.connection import exec_command +from ansible.module_utils.network_common import to_list from ansible.module_utils.six import iteritems USE_PERSISTENT_CONNECTION = True @@ -97,10 +98,11 @@ def map_obj_to_commands(updates, module): elif want['state'] == 'absent' and have['state'] == 'present': commands.append('delete system services netconf') - elif want['netconf_port'] != have.get('netconf_port'): - commands.append( - 'set system services netconf ssh port %s' % want['netconf_port'] - ) + elif want['state'] == 'present': + if want['netconf_port'] != have.get('netconf_port'): + commands.append( + 'set system services netconf ssh port %s' % want['netconf_port'] + ) return commands @@ -110,7 +112,12 @@ def parse_port(config): return int(match.group(1)) def map_config_to_obj(module): - config = get_config(module, ['system services netconf']) + cmd = 'show configuration system services netconf' + rc, out, err = exec_command(module, cmd) + if rc != 0: + module.fail_json(msg='unable to retrieve current config', stderr=err) + config = str(out).strip() + obj = {'state': 'absent'} if config: obj.update({ @@ -138,12 +145,6 @@ def map_params_to_obj(module): return obj -def get_config(module, flags=[]): - rc, out, err = exec_command(module, cmd) - if rc != 0: - module.fail_json(msg='unable to retrieve current config', stderr=err) - return str(out).strip() - def load_config(module, config, commit=False): exec_command(module, 'configure') diff --git a/test/integration/targets/junos_config/tests/netconf/src_basic.yaml b/test/integration/targets/junos_config/tests/netconf/src_basic.yaml index fdf0cd3b234..c4bfbfd667e 100644 --- a/test/integration/targets/junos_config/tests/netconf/src_basic.yaml +++ b/test/integration/targets/junos_config/tests/netconf/src_basic.yaml @@ -17,8 +17,6 @@ - assert: that: - "result.changed == true" -# https://github.com/ansible/ansible-modules-core/issues/4807 - - "result.updates is not defined" - name: check device with config junos_config: @@ -29,7 +27,5 @@ - assert: that: - "result.changed == false" -# https://github.com/ansible/ansible-modules-core/issues/4807 - - "result.updates is not defined" - debug: msg="END netconf/src_basic.yaml" diff --git a/test/integration/targets/junos_netconf/tests/cli/changeport.yaml b/test/integration/targets/junos_netconf/tests/cli/changeport.yaml index 3051a30ad65..ce153fad924 100644 --- a/test/integration/targets/junos_netconf/tests/cli/changeport.yaml +++ b/test/integration/targets/junos_netconf/tests/cli/changeport.yaml @@ -33,17 +33,15 @@ - "result.changed == false" - name: Ensure we can communicate over 8080 - junos_config: - lines: - - set system host-name {{ inventory_hostname_short }} + junos_command: + rpcs: get-software-information provider: "{{ netconf }}" port: 8080 # This protects against the port override above not being honoured and a bug setting the port - name: Ensure we can NOT communicate over default port - junos_config: - lines: - - set system host-name {{ inventory_hostname_short }} + junos_command: + rpcs: get-software-information provider: "{{ netconf }}" register: result ignore_errors: true @@ -60,9 +58,8 @@ register: result - name: Ensure we can communicate over netconf - junos_config: - lines: - - set system host-name {{ inventory_hostname_short }} + junos_command: + rpcs: get-software-information provider: "{{ netconf }}" - debug: msg="END netconf/changeport.yaml" diff --git a/test/integration/targets/junos_netconf/tests/cli/netconf.yaml b/test/integration/targets/junos_netconf/tests/cli/netconf.yaml index c01f21c19b4..56bf64ce8be 100644 --- a/test/integration/targets/junos_netconf/tests/cli/netconf.yaml +++ b/test/integration/targets/junos_netconf/tests/cli/netconf.yaml @@ -21,9 +21,8 @@ ################################### - name: Ensure we can communicate over netconf - junos_config: - lines: - - set system host-name {{ inventory_hostname_short }} + junos_command: + rpcs: get-software-information provider: "{{ netconf }}" # Disable netconf @@ -48,11 +47,13 @@ that: - "result.changed == false" +- name: wait for persistent socket to timeout + pause: + seconds: 30 - name: Ensure we can NOT talk via netconf - junos_config: - lines: - - set system host-name {{ inventory_hostname_short }} + junos_command: + rpcs: get-software-information provider: "{{ netconf }}" register: result ignore_errors: true