From 235d139e5d1e386e434c663636bf73af06adf6e5 Mon Sep 17 00:00:00 2001 From: Adrian Likins Date: Tue, 3 Oct 2017 14:01:40 -0400 Subject: [PATCH] Fixes for facts distribution.py (#31110) 'distribution' facts were being set after checking the existence of the dist file, and then being set again with more detail after they were succesfully parsed. But if the dist file was not succesfully parsed and matched the required names, the loop continues without resetting the earlier set facts. This is how 'Mandriva' would end up being the 'distribution' file for unrelated cases (it would find /etc/lsb-release, set distro to 'Mandriva', then fail to parse/match and continue the loop. If no other checks worked, 'Mandriva' would stick). * parse_dist_file_NA should check 'name' not distro for NA parse_distribution_file_NA was checking the incoming 'distribution' fact to be 'NA', but the fact itself can be specific at that point ('KDE Neon', for ex) but the check is really if the 'name' it was passed is NA. * for matches on OS_RELEASE_ALIAS (ie, 'Archlinux') do not continue if the dist file content doesn't match. Previously it had to because of the 'Mandriva' bug mentioned above. This is a more general fix for #30693 than #30723 Fixes #30693 Related to #30600 --- .../module_utils/facts/system/distribution.py | 35 +++++++------------ 1 file changed, 12 insertions(+), 23 deletions(-) diff --git a/lib/ansible/module_utils/facts/system/distribution.py b/lib/ansible/module_utils/facts/system/distribution.py index c0b86014a7..3d75c57be2 100644 --- a/lib/ansible/module_utils/facts/system/distribution.py +++ b/lib/ansible/module_utils/facts/system/distribution.py @@ -113,11 +113,9 @@ class DistributionFiles: # only the distribution name is set, the version is assumed to be correct from platform.dist() if self.SEARCH_STRING[name] in dist_file_content: # this sets distribution=RedHat if 'Red Hat' shows up in data - # self.facts['distribution'] = name dist_file_dict['distribution'] = name else: # this sets distribution to what's in the data, e.g. CentOS, Scientific, ... - # self.facts['distribution'] = dist_file_content.split()[0] dist_file_dict['distribution'] = dist_file_content.split()[0] return True, dist_file_dict @@ -126,15 +124,14 @@ class DistributionFiles: if self.OS_RELEASE_ALIAS[name] in dist_file_content: dist_file_dict['distribution'] = name return True, dist_file_dict + return False, dist_file_dict # call a dedicated function for parsing the file content # TODO: replace with a map or a class try: # FIXME: most of these dont actually look at the dist file contents, but random other stuff distfunc_name = 'parse_distribution_file_' + name - # print('distfunc_name: %s' % distfunc_name) distfunc = getattr(self, distfunc_name) - # print('distfunc: %s' % distfunc) parsed, dist_file_dict = distfunc(name, dist_file_content, path, collected_facts) return parsed, dist_file_dict except AttributeError as exc: @@ -180,31 +177,29 @@ class DistributionFiles: # /etc/os-release with a different name if has_dist_file and allow_empty: dist_file_facts['distribution'] = name + dist_file_facts['distribution_file_path'] = path + dist_file_facts['distribution_file_variety'] = name break if not has_dist_file: # keep looking continue - # first valid os dist file we find we count - # FIXME: coreos and a few other bits expect this - # self.facts['distribution'] = name - dist_file_facts['distribution'] = name - dist_file_facts['distribution_file_variety'] = name - dist_file_facts['distribution_file_path'] = path - parsed_dist_file, parsed_dist_file_facts = self._parse_dist_file(name, dist_file_content, path, dist_file_facts) - dist_file_facts['distribution_file_parsed'] = parsed_dist_file - - # finally found the right os dit file and were able to parse it + # finally found the right os dist file and were able to parse it if parsed_dist_file: + dist_file_facts['distribution'] = name + dist_file_facts['distribution_file_path'] = path + # distribution and file_variety are the same here, but distribution + # will be changed/mapped to a more specific name. + # ie, dist=Fedora, file_variety=RedHat + dist_file_facts['distribution_file_variety'] = name + dist_file_facts['distribution_file_parsed'] = parsed_dist_file dist_file_facts.update(parsed_dist_file_facts) break return dist_file_facts -# distribution_facts.update(dist_file_facts) -# return distribution_facts # TODO: FIXME: split distro file parsing into its own module or class def parse_distribution_file_Slackware(self, name, data, path, collected_facts): @@ -338,7 +333,7 @@ class DistributionFiles: na_facts = {} for line in data.splitlines(): distribution = re.search("^NAME=(.*)", line) - if distribution and collected_facts['distribution'] == 'NA': + if distribution and name == 'NA': na_facts['distribution'] = distribution.group(1).strip('"') version = re.search("^VERSION=(.*)", line) if version and collected_facts['distribution_version'] == 'NA': @@ -532,11 +527,8 @@ class Distribution(object): def get_distribution_SunOS(self): sunos_facts = {} - # print('platform.release: %s' % distribution_release) data = get_file_content('/etc/release').splitlines()[0] - # print('get_file_content: data=%s' % data) - if 'Solaris' in data: ora_prefix = '' if 'Oracle Solaris' in data: @@ -550,8 +542,6 @@ class Distribution(object): uname_v = get_uname_version(self.module) distribution_version = None - # print('uname_v: %s' % uname_v) - if 'SmartOS' in data: sunos_facts['distribution'] = 'SmartOS' if _file_exists('/etc/product'): @@ -567,7 +557,6 @@ class Distribution(object): sunos_facts['distribution'] = 'Nexenta' distribution_version = data.split()[-1].lstrip('v') - # print('sunos_facts: %s' % sunos_facts) if sunos_facts.get('distribution', '') in ('SmartOS', 'OpenIndiana', 'OmniOS', 'Nexenta'): sunos_facts['distribution_release'] = data.strip() if distribution_version is not None: