From 99171a9c6f6029716f93eee92a149b5afe622cf1 Mon Sep 17 00:00:00 2001 From: Matt Martz Date: Wed, 12 Sep 2018 09:28:54 -0500 Subject: [PATCH] [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+ --- .../fragments/get-url-fix-idempotency.yaml | 2 ++ .../modules/net_tools/basics/get_url.py | 26 ++++++++++--------- .../targets/get_url/tasks/main.yml | 13 +++++++++- 3 files changed, 28 insertions(+), 13 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 00000000000..c09c6d8acda --- /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 5ec01817d3a..b8afb83ee17 100644 --- a/lib/ansible/modules/net_tools/basics/get_url.py +++ b/lib/ansible/modules/net_tools/basics/get_url.py @@ -466,18 +466,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) - - checksum_mismatch = True + if checksum != destination_checksum: + 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 d67eea69843..0c3df2fbaf6 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/27617 - name: set role facts