Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #14454 with_first_found fails with ansible 2 #15543

Closed
wants to merge 2 commits into from

Conversation

twouters
Copy link
Contributor

@twouters twouters commented Apr 22, 2016

ISSUE TYPE
  • Bugfix Pull Request
ANSIBLE VERSION
"ansible 2.0.2.0"
SUMMARY

Two issues popped up while using with_first_found in combination with delegate_to.

The following works fine without delegate_to if file findme exists in <roledir>/templates/ but fails when delegate_to is used.

- name: debugging first_found plugin
  debug: msg="looking for files and found {{ item }}"
  with_first_found:
    - files:
      - "findme"
    - paths:
      - "templates"
  delegate_to: "{{ some_server }}"

The following always fails, regardless of delegate_to if file findme exists in <roledir>/templates/extra/.

- name: debugging first_found plugin
  debug: msg="looking for files and found {{ item }}"
  with_first_found:
    - files:
      - "findme"
    - paths:
      - "templates/extra"
delegate_to breakage

The variable roledir is unavailable when using delegate_to and file lookups will thus only happen in basedir (the directory where the playbook lives).

if roledir is not None:
# check the templates and vars directories too,if they exist
for subdir in ('templates', 'vars', 'files'):
path = self._loader.path_dwim_relative(roledir, subdir, fn)
if os.path.exists(path):
return [path]
# if none of the above were found, just check the
# current filename against the current dir
path = self._loader.path_dwim(fn)
if os.path.exists(path):
return [path]

                if roledir is not None:
                    # check the templates and vars directories too,if they exist
                    for subdir in ('templates', 'vars', 'files'):
                        path = self._loader.path_dwim_relative(roledir, subdir, fn)
                        if os.path.exists(path):
                            return [path]

                # if none of the above were found, just check the
                # current filename against the current dir
                path = self._loader.path_dwim(fn)
                if os.path.exists(path):
                    return [path]

suggested fix: It looks like the variable role_path is always available, so we can use that instead of roledir. See commit 5c86c19

paths not considered while looking for files

Provided paths are always ignored and searches will only happen in templates, vars and files directories under roledir.

There seem to be a few issues that cause total_search to be incomplete (i.e. not including the provided paths):

  • If pathlist is set during the first iteration of terms, it will be reset to an empty list during any following iterations. Same thing for filelist.
  • Appending the file path to total_search results in an infinite loop while iterating filelist, since we're really appending to filelist. So we should copy filelist using list() here. (The infinite loop never hit us because either filelist or pathlist has always been an empty list.)

suggested fix: See commit cd8c0b1

@twouters twouters force-pushed the fix-first_found branch 2 times, most recently from 8ffbc78 to f7c75df Compare April 22, 2016 11:23
@twouters twouters changed the title Fix first_found plugin Fix #14454 with_first_found fails with ansible 2 Apr 22, 2016
@twouters twouters force-pushed the fix-first_found branch 2 times, most recently from 7f97379 to fd7cd7a Compare April 22, 2016 12:03
@twouters
Copy link
Contributor Author

Updated summary, I hope it makes some sense now.

- If `pathlist` is set during the first iteration of `terms`, it will be reset
to an empty list during the following iterations.
- Appending the file path to `total_search` results in an infinite loop
while iterating `filelist`, since we're really appending to `filelist`.
So we should copy `filelist` using `list()` here.
The roledir variable isn't defined when using `delegate_to` but it looks
like role_path is available, so use this one instead.
@ansibot ansibot added affects_2.3 This issue/PR affects Ansible v2.3 bugfix_pullrequest labels Dec 13, 2016
@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Dec 15, 2016
@ansibot ansibot added needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. and removed needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. labels Jan 2, 2017
@ansibot
Copy link
Contributor

ansibot commented Jan 6, 2017

@twouters This PR was tested by travis-ci.org, which is no longer used. Please rebase your branch to trigger running of current tests.

click here for bot help

@ansibot ansibot added the stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. label Apr 11, 2017
@ansibot ansibot added the support:core This issue/PR relates to code supported by the Ansible Engineering Team. label Jun 29, 2017
@ansibot ansibot added the new_contributor This PR is the first contribution by a new community member. label Oct 18, 2017
@ansibot ansibot removed the new_contributor This PR is the first contribution by a new community member. label Nov 3, 2017
@rdxmb
Copy link

rdxmb commented Jan 2, 2018

any chances to get this merged?

@ansibot ansibot added the new_contributor This PR is the first contribution by a new community member. label Jan 26, 2018
@ansibot ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 2, 2018
@ansibot ansibot added support:community This issue/PR relates to code supported by the Ansible community. and removed support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Sep 18, 2018
@ansibot ansibot added support:core This issue/PR relates to code supported by the Ansible Engineering Team. and removed support:community This issue/PR relates to code supported by the Ansible community. labels Jun 18, 2019
@samdoran samdoran added the needs_verified This issue needs to be verified/reproduced by maintainer label Jun 21, 2019
@s-hertel
Copy link
Contributor

Thanks for the submission. 9abeecb appears to have fixed the delegate_to bug you found. I think the templates/extra example you made work is predicated on a typo. Here is the fixed example that works for me (note that files and paths are keys of the same dictionary instead):

  - debug: msg="looking for files and found {{ item }}"
    with_first_found:
      - files:
        - findme
        paths:
        - templates/extra

Feel free to stop by IRC or the mailing list if you have questions.

@s-hertel s-hertel closed this Jun 21, 2019
@ansible ansible locked and limited conversation to collaborators Aug 5, 2019
@sivel sivel removed the needs_verified This issue needs to be verified/reproduced by maintainer label Feb 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.3 This issue/PR affects Ansible v2.3 bug This issue/PR relates to a bug. c:plugins/lookup needs_rebase https://docs.ansible.com/ansible/devel/dev_guide/developing_rebasing.html needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. new_contributor This PR is the first contribution by a new community member. plugins/lookup stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants