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

merged 2 commits into from May 15, 2018

Conversation

s-hertel
Copy link
Contributor

SUMMARY

This re-enables uploading remote files (which worked prior to 4d58d16). Fixes #38656.
2.5 backport candidate.

ISSUE TYPE
  • Bugfix Pull Request
COMPONENT NAME

lib/ansible/plugins/action/aws_s3.py

ANSIBLE VERSION
2.6

@ansibot ansibot added bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Apr 19, 2018
willthames
willthames reviewed Apr 19, 2018
View reviewed changes
lib/ansible/plugins/action/aws_s3.py
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"

@mkrizek mkrizek removed the needs_triage Needs a first human triage before being processed. label Apr 20, 2018
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Apr 28, 2018
@s-hertel s-hertel force-pushed the fix_aws_s3_module_remote_src branch from d1f6ec0 to dcfd6db Compare May 2, 2018 19:08
@ansibot ansibot added needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels May 2, 2018
@s-hertel
Copy link
Contributor Author

s-hertel commented May 3, 2018

bot_status

@ansibot
Copy link
Contributor

ansibot commented May 3, 2018

Components

lib/ansible/plugins/action/aws_s3.py
support: core
maintainers:

Metadata

waiting_on: ansible
changes_requested_by: null
needs_info: False
needs_revision: False
needs_rebase: False
merge_commits: []
mergeable_state: clean
shippable_status: success
maintainer_shipits (module maintainers): False
community_shipits (namespace maintainers): False
ansible_shipits (core team members): False
shipit_actors (maintainer or core team member): None
shipit_actors_other:
automerge: automerge shipit test failed

click here for bot help

@ansibot ansibot removed the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label May 3, 2018
@ryansb ryansb mentioned this pull request May 4, 2018
Closed
@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label May 12, 2018
@ryansb ryansb merged commit b8a93c1 into ansible:devel May 15, 2018
s-hertel added a commit to s-hertel/ansible that referenced this pull request May 15, 2018
@s-hertel
[aws_s3] Fix uploading src option on the target machine to a bucket (a…
df1de63 
…nsible#39023)

* Fix backward compatibility for uploading src option on the target machine to a bucket

* Allow the module to handle errors for nonexistent files

(cherry picked from commit b8a93c1)
This was referenced May 15, 2018
Merged
Merged
s-hertel added a commit that referenced this pull request May 15, 2018
@s-hertel
aws_s3: fix uploading from the target - backport #39023 (#40192)
fe56d68 
* [aws_s3] Fix uploading src option on the target machine to a bucket (#39023)

* Fix backward compatibility for uploading src option on the target machine to a bucket

* Allow the module to handle errors for nonexistent files

(cherry picked from commit b8a93c1)

* changelog
jacum pushed a commit to jacum/ansible that referenced this pull request Jun 26, 2018
@s-hertel
[aws_s3] Fix uploading src option on the target machine to a bucket (a…
8127e77 
…nsible#39023)

* Fix backward compatibility for uploading src option on the target machine to a bucket

* Allow the module to handle errors for nonexistent files
ilicmilan pushed a commit to ilicmilan/ansible that referenced this pull request Nov 7, 2018
@s-hertel @ilicmilan
[aws_s3] Fix uploading src option on the target machine to a bucket (a…
641f397 
…nsible#39023)

* Fix backward compatibility for uploading src option on the target machine to a bucket

* Allow the module to handle errors for nonexistent files
@ansible ansible locked and limited conversation to collaborators May 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@willthames willthames willthames approved these changes

@ryansb ryansb ryansb approved these changes

Assignees
No one assigned
Labels
bug This issue/PR relates to a bug. stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

aws_s3: "src" always looking locally for files.
5 participants
@s-hertel @ansibot @willthames @ryansb @mkrizek