From 78ced5318f25593bf0163fb85a5f8f81cdacfcb0 Mon Sep 17 00:00:00 2001 From: Toshio Kuratomi Date: Wed, 5 Apr 2017 17:26:11 -0700 Subject: [PATCH] Fix for recursive copy slowness Copy module was walking over files in subdirectories repeatedly (a directory tree a few levels deep could bring the time spent into the tens of minutes) This was traced to the fix for this bug report: https://github.com/ansible/ansible/issues/13013 Fixed #13013 a different way and added an integration test to check for regressions of #13013 as we optimize this code. Fixes #21513 --- lib/ansible/plugins/action/copy.py | 22 +--------- test/integration/targets/copy/tasks/main.yml | 45 ++++++++++++++++++++ 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/lib/ansible/plugins/action/copy.py b/lib/ansible/plugins/action/copy.py index de9d512f46..3745933b08 100644 --- a/lib/ansible/plugins/action/copy.py +++ b/lib/ansible/plugins/action/copy.py @@ -113,7 +113,7 @@ class ActionModule(ActionBase): sz = len(source.rsplit('/', 1)[0]) + 1 # Walk the directory and append the file tuples to source_files. - for base_path, sub_folders, files in os.walk(to_bytes(source)): + for base_path, sub_folders, files in os.walk(to_bytes(source), followlinks=True): for file in files: full_path = to_text(os.path.join(base_path, file), errors='surrogate_or_strict') rel_path = full_path[sz:] @@ -121,10 +121,6 @@ class ActionModule(ActionBase): rel_path = rel_path[1:] source_files.append((full_path, rel_path)) - # recurse into subdirs - for sf in sub_folders: - source_files += self._get_recursive_files(os.path.join(source, to_text(sf)), sz=sz) - # If it's recursive copy, destination is always a dir, # explicitly mark it so (note - copy module relies on this). if not self._connection._shell.path_has_trailing_slash(dest): @@ -326,22 +322,6 @@ class ActionModule(ActionBase): return result - def _get_recursive_files(self, topdir, sz=0): - ''' Recursively create file tuples for sub folders ''' - r_files = [] - for base_path, sub_folders, files in os.walk(to_bytes(topdir)): - for fname in files: - full_path = to_text(os.path.join(base_path, fname), errors='surrogate_or_strict') - rel_path = full_path[sz:] - if rel_path.startswith('/'): - rel_path = rel_path[1:] - r_files.append((full_path, rel_path)) - - for sf in sub_folders: - r_files += self._get_recursive_files(os.path.join(topdir, to_text(sf)), sz=sz) - - return r_files - def _create_content_tempfile(self, content): ''' Create a tempfile containing defined content ''' fd, content_tempfile = tempfile.mkstemp() diff --git a/test/integration/targets/copy/tasks/main.yml b/test/integration/targets/copy/tasks/main.yml index 128c0a793e..7e799e2d95 100644 --- a/test/integration/targets/copy/tasks/main.yml +++ b/test/integration/targets/copy/tasks/main.yml @@ -258,3 +258,48 @@ assert: that: - replace_follow_result.checksum == target_file_result.stdout + +- name: create a test dir to copy + file: + path: '{{ output_dir }}/top_dir' + state: directory + +- name: create a test dir to symlink to + file: + path: '{{ output_dir }}/linked_dir' + state: directory + +- name: create a file in the test dir + copy: + dest: '{{ output_dir }}/linked_dir/file1' + content: 'hello world' + +- name: create a link to the test dir + file: + path: '{{ output_dir }}/top_dir/follow_link_dir' + src: '{{ output_dir }}/linked_dir' + state: link + +- name: copy the directory's link + copy: + src: '{{ output_dir }}/top_dir' + dest: '{{ output_dir }}/new_dir' + follow: True + +- name: stat the copied path + stat: + path: '{{ output_dir }}/new_dir/top_dir/follow_link_dir' + register: stat_dir_result + +- name: stat the copied path + stat: + path: '{{ output_dir }}/new_dir/top_dir/follow_link_dir/file1' + register: stat_file_in_dir_result + +- name: assert that the directory exists + assert: + that: + - stat_dir_result.stat.exists + - stat_dir_result.stat.isdir + - stat_file_in_dir_result.stat.exists + - stat_file_in_dir_result.stat.isreg