From 5be32aa5af6688d8732818d16e0aefdb1b5153ed Mon Sep 17 00:00:00 2001 From: Dag Wieers Date: Wed, 28 Jun 2017 19:08:04 +0100 Subject: [PATCH] Move ping and win_ping closer together (#26028) So in an effort to verify if Windows modules are feature complete compared to the python equivalent, I stumbled upon these differences. This PR includes: - Add missing 'data' option from documentation - Simplify ping module - Update integration tests to test exception --- lib/ansible/modules/system/ping.py | 46 ++++++++--- lib/ansible/modules/windows/win_ping.py | 19 +++-- test/integration/targets/ping/tasks/main.yml | 26 ++++-- .../targets/win_ping/tasks/main.yml | 82 +++++++++++-------- 4 files changed, 117 insertions(+), 56 deletions(-) diff --git a/lib/ansible/modules/system/ping.py b/lib/ansible/modules/system/ping.py index e59901f0bd3..9b0a61b92d2 100644 --- a/lib/ansible/modules/system/ping.py +++ b/lib/ansible/modules/system/ping.py @@ -24,12 +24,11 @@ ANSIBLE_METADATA = {'metadata_version': '1.0', 'status': ['stableinterface'], 'supported_by': 'core'} - DOCUMENTATION = ''' --- module: ping version_added: historical -short_description: Try to connect to host, verify a usable python and return C(pong) on success. +short_description: Try to connect to host, verify a usable python and return C(pong) on success description: - A trivial test module, this module always returns C(pong) on successful contact. It does not make sense in playbooks, but it is useful from @@ -38,15 +37,35 @@ description: - For Windows targets, use the M(ping) module instead. notes: - For Windows targets, use the M(ping) module instead. -options: {} +options: + data: + description: + - Data to return for the C(ping) return value. + - If this parameter is set to C(crash), the module will cause an exception. + default: pong author: - - "Ansible Core Team" - - "Michael DeHaan" + - Ansible Core Team + - Michael DeHaan ''' EXAMPLES = ''' # Test we can logon to 'webservers' and execute python with json lib. -ansible webservers -m ping +# ansible webservers -m ping + +# Example from an Ansible Playbook +- ping: + +# Induce an exception to see what happens +- ping: + data: crash +''' + +RETURN = ''' +ping: + description: value provided with the data parameter + returned: success + type: string + sample: pong ''' from ansible.module_utils.basic import AnsibleModule @@ -55,15 +74,18 @@ from ansible.module_utils.basic import AnsibleModule def main(): module = AnsibleModule( argument_spec=dict( - data=dict(required=False, default=None), + data=dict(type='str', default='pong'), ), supports_check_mode=True ) - result = dict(ping='pong') - if module.params['data']: - if module.params['data'] == 'crash': - raise Exception("boom") - result['ping'] = module.params['data'] + + if module.params['data'] == 'crash': + raise Exception("boom") + + result = dict( + ping=module.params['data'], + ) + module.exit_json(**result) diff --git a/lib/ansible/modules/windows/win_ping.py b/lib/ansible/modules/windows/win_ping.py index 1a2f0a7f71f..504a1b64d6d 100644 --- a/lib/ansible/modules/windows/win_ping.py +++ b/lib/ansible/modules/windows/win_ping.py @@ -25,7 +25,6 @@ ANSIBLE_METADATA = {'metadata_version': '1.0', 'status': ['stableinterface'], 'supported_by': 'core'} - DOCUMENTATION = r''' --- module: win_ping @@ -38,11 +37,13 @@ description: options: data: description: - - Alternate data to return instead of 'pong' - default: 'pong' + - Alternate data to return instead of 'pong'. + - If this parameter is set to C(crash), the module will cause an exception. + default: pong notes: - For non-Windows targets, use the M(ping) module instead. -author: "Chris Church (@cchurch)" +author: +- Chris Church (@cchurch) ''' EXAMPLES = r''' @@ -52,7 +53,15 @@ EXAMPLES = r''' # Example from an Ansible Playbook - win_ping: -# Induce a crash to see what happens +# Induce an exception to see what happens - win_ping: data: crash ''' + +RETURN = ''' +ping: + description: value provided with the data parameter + returned: success + type: string + sample: pong +''' diff --git a/test/integration/targets/ping/tasks/main.yml b/test/integration/targets/ping/tasks/main.yml index d7cbe64aff4..7d4ee9562c4 100644 --- a/test/integration/targets/ping/tasks/main.yml +++ b/test/integration/targets/ping/tasks/main.yml @@ -23,15 +23,31 @@ - name: assert the ping worked assert: that: - - "result.changed == false" - - "result.ping == 'pong'" + - not result|failed + - not result|changed + - result.ping == 'pong' - name: ping with data - ping: data="testing" + ping: + data: testing register: result - name: assert the ping worked with data assert: that: - - "result.changed == false" - - "result.ping == 'testing'" + - not result|failed + - not result|changed + - result.ping == 'testing' + +- name: ping with data=crash + ping: + data: crash + register: result + ignore_errors: yes + +- name: assert the ping failed with data=boom + assert: + that: + - result|failed + - not result|changed + - "'Exception: boom' in result.module_stderr" diff --git a/test/integration/targets/win_ping/tasks/main.yml b/test/integration/targets/win_ping/tasks/main.yml index 1a3eb5fde05..a25a3d6b28b 100644 --- a/test/integration/targets/win_ping/tasks/main.yml +++ b/test/integration/targets/win_ping/tasks/main.yml @@ -23,20 +23,21 @@ - name: check win_ping result assert: that: - - "not win_ping_result|failed" - - "not win_ping_result|changed" - - "win_ping_result.ping == 'pong'" + - not win_ping_result|failed + - not win_ping_result|changed + - win_ping_result.ping == 'pong' - name: test win_ping with data - win_ping: data=☠ + win_ping: + data: ☠ register: win_ping_with_data_result - name: check win_ping result with data assert: that: - - "not win_ping_with_data_result|failed" - - "not win_ping_with_data_result|changed" - - "win_ping_with_data_result.ping == '☠'" + - not win_ping_with_data_result|failed + - not win_ping_with_data_result|changed + - win_ping_with_data_result.ping == '☠' - name: test win_ping.ps1 with data as complex args win_ping.ps1: @@ -46,9 +47,9 @@ - name: check win_ping.ps1 result with data assert: that: - - "not win_ping_ps1_result|failed" - - "not win_ping_ps1_result|changed" - - "win_ping_ps1_result.ping == 'bleep'" + - not win_ping_ps1_result|failed + - not win_ping_ps1_result|changed + - win_ping_ps1_result.ping == 'bleep' - name: test win_ping with extra args to verify that v2 module replacer escaping works as expected win_ping: @@ -76,9 +77,22 @@ - name: check that win_ping with extra args succeeds and ignores everything except data assert: that: - - "not win_ping_extra_args_result|failed" - - "not win_ping_extra_args_result|changed" - - "win_ping_extra_args_result.ping == 'bloop'" + - not win_ping_extra_args_result|failed + - not win_ping_extra_args_result|changed + - win_ping_extra_args_result.ping == 'bloop' + +- name: test win_ping using data=crash so that it throws an exception + win_ping: + data: crash + register: win_ping_crash_result + ignore_errors: yes + +- name: check win_ping_crash result + assert: + that: + - win_ping_crash_result|failed + - not win_ping_crash_result|changed + - "'FullyQualifiedErrorId : boom' in win_ping_crash_result.module_stderr" # TODO: fix code or tests? discrete error returns from PS are strange... @@ -90,11 +104,11 @@ #- name: check win_ping_throw result # assert: # that: -# - "win_ping_throw_result|failed" -# - "not win_ping_throw_result|changed" -# - "win_ping_throw_result.msg == 'MODULE FAILURE'" -# - "win_ping_throw_result.exception" -# - "win_ping_throw_result.error_record" +# - win_ping_throw_result|failed +# - not win_ping_throw_result|changed +# - win_ping_throw_result.msg == 'MODULE FAILURE' +# - win_ping_throw_result.exception +# - win_ping_throw_result.error_record # #- name: test modified win_ping that throws a string exception # action: win_ping_throw_string @@ -104,11 +118,11 @@ #- name: check win_ping_throw_string result # assert: # that: -# - "win_ping_throw_string_result|failed" -# - "not win_ping_throw_string_result|changed" -# - "win_ping_throw_string_result.msg == 'no ping for you'" -# - "win_ping_throw_string_result.exception" -# - "win_ping_throw_string_result.error_record" +# - win_ping_throw_string_result|failed +# - not win_ping_throw_string_result|changed +# - win_ping_throw_string_result.msg == 'no ping for you' +# - win_ping_throw_string_result.exception +# - win_ping_throw_string_result.error_record # #- name: test modified win_ping that has a syntax error # action: win_ping_syntax_error @@ -118,10 +132,10 @@ #- name: check win_ping_syntax_error result # assert: # that: -# - "win_ping_syntax_error_result|failed" -# - "not win_ping_syntax_error_result|changed" -# - "win_ping_syntax_error_result.msg" -# - "win_ping_syntax_error_result.exception" +# - win_ping_syntax_error_result|failed +# - not win_ping_syntax_error_result|changed +# - win_ping_syntax_error_result.msg +# - win_ping_syntax_error_result.exception # #- name: test modified win_ping that has an error that only surfaces when strict mode is on # action: win_ping_strict_mode_error @@ -131,10 +145,10 @@ #- name: check win_ping_strict_mode_error result # assert: # that: -# - "win_ping_strict_mode_error_result|failed" -# - "not win_ping_strict_mode_error_result|changed" -# - "win_ping_strict_mode_error_result.msg" -# - "win_ping_strict_mode_error_result.exception" +# - win_ping_strict_mode_error_result|failed +# - not win_ping_strict_mode_error_result|changed +# - win_ping_strict_mode_error_result.msg +# - win_ping_strict_mode_error_result.exception # #- name: test modified win_ping to verify a Set-Attr fix # action: win_ping_set_attr data="fixed" @@ -143,6 +157,6 @@ #- name: check win_ping_set_attr_result result # assert: # that: -# - "not win_ping_set_attr_result|failed" -# - "not win_ping_set_attr_result|changed" -# - "win_ping_set_attr_result.ping == 'fixed'" +# - not win_ping_set_attr_result|failed +# - not win_ping_set_attr_result|changed +# - win_ping_set_attr_result.ping == 'fixed'