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

with_first_found fails to find a file if an undefined variable is used in the list since ansible 2.9 #70772

Closed
afunix opened this issue Jul 20, 2020 · 16 comments · Fixed by #81178
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. easyfix This issue is considered easy to fix by aspiring contributors. has_pr This issue has an associated PR. P3 Priority 3 - Approved, No Time Limitation python3 support:core This issue/PR relates to code supported by the Ansible Engineering Team. verified This issue has been verified/reproduced by maintainer

Comments

@afunix
Copy link
Contributor

afunix commented Jul 20, 2020

SUMMARY

with_first_found fails "No file was found when using first_found. ..." even if a file matches search conditions, when an undefined variable is used in files parameter.
This is started after I've upgraded from ansible 2.8 to 2.9, so I suppose it is a bug.

ISSUE TYPE
  • Bug Report
COMPONENT NAME

first_found

ANSIBLE VERSION
ansible 2.9.10
  config file = None
  configured module search path = ['/Users/afunix/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /usr/local/Cellar/ansible/2.9.10/libexec/lib/python3.8/site-packages/ansible
  executable location = /usr/local/bin/ansible
  python version = 3.8.4 (default, Jul 14 2020, 02:58:48) [Clang 11.0.3 (clang-1103.0.32.62)]
CONFIGURATION
DEFAULT_VAULT_PASSWORD_FILE(env: ANSIBLE_VAULT_PASSWORD_FILE) = /Users/afunix/.vault
OS / ENVIRONMENT

Host: macOS 10.15.5
Target: CentOS 7.8

STEPS TO REPRODUCE

playbook.yml

---
- hosts: all
  roles:
      - test

roles/test/tasks/main.yml

---
- debug:
      msg: "path: {{role_path}}/vars"

- debug:
    msg: "file: {{ansible_distribution}}-{{ansible_distribution_major_version}}.yml"

- name: Load variables specific for OS family
  include_vars: "{{item}}"
  with_first_found:
    - files:
        - "{{ansible_lsb.id}}-{{ansible_lsb.major_release}}.yml"
        - "{{ansible_distribution}}-{{ansible_distribution_major_version}}.yml"
      paths:
        - "{{role_path}}/vars"

roles/test/vars/CentOS-7.yml

---
test: test
EXPECTED RESULTS

ansible-playbook is able to find CentOS-7.yml when running against CentOS 7.8 system, since the file exists.

ACTUAL RESULTS

ansible-playbook cannot find CentOS-7.yml when running against CentOS 7.8 system.

The issue goes away once I remove "{{ansible_lsb.id}}-{{ansible_lsb.major_release}}.yml" from files.

Checked ansible -m setup for the host and ansible_lsb is empty.

$ ansible-playbook -i inventory playbook.yml 

PLAY [all] *****************************************************************************************************************************************************************************************************

TASK [Gathering Facts] *****************************************************************************************************************************************************************************************
ok: [alice]

TASK [test : debug] ********************************************************************************************************************************************************************************************
ok: [alice] => {
    "msg": "path: /Users/afunix/tmp/ansible-with-first-found/roles/test/vars"
}

TASK [test : debug] ********************************************************************************************************************************************************************************************
ok: [alice] => {
    "msg": "file: CentOS-7.yml"
}

TASK [test : Load variables specific for OS family] ************************************************************************************************************************************************************
fatal: [alice]: FAILED! => {"msg": "No file was found when using first_found. Use errors='ignore' to allow this task to be skipped if no files are found"}

PLAY RECAP *****************************************************************************************************************************************************************************************************
alice                      : ok=3    changed=0    unreachable=0    failed=1    skipped=0    rescued=0    ignored=0   

$ stat /Users/afunix/tmp/ansible-with-first-found/roles/test/vars/CentOS-7.yml
16777220 38020305 -rw-r--r-- 1 afunix staff 0 15 "Jul 20 18:11:55 2020" "Jul 20 18:11:38 2020" "Jul 20 18:11:38 2020" "Jul 20 18:11:38 2020" 4096 8 0 /Users/afunix/tmp/ansible-with-first-found/roles/test/vars/CentOS-7.yml

$ ansible -i inventories/production/hosts -m setup alice | grep ansible_lsb
        "ansible_lsb": {},
@ansibot
Copy link
Contributor

ansibot commented Jul 20, 2020

Files identified in the description:
None

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

@ansibot ansibot added affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. needs_triage Needs a first human triage before being processed. python3 support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jul 20, 2020
@ansibot
Copy link
Contributor

ansibot commented Jul 21, 2020

Files identified in the description:

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

@bcoca bcoca added needs_verified This issue needs to be verified/reproduced by maintainer P3 Priority 3 - Approved, No Time Limitation and removed needs_triage Needs a first human triage before being processed. labels Jul 21, 2020
@sivel sivel added verified This issue has been verified/reproduced by maintainer and removed needs_verified This issue needs to be verified/reproduced by maintainer labels Jul 21, 2020
@sivel
Copy link
Member

sivel commented Jul 21, 2020

The issue here, is in filtering out untemplated terms in the search. We only account for strings in the terms in this code path.

The following diff should resolve the issue:

diff --git a/lib/ansible/executor/task_executor.py b/lib/ansible/executor/task_executor.py
index 5034bbc862..373f47e082 100644
--- a/lib/ansible/executor/task_executor.py
+++ b/lib/ansible/executor/task_executor.py
@@ -19,6 +19,7 @@ from ansible import constants as C
 from ansible.errors import AnsibleError, AnsibleParserError, AnsibleUndefinedVariable, AnsibleConnectionFailure, AnsibleActionFail, AnsibleActionSkip
 from ansible.executor.task_result import TaskResult
 from ansible.executor.module_common import get_action_args_with_defaults
+from ansible.module_utils.common._collections_compat import Mapping
 from ansible.module_utils.parsing.convert_bool import boolean
 from ansible.module_utils.six import iteritems, string_types, binary_type
 from ansible.module_utils.six.moves import xrange
@@ -223,8 +224,17 @@ class TaskExecutor:
 
                 loop_terms = listify_lookup_plugin_terms(terms=self._task.loop, templar=templar, loader=self._loader, fail_on_undefined=fail,
                                                          convert_bare=False)
+
                 if not fail:
-                    loop_terms = [t for t in loop_terms if not templar.is_template(t)]
+                    intermediate = []
+                    for term in loop_terms:
+                        if isinstance(term, Mapping) and 'files' in term:
+                            term['files'] = [t for t in term['files'] if not templar.is_template(t)]
+                            intermediate.append(term)
+                        else:
+                            if not templar.is_template(term):
+                                intermediate.append(term)
+                    loop_terms = intermediate
 
                 # get lookup
                 mylookup = self._shared_loader_obj.lookup_loader.get(self._task.loop_with, loader=self._loader, templar=templar)

If anyone is interested in testing this further, and creating tests to validate it, please feel free to do so.

@sivel sivel added the easyfix This issue is considered easy to fix by aspiring contributors. label Jul 21, 2020
@bmillemathias
Copy link
Contributor

Hey @afunix

Are you able to test the fix and provide some feedback ?

@bmillemathias
Copy link
Contributor

@afunix I eventually took the patch and did a PR.

@ansibot ansibot added the has_pr This issue has an associated PR. label Jul 23, 2020
@afunix
Copy link
Contributor Author

afunix commented Jul 23, 2020

I confirm the change fixes my issue.

@bcoca
Copy link
Member

bcoca commented May 11, 2022

needs_verified (first_found input was revamped and that might have fixed this issue)

@afunix
Copy link
Contributor Author

afunix commented May 19, 2022

I can still reproduce the issue on ansible 2.12.5 (MacOSX, installed with brew)

@zmingxie
Copy link

I'm also still hitting this issue and wondering if there is a fix for this

openstack-mirroring pushed a commit to openstack-archive/tripleo-ansible that referenced this issue Jul 29, 2022
We don't have any files in vars/ for this role. If the
'distribution' facts are not collected it would fail
with "No file was found when using first_found" due to
this open ansible bug[1].

[1] ansible/ansible#70772

Change-Id: I43f18466a064234cb62c5b6d7f9a2701dce9cb04
openstack-mirroring pushed a commit to openstack/openstack that referenced this issue Jul 29, 2022
* Update tripleo-ansible from branch 'master'
  to 4a32ee47f30a24e668868a90c176a79ed9b0ec1d
  - Merge "Drop gather variables task tripleo_keystone_resources"
  - Drop gather variables task tripleo_keystone_resources
    
    We don't have any files in vars/ for this role. If the
    'distribution' facts are not collected it would fail
    with "No file was found when using first_found" due to
    this open ansible bug[1].
    
    [1] ansible/ansible#70772
    
    Change-Id: I43f18466a064234cb62c5b6d7f9a2701dce9cb04
openstack-mirroring pushed a commit to openstack-archive/tripleo-ansible that referenced this issue Aug 24, 2022
We don't have any files in vars/ for this role. If the
'distribution' facts are not collected it would fail
with "No file was found when using first_found" due to
this open ansible bug[1].

[1] ansible/ansible#70772

Change-Id: I43f18466a064234cb62c5b6d7f9a2701dce9cb04
(cherry picked from commit e205948)
@ruoshan
Copy link

ruoshan commented Jul 5, 2023

This is my work-around, instead of sth like this:

  with_first_found:
    - files:
        - "{{ansible_lsb.id}}-{{ansible_lsb.major_release}}.yml"
        - "{{ansible_distribution}}-{{ansible_distribution_major_version}}.yml"
      paths:
        - "{{role_path}}/vars"

instead, you can use:

  with_first_found:
    - files:
        - "{{ansible_lsb.id}}-{{ansible_lsb.major_release}}.yml"
      paths:
        - "{{role_path}}/vars"
    - files:
        - "{{ansible_distribution}}-{{ansible_distribution_major_version}}.yml"
      paths:
        - "{{role_path}}/vars"

@bcoca
Copy link
Member

bcoca commented Jul 5, 2023

^ weird enough, this does work ... but that makes me thing the issue is how we look at how the terms get passed into the lookup itself vs the processing in the lookup

@bcoca
Copy link
Member

bcoca commented Jul 5, 2023

This line seems to be the problem:
https://github.com/ansible/ansible/blob/devel/lib/ansible/executor/task_executor.py#L234

If i remove it 'everything works', otherwise it removes the lines that require templating from being sent to the plugin, which then gets [] (empty list) of files to find ....

Going to try to figure out why that was added

@bcoca
Copy link
Member

bcoca commented Jul 5, 2023

The issue was that the 'undefined' var forced the whole item to be removed in task_executor, so the form that had everything in 'one place' ended up empty, the 2nd form @ruoshan showed, 'worked'TM because only the item with the undefined variable was removed. The code was iterating on the top level list of dicts, vs list of files in dict key named 'files', so it removed the full dict.

@ruoshan
Copy link

ruoshan commented Jul 10, 2023

@bcoca I was wrong, that work-around has other issue. if the files in both files section exist, the last one will be chose.

@bcoca
Copy link
Member

bcoca commented Jul 10, 2023

That is the nature of lists, each item is still processed, you would have to reverse order for that case to match the behavior you want.

This is not something that was ever documented/advertised as supported (list of dicts), not sure if i should make it an error, document the behavior or change it to also break on first found.

@bcoca
Copy link
Member

bcoca commented Jul 10, 2023

@roushan hmm, my tests seem to show it working as the last option above, its 'first found'

@ansible ansible locked and limited conversation to collaborators Jul 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.9 This issue/PR affects Ansible v2.9 bug This issue/PR relates to a bug. easyfix This issue is considered easy to fix by aspiring contributors. has_pr This issue has an associated PR. P3 Priority 3 - Approved, No Time Limitation python3 support:core This issue/PR relates to code supported by the Ansible Engineering Team. verified This issue has been verified/reproduced by maintainer
Projects
None yet
7 participants