fixed lookup search path #16630

bcoca · 2016-07-07T16:12:35Z

ISSUE TYPE

Bugfix Pull Request

ANSIBLE VERSION

2.2

SUMMARY

added ansible_search_path var that contains the proper list and in order
removed roledir var which was only used by first_found, rest used role_path
added needle function for lookups that mirrors the action plugin one, now
both types of plugins use same pathing.

Also fixes first_found in particular, to be 'aware' of task subdir requirements, this info is now available to all with_<lookup>.

fixes #16630

alikins · 2016-07-07T16:58:52Z

lib/ansible/plugins/lookup/__init__.py

@@ -101,3 +102,19 @@ def run(self, terms, variables=None, **kwargs):
            result_string = to_unicode(result_string)
        """
        pass
+
+    def find_needle(self, myvars, subdir, needle):


Maybe 'def find_file_in_expected_search_path()' or 'find_file_in_search_path()' ?

And maybe a

def find_file_in_search_paths(self, myvars, subdirs, target_name): results = [] for subdir in subdirs: try: result.append(self.find_file_in_search_path(myvars, subdir, target_name)) # could use a more specific exception here. Or let find_needle/find_file_in_search_path() return results as [] except AnsibleError: continue return results

And/or, throw some variety of IGotTooManyAnswersException if find_file_in_search_paths end up with len(result) > 1

something like:

def find_file_in_search_paths(self, myvars, subdirs, target_name): results = [] for subdir in subdirs: try: result.append(self.find_file_in_search_path(myvars, subdir, target_name)) # could use a more specific exception here. Or let find_needle/find_file_in_search_path() return results as [] except AnsibleError: continue if not results: raise NoFilesFoundInSearchPathException(target_file=target_name, subdir=subdir) if len(results) > 1: raise TooManyFilesException(target_file=target_name, subdir=subdir, results=results) return results

it cannot, as it always returns the first one found

to rephrase, we don't care that multiple files can match, it MUST return the first one it finds, that is why search path order matters.

added ansible_search_path var that contains the proper list and in order removed roledir var which was only used by first_found, rest used role_path added needle function for lookups that mirrors the action plugin one, now both types of plugins use same pathing.

eb4x · 2016-07-14T11:12:54Z

lib/ansible/executor/task_executor.py

+                mylookup = self._shared_loader_obj.lookup_loader.get(self._task.loop, loader=self._loader, templar=templar)
+
+                # give lookup task 'context' for subdir (mostly needed for first_found)
+                for subdir in ['tempalte', 'var', 'file']: #TODO: move this to constants?


template is misspelled.

'with_first_found' lookup into {files,templates} directory so we must use relative path for vars/* (See See ansible/ansible#14454) $ ansible --version ansible 2.2.0

Nosmoht · 2016-07-19T07:50:23Z

Hi,

with this change no files in role/vars are looked up anymore. I've to rename the vars directory inside each role to files. Is this expected or a bug?

* fixed lookup search path added ansible_search_path var that contains the proper list and in order removed roledir var which was only used by first_found, rest used role_path added needle function for lookups that mirrors the action plugin one, now both types of plugins use same pathing. * added missing os import * renamed as per feedback * fixed missing rename in first_found * also fixed first_found * fixed import to match new error class * fixed getattr ref

PR ansible#16630 broke globbing in with_fileglob for globbed path components by changing from path_dwim_relative to path_dwim_relative_stack. While the former does something like this (for a path specification like "/tmp/*/some.conf", see Issue ansible#17136 for an example playbook) search = ["/tmp/*"] for candidate in search: if os.path.exists(candidate): break return candidate the check inside path_dwim_relative_stack works slightly differently: result = None if os.path.exists("/tmp/*"): result = "/tmp/*" return result The way the loop in the former example works makes path_dwim_relative return the last element from the 'search' list even if os.path.exists never returned True, while path_dwim_relative_stack returns None in the same case. This ultimately makes the fileglob plugin fail because find_file_in_search_path returns None now. To allow asterisks in paths again, this change breaks the search for potential targets out of path_dwim_relative_stack. This function can then be called from the fileglob plugin to find all potential target directories without checking if these dirs (like "/tmp/*") actually exist. fileglob will now apply globbing first and check for the actual existence of the files _after_ globbing.

PR ansible#16630 broke globbing in with_fileglob for globbed path components by changing from path_dwim_relative to path_dwim_relative_stack. While the former does something like this (for a path specification like "/tmp/*/some.conf", see Issue ansible#17136 for an example playbook) search = ["/tmp/*"] for candidate in search: if os.path.exists(candidate): break return candidate the check inside path_dwim_relative_stack works slightly differently: result = None if os.path.exists("/tmp/*"): result = "/tmp/*" return result The way the loop in the former example works makes path_dwim_relative return the last element from the 'search' list even if os.path.exists never returned True, while path_dwim_relative_stack returns None in the same case. This ultimately makes the fileglob plugin fail because find_file_in_search_path returns None now. To allow asterisks in paths again, this change breaks the search for potential targets out of path_dwim_relative_stack. This function can then be called from the fileglob plugin to find all potential target directories without checking if these dirs (like "/tmp/*") actually exist. fileglob will now apply globbing first and check for the actual existence of the files _after_ globbing. Additionally, fileglob now calls os.path.normpath() on the results of glob.glob and eliminates duplicate results.

bcoca added the bugfix_pull_request label Jul 7, 2016

bcoca added this to the 2.2.0 milestone Jul 7, 2016

bcoca force-pushed the lookup_path_fix branch from 2685703 to 0d9ea82 Compare July 7, 2016 16:42

alikins reviewed Jul 7, 2016
View reviewed changes

bcoca force-pushed the lookup_path_fix branch from d795806 to b7972da Compare July 8, 2016 16:55

bcoca added 5 commits July 12, 2016 10:08

added missing os import

2af55d8

renamed as per feedback

1cd4d80

fixed missing rename in first_found

646ba16

also fixed first_found

fff858c

bcoca force-pushed the lookup_path_fix branch from b7972da to fff858c Compare July 12, 2016 14:12

bcoca added 2 commits July 12, 2016 10:13

fixed import to match new error class

8d2f08f

fixed getattr ref

80139bd

bcoca merged commit 3c39bb5 into ansible:devel Jul 13, 2016

eb4x reviewed Jul 14, 2016
View reviewed changes

luto mentioned this pull request Nov 29, 2016

with_fileglob does not handle a * to match a directory #17136

Closed

rupran mentioned this pull request Dec 13, 2016

fileglob: make fileglob work with globbed path components again (Fixes: #17136) #19297

Closed

sivel mentioned this pull request Jan 23, 2017

Using with_fileglob and "delegate_to: localhost" causes ansible to make ssh connection to localhost #18549

Closed

rupran mentioned this pull request Apr 6, 2017

[WIP] Allow fileglob of dirs #23265

Closed

ansibot added bug This issue/PR relates to a bug. and removed bugfix_pull_request labels Mar 5, 2018

ansible locked and limited conversation to collaborators Apr 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fixed lookup search path #16630

fixed lookup search path #16630

bcoca commented Jul 7, 2016 •

edited

alikins Jul 7, 2016

alikins Jul 7, 2016

alikins Jul 7, 2016

bcoca Jul 7, 2016

bcoca Jul 7, 2016

eb4x Jul 14, 2016

Nosmoht commented Jul 19, 2016

fixed lookup search path #16630

fixed lookup search path #16630

Conversation

bcoca commented Jul 7, 2016 • edited

ISSUE TYPE

ANSIBLE VERSION

SUMMARY

alikins Jul 7, 2016

Choose a reason for hiding this comment

alikins Jul 7, 2016

Choose a reason for hiding this comment

alikins Jul 7, 2016

Choose a reason for hiding this comment

bcoca Jul 7, 2016

Choose a reason for hiding this comment

bcoca Jul 7, 2016

Choose a reason for hiding this comment

eb4x Jul 14, 2016

Choose a reason for hiding this comment

Nosmoht commented Jul 19, 2016

bcoca commented Jul 7, 2016 •

edited