Skip to content

Commit

Permalink
Fix regression in search path behaviour
Browse files Browse the repository at this point in the history 
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 ansible#19048
  • Loading branch information
@dagwieers @bcoca
dagwieers authored and bcoca committed Dec 14, 2016
1 parent 18b7852 commit 7c71c67
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 42 deletions.
15 changes: 9 additions & 6 deletions lib/ansible/parsing/dataloader.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,7 @@ def path_dwim_relative_stack(self, paths, dirname, source):
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')
Expand All @@ -315,18 +316,20 @@ def path_dwim_relative_stack(self, paths, dirname, source):
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)))
Expand Down
55 changes: 21 additions & 34 deletions test/integration/targets/lookup_paths/play.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions test/integration/targets/lookup_paths/testplay.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 7c71c67

Please sign in to comment.