Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[aws_s3] Fix uploading src option on the target machine to a bucket #39023

Merged
merged 2 commits into from
May 15, 2018
Merged

[aws_s3] Fix uploading src option on the target machine to a bucket #39023

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
66f5e3f
Fix backward compatibility for uploading src option on the target mac…
s-hertel Apr 19, 2018
dcfd6db
Allow the module to handle errors for nonexistent files
s-hertel May 2, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 12 additions & 6 deletions lib/ansible/plugins/action/aws_s3.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

import os

from ansible.errors import AnsibleError, AnsibleAction, AnsibleActionFail
from ansible.errors import AnsibleError, AnsibleAction, AnsibleActionFail, AnsibleFileNotFound
from ansible.module_utils._text import to_text
from ansible.plugins.action import ActionBase

Expand All @@ -43,11 +43,17 @@ def run(self, tmp=None, task_vars=None):
new_module_args = self._task.args.copy()
if source:
source = os.path.expanduser(source)
try:
source = self._loader.get_real_file(self._find_needle('files', source))
new_module_args['src'] = source
except AnsibleError as e:
raise AnsibleActionFail(to_text(e))

# For backward compatibility check if the file exists on the remote; it should take precedence
if not self._remote_file_exists(source):
try:
source = self._loader.get_real_file(self._find_needle('files', source))
new_module_args['src'] = source
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we also need to copy the file from controller to target? I've only ever tried this with controller == target.

Copy link
Contributor Author

@s-hertel s-hertel May 2, 2018 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey, @willthames sorry for taking a while to get back to this one. I wasn't sure exactly how it works and what the expected behavior is so I took some time to test it out and compare with older versions. It appears the controller file has never been copied so that would be a feature. Since I'd like this to be backported that could be a separate discussion maybe (though looks like it would add potential for more problems). I have tested with a target that is not the controller, and this fixes uploading a file that exists on the target.

I think I'm going to clarify the error if the file doesn't exist on the remote or the controller because it is a little confusing:

"msg": "Could not find or access '/tmp/onlyonremote.txt' on the Ansible Controller.\nIf you are using a module and expect the file to exist on the remote, see the remote_src option"

If the file exists on the controller but not the remote I get this message, which seems better:

"msg": "Local object for PUT does not exist"

except AnsibleFileNotFound as e:
# module handles error message for nonexistent files
new_module_args['src'] = source
except AnsibleError as e:
raise AnsibleActionFail(to_text(e))

# execute the aws_s3 module now, with the updated args
result.update(self._execute_module(module_args=new_module_args, task_vars=task_vars))
Expand Down