-
Notifications
You must be signed in to change notification settings - Fork 44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Enhancement][1745][zos_script]Gracefully_fail_when_remote_src_does_not_exist #1894
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like if statement logic needs a revamp.
changelogs/fragments/1894-Gracefully_fail_when_remote_src_does_not_exist.yml
Outdated
Show resolved
Hide resolved
changelogs/fragments/1894-Gracefully_fail_when_remote_src_does_not_exist.yml
Outdated
Show resolved
Hide resolved
plugins/modules/zos_script.py
Outdated
@@ -361,6 +362,14 @@ def run_module(): | |||
msg='The given chdir {0} does not exist on the system.'.format(chdir) | |||
) | |||
|
|||
if remote_src and not os.path.exists(cmd_str): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not taking into account when the cmd_str
contains args. Modified the following test case
def test_rexx_script_with_args(ansible_zos_module):
hosts = ansible_zos_module
try:
rexx_script = REXX_SCRIPT_ARGS
script_path = create_local_file(rexx_script, 'rexx')
first_arg = 'one'
second_arg = 'two'
args = f'FIRST={first_arg} SECOND={second_arg}'
script_path = '/test.rexx'
cmd = f"{script_path} '{args}'"
hosts.all.shell(cmd="echo '{0}' '{1}' ".format(REXX_SCRIPT_ARGS, script_path))
zos_script_result = hosts.all.zos_script(
cmd=cmd,
remote_src=True,
)
for result in zos_script_result.contacted.values():
print(result)
assert result.get('changed') is True
assert result.get('failed', False) is False
assert result.get('rc') == 0
assert first_arg in result.get('stdout', '')
assert second_arg in result.get('stdout', '')
# Making sure the action plugin passed every argument to the module.
assert args in result.get('invocation').get('module_args').get('cmd')
assert args in result.get('remote_cmd')
assert result.get('stderr', '') == ''
finally:
if os.path.exists(script_path):
os.remove(script_path)
and fails with error message:
{'failed': True, 'changed': False, 'msg': "File /test.rexx 'FIRST=one SECOND=two' does not exists on the system, skipping script", 'invocation': {'module_args': {'chdir': None, 'cmd': "/test.rexx 'FIRST=one SECOND=two'", 'creates': None, 'executable': None, 'remote_src': True, 'removes': None, 'use_template': False, 'template_parameters': None}}, '_ansible_no_log': False}
This was not discovered in functional tests because there is no functional tests that test scripts with args when remote_src
is true.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch
@@ -480,3 +480,20 @@ def test_user_run_script_from_another_user(ansible_zos_module, z_python_interpre | |||
finally: | |||
hosts.all.file(path=script_path, state="absent") | |||
managed_user.delete_managed_user() | |||
|
|||
def test_remote_script_doest_not_exist(ansible_zos_module): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just above comment please test with args too.
…_not_exist.yml Co-authored-by: Fernando Flores <[email protected]>
…_not_exist.yml Co-authored-by: Fernando Flores <[email protected]>
plugins/modules/zos_script.py
Outdated
result = dict( | ||
changed=False, | ||
skipped=True, | ||
msg='File {0} does not exists on the system, skipping script'.format(script_path) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A tiny typo: exist
instead of exists
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! You even added a test case for testing a script with args and remote.
SUMMARY
Add verification to remote_src and file does not exist with error message
Fixes #1745
ISSUE TYPE
ADDITIONAL INFORMATION
Here is the new error message