From e7740b80fd30a456779f0a8cf86a67d672309e5c Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 12 Sep 2018 09:28:54 -0500 Subject: [PATCH] [stable-2.6] [stable-2.7] Fix logic to not re-download existing files when force=no (#45495) (#45509) * [stable-2.7] Fix logic to not re-download existing files when force=no (#45495) * Fix logic to not re-download existing files when force=no. Fixes #45491 * Reduce logic complexity. (cherry picked from commit 5785de582f99c13d02cb1dbb03af76cacdb5182d) Co-authored-by: Matt Martz * Backport of get_url fix cannot use result result was only added in 2.8+. (cherry picked from commit 99171a9c6f6029716f93eee92a149b5afe622cf1) Co-authored-by: Matt Martz --- .../fragments/get-url-fix-idempotency.yaml | 2 ++ .../modules/net_tools/basics/get_url.py | 24 ++++++++++--------- .../targets/get_url/tasks/main.yml | 13 +++++++++- 3 files changed, 27 insertions(+), 12 deletions(-) create mode 100644 changelogs/fragments/get-url-fix-idempotency.yaml diff --git a/changelogs/fragments/get-url-fix-idempotency.yaml b/changelogs/fragments/get-url-fix-idempotency.yaml new file mode 100644 index 0000000000..c09c6d8acd --- /dev/null +++ b/changelogs/fragments/get-url-fix-idempotency.yaml @@ -0,0 +1,2 @@ +bugfixes: +- get_url - Don't re-download files unnecessarily when force=no (https://github.com/ansible/ansible/issues/45491) diff --git a/lib/ansible/modules/net_tools/basics/get_url.py b/lib/ansible/modules/net_tools/basics/get_url.py index cbbf125f08..48b90f80cf 100644 --- a/lib/ansible/modules/net_tools/basics/get_url.py +++ b/lib/ansible/modules/net_tools/basics/get_url.py @@ -450,18 +450,20 @@ def main(): if not force and checksum != '': destination_checksum = module.digest_from_file(dest, algorithm) - if checksum == destination_checksum: - # Not forcing redownload, unless checksum does not match - # allow file attribute changes - module.params['path'] = dest - file_args = module.load_file_common_arguments(module.params) - file_args['path'] = dest - changed = module.set_fs_attributes_if_different(file_args, False) - if changed: - module.exit_json(msg="file already exists but file attributes changed", dest=dest, url=url, changed=changed) - module.exit_json(msg="file already exists", dest=dest, url=url, changed=changed) + if checksum != destination_checksum: + checksum_mismatch = True - checksum_mismatch = True + # Not forcing redownload, unless checksum does not match + if not force and not checksum_mismatch: + # Not forcing redownload, unless checksum does not match + # allow file attribute changes + module.params['path'] = dest + file_args = module.load_file_common_arguments(module.params) + file_args['path'] = dest + changed = module.set_fs_attributes_if_different(file_args, False) + if changed: + module.exit_json(msg="file already exists but file attributes changed", dest=dest, url=url, changed=changed) + module.exit_json(msg="file already exists", dest=dest, url=url, changed=changed) # If the file already exists, prepare the last modified time for the # request. diff --git a/test/integration/targets/get_url/tasks/main.yml b/test/integration/targets/get_url/tasks/main.yml index f3af012319..971153cb89 100644 --- a/test/integration/targets/get_url/tasks/main.yml +++ b/test/integration/targets/get_url/tasks/main.yml @@ -241,7 +241,7 @@ # https://github.com/ansible/ansible/issues/29614 - name: Change mode on an already downloaded file and specify checksum get_url: - url: 'https://{{ httpbin_host }}/' + url: 'https://{{ httpbin_host }}/get' dest: '{{ output_dir }}/test' checksum: 'sha256:7036ede810fad2b5d2e7547ec703cae8da61edbba43c23f9d7203a0239b765c4.' mode: '0775' @@ -257,6 +257,17 @@ - result is changed - "stat_result.stat.mode == '0775'" +- name: Get a file that already exists + get_url: + url: 'https://{{ httpbin_host }}/get' + dest: '{{ output_dir }}/test' + register: result + +- name: Assert that we didn't re-download unnecessarily + assert: + that: + - result is not changed + #https://github.com/ansible/ansible/issues/16191 - name: Test url split with no filename get_url: