From 268645c17a98fd65688b956e888ce8a9559488d1 Mon Sep 17 00:00:00 2001 From: Dag Wieers Date: Fri, 9 Dec 2016 10:18:15 +0100 Subject: [PATCH] Fix regression in search path behaviour This PR fixes a few issues: - Missing role parent directory for relative paths - Fix integration tests (add missing stage) - Redesign integration tests - Incorrect order with tasks-lookups - Duplicate paths are listed - Repetitive tasks/tasks or files/files were possible ==== using copy with test.txt Before: ``` 491 1481281038.29393: search_path: /home/dag/home-made/ansible.testing/roles/test134/files/test.txt /home/dag/home-made/ansible.testing/roles/test134/tasks/test.txt /home/dag/home-made/ansible.testing/roles/test134/tasks/files/test.txt /home/dag/home-made/ansible.testing/roles/test134/tasks/tasks/test.txt /home/dag/home-made/ansible.testing/files/test.txt /home/dag/home-made/ansible.testing/test.txt ``` After: ``` 32505 1481280963.22418: search_path: /home/dag/home-made/ansible.testing/roles/test134/files/test.txt /home/dag/home-made/ansible.testing/roles/test134/test.txt /home/dag/home-made/ansible.testing/roles/test134/tasks/files/test.txt /home/dag/home-made/ansible.testing/roles/test134/tasks/test.txt /home/dag/home-made/ansible.testing/files/test.txt /home/dag/home-made/ansible.testing/test.txt ``` ==== Using copy with files/test.txt Before: ``` 31523 1481280499.63052: search_path: /home/dag/home-made/ansible.testing/roles/test134/files/test.txt /home/dag/home-made/ansible.testing/roles/test134/tasks/files/test.txt /home/dag/home-made/ansible.testing/roles/test134/tasks/files/test.txt /home/dag/home-made/ansible.testing/roles/test134/tasks/tasks/files/test.txt /home/dag/home-made/ansible.testing/files/files/test.txt /home/dag/home-made/ansible.testing/files/test.txt ``` After: ``` 31110 1481280299.38778: search_path: /home/dag/home-made/ansible.testing/roles/test134/files/test.txt /home/dag/home-made/ansible.testing/roles/test134/tasks/files/test.txt /home/dag/home-made/ansible.testing/files/test.txt ``` ==== Using template with files/test.txt.j2 Before: ``` 30074 1481280064.15191: search_path: /home/dag/home-made/ansible.testing/roles/test134/templates/files/test.txt.j2 /home/dag/home-made/ansible.testing/roles/test134/tasks/files/test.txt.j2 /home/dag/home-made/ansible.testing/roles/test134/tasks/templates/files/test.txt.j2 /home/dag/home-made/ansible.testing/roles/test134/tasks/tasks/files/test.txt.j2 /home/dag/home-made/ansible.testing/templates/files/test.txt.j2 /home/dag/home-made/ansible.testing/files/test.txt.j2 ``` After: ``` 29201 1481279823.52752: search_path: /home/dag/home-made/ansible.testing/roles/test134/templates/files/test.txt.j2 /home/dag/home-made/ansible.testing/roles/test134/files/test.txt.j2 /home/dag/home-made/ansible.testing/roles/test134/tasks/templates/files/test.txt.j2 /home/dag/home-made/ansible.testing/roles/test134/tasks/files/test.txt.j2 /home/dag/home-made/ansible.testing/templates/files/test.txt.j2 /home/dag/home-made/ansible.testing/files/test.txt.j2 ``` This fixes #19048 (cherry picked from commit 7c71c678fae64f6962604b76664dd320128130d1) --- lib/ansible/parsing/dataloader.py | 15 +++--- test/integration/lookup_paths/play.yml | 55 +++++++++------------- test/integration/lookup_paths/testplay.yml | 7 ++- 3 files changed, 35 insertions(+), 42 deletions(-) diff --git a/lib/ansible/parsing/dataloader.py b/lib/ansible/parsing/dataloader.py index 594475bb20b..6bac8e72378 100644 --- a/lib/ansible/parsing/dataloader.py +++ b/lib/ansible/parsing/dataloader.py @@ -301,6 +301,7 @@ class DataLoader(): result = test_path else: search = [] + display.debug(u'evaluation_path:\n\t%s' % '\n\t'.join(paths)) for path in paths: upath = unfrackpath(path) b_upath = to_bytes(upath, errors='surrogate_or_strict') @@ -315,18 +316,20 @@ class DataLoader(): search.append(os.path.join(b_mydir, b_source)) else: # don't add dirname if user already is using it in source - if b_source.split(b'/')[0] == b_dirname: - search.append(os.path.join(b_upath, b_source)) - else: + if b_source.split(b'/')[0] != b_dirname: search.append(os.path.join(b_upath, b_dirname, b_source)) - search.append(os.path.join(b_upath, b'tasks', b_source)) + search.append(os.path.join(b_upath, b_source)) + elif b_dirname not in b_source.split(b'/'): # don't add dirname if user already is using it in source - search.append(os.path.join(b_upath, b_dirname, b_source)) + if b_source.split(b'/')[0] != dirname: + search.append(os.path.join(b_upath, b_dirname, b_source)) search.append(os.path.join(b_upath, b_source)) # always append basedir as last resort - search.append(os.path.join(to_bytes(self.get_basedir()), b_dirname, b_source)) + # don't add dirname if user already is using it in source + if b_source.split(b'/')[0] != dirname: + search.append(os.path.join(to_bytes(self.get_basedir()), b_dirname, b_source)) search.append(os.path.join(to_bytes(self.get_basedir()), b_source)) display.debug(u'search_path:\n\t%s' % to_text(b'\n\t'.join(search))) diff --git a/test/integration/lookup_paths/play.yml b/test/integration/lookup_paths/play.yml index 26a9ce10b9c..bded090086f 100644 --- a/test/integration/lookup_paths/play.yml +++ b/test/integration/lookup_paths/play.yml @@ -6,53 +6,40 @@ - file: path={{playbook_dir}}/files state=directory - file: path={{playbook_dir}}/roles/showfile/files state=directory - copy: dest={{playbook_dir}}/roles/showfile/files/testfile content='in role files' - - copy: dest={{playbook_dir}}/roles/showfile/tasks/testfile content='in role tasks' - copy: dest={{playbook_dir}}/roles/showfile/testfile content='in role' + - copy: dest={{playbook_dir}}/roles/showfile/tasks/testfile content='in role tasks' - copy: dest={{playbook_dir}}/files/testfile content='in files' - copy: dest={{playbook_dir}}/testfile content='in local' - - set_fact: role_out="in role files" play_out="in files" stage='setup' - include: testplay.yml - -- name: remove from role/files - hosts: testhost - connection: local - gather_facts: false - tasks: - - file: path={{playbook_dir}}/roles/showfile/files/testfile state=absent - - set_fact: role_out="in role tasks" play_out="in files" stage='remove 1' + vars: + remove: nothing + role_out: in role files + play_out: in files - include: testplay.yml - -- name: remove from role/tasks - hosts: testhost - connection: local - gather_facts: false - tasks: - - file: path={{playbook_dir}}/roles/showfile/tasks/testfile state=absent - - set_fact: role_out="in files" play_out="in files" stage='remote 2' + vars: + remove: roles/showfile/files/testfile + role_out: in role + play_out: in files - include: testplay.yml - -- name: remove from role dir - hosts: testhost - connection: local - gather_facts: false - tasks: - - file: path={{playbook_dir}}/roles/showfile/testfile state=absent - - set_fact: role_out="in files" play_out="in files" stage='remove 3' + vars: + remove: roles/showfile/testfile + role_out: in role tasks + play_out: in files - include: testplay.yml - -- name: remove from play/files - hosts: testhost - connection: local - gather_facts: false - tasks: - - file: path={{playbook_dir}}/files/testfile state=absent - - set_fact: role_out="in local" play_out="in local" stage='remove 4' + vars: + remove: roles/showfile/tasks/testfile + role_out: in files + play_out: in files - include: testplay.yml + vars: + remove: files/testfile + role_out: in local + play_out: in local - name: cleanup hosts: testhost diff --git a/test/integration/lookup_paths/testplay.yml b/test/integration/lookup_paths/testplay.yml index 8bf4be2c08b..a2055158ee1 100644 --- a/test/integration/lookup_paths/testplay.yml +++ b/test/integration/lookup_paths/testplay.yml @@ -2,13 +2,16 @@ hosts: testhost connection: local gather_facts: false + pre_tasks: + - name: remove {{ remove }} + file: path={{ playbook_dir }}/{{ remove }} state=absent roles: - showfile - tasks: + post_tasks: - name: from play set_fact: play_result="{{lookup('file', 'testfile')}}" - - name: output output {{stage}} + - name: output stage {{ remove }} removed debug: msg="play> {{play_out}}, role> {{role_out}}" - name: verify that result match expected