Skip to content

Commit

Permalink
Fixes for facts distribution.py (ansible#31110)
Browse files Browse the repository at this point in the history
'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 ansible#30693 than ansible#30723

Fixes  ansible#30693
Related to ansible#30600
  • Loading branch information
alikins authored Oct 3, 2017
1 parent dae0ad1 commit 235d139
Showing 1 changed file with 12 additions and 23 deletions.
35 changes: 12 additions & 23 deletions lib/ansible/module_utils/facts/system/distribution.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,9 @@ def _parse_dist_file(self, name, dist_file_content, path, collected_facts):
# 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
Expand All @@ -126,15 +124,14 @@ def _parse_dist_file(self, name, dist_file_content, path, collected_facts):
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:
Expand Down Expand Up @@ -180,31 +177,29 @@ def process_dist_files(self):
# /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):
Expand Down Expand Up @@ -338,7 +333,7 @@ def parse_distribution_file_NA(self, name, data, path, collected_facts):
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':
Expand Down Expand Up @@ -532,11 +527,8 @@ def get_distribution_SMGL(self):
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:
Expand All @@ -550,8 +542,6 @@ def get_distribution_SunOS(self):
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'):
Expand All @@ -567,7 +557,6 @@ def get_distribution_SunOS(self):
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:
Expand Down

0 comments on commit 235d139

Please sign in to comment.