Port from plaform.dist to ansible.module_utils.distro.linux_distribution

ci_complete

changelogs/fragments/platform-dist-to-nir0s-distro.yaml

@@ -0,0 +1,5 @@
+---
+minor_changes:
+- Python-3.8 removes platform.dist() from the standard library. To maintain
+  compatibility we've switched to an alternative library, nir0s/distro to
+  detect the distribution for fact gathering.

hacking/tests/gen_distribution_version_testcase.py

@@ -3,17 +3,21 @@
 """
 This script generated test_cases for test_distribution_version.py.
 
-To do so it outputs the relevant files from /etc/*release, the output of platform.dist() and the current ansible_facts regarding the distribution version.
+To do so it outputs the relevant files from /etc/*release, the output of distro.linux_distribution()
+and the current ansible_facts regarding the distribution version.
 
 This assumes a working ansible version in the path.
 """
 
-import platform
+
 import os.path
 import subprocess
 import json
 import sys
 
+from ansible.module_utils import distro
+
+
 filelist = [
     '/etc/oracle-release',
     '/etc/slackware-version',
@@ -44,8 +48,7 @@ for f in filelist:
             with open(f) as fh:
                 fcont[f] = fh.read()
 
-dist = platform.dist()
-
+dist = distro.linux_distribution(full_distribution_name=False)
 
 facts = ['distribution', 'distribution_version', 'distribution_release', 'distribution_major_version', 'os_family']
 

lib/ansible/module_utils/common/sys_info.py

@@ -1,7 +1,17 @@
+# Copyright (c), Michael DeHaan <michael.dehaan@gmail.com>, 2012-2013
+# Copyright (c), Toshio Kuratomi <tkuratomi@ansible.com> 2016
+# Simplified BSD License (see licenses/simplified_bsd.txt or https://opensource.org/licenses/BSD-2-Clause)
+
+from __future__ import absolute_import, division, print_function
+__metaclass__ = type
+
 import os
 import platform
 
+from ansible.module_utils import distro
 
+
+# Backwards compat.  New code should just use platform.system()
 def get_platform():
     '''
     :rtype: NativeString
@@ -16,39 +26,60 @@ def get_distribution():
     :returns: Name of the distribution the module is running on
     '''
     distribution = None
-    additional_linux = ('alpine', 'arch', 'devuan')
-    supported_dists = platform._supported_dists + additional_linux
 
     if platform.system() == 'Linux':
-        try:
-            distribution = platform.linux_distribution(supported_dists=supported_dists)[0].capitalize()
-            if not distribution and os.path.isfile('/etc/system-release'):
-                distribution = platform.linux_distribution(supported_dists=['system'])[0].capitalize()
-                if 'Amazon' in distribution:
+        distribution = distro.name().capitalize()
+
+        # FIXME: Would we need to normalize these if we used: id() instead of name()?
+        distribution_words = distribution.split()
+        if 'Amazon' in distribution_words:
             distribution = 'Amazon'
-                else:
+        elif not distribution:
             distribution = 'OtherLinux'
-        except Exception:
-            # FIXME: MethodMissing, I assume?
-            distribution = platform.dist()[0].capitalize()
+
     return distribution
 
 
 def get_distribution_version():
     '''
     :rtype: NativeString or None
-    :returns: A string representation of the version of the distribution
+    :returns: A string representation of the version of the distribution.  None if this is not run
+        on a Linux machine
+    '''
+    version = None
+    if platform.system() == 'Linux':
+        version = distro.version()
+    return version
+
+
+def get_distribution_codename():
+    '''
+    Return the code name for this Linux Distribution
+
+    :rtype: NativeString or None
+    :returns: A string representation of the distribution's codename or None if not a Linux distro
     '''
-    distribution_version = None
+    codename = None
     if platform.system() == 'Linux':
-        try:
-            distribution_version = platform.linux_distribution()[1]
-            if not distribution_version and os.path.isfile('/etc/system-release'):
-                distribution_version = platform.linux_distribution(supported_dists=['system'])[1]
-        except Exception:
-            # FIXME: MethodMissing, I assume?
-            distribution_version = platform.dist()[1]
-    return distribution_version
+        # Until this gets merged and we update our bundled copy of distro:
+        # https://github.com/nir0s/distro/pull/230
+        # Fixes Fedora 28+ not having a code name and Ubuntu Xenial Xerus needing to be "xenial"
+        os_release_info = distro.os_release_info()
+        codename = os_release_info.get('version_codename')
+
+        if codename is None:
+            codename = os_release_info.get('ubuntu_codename')
+
+        if codename is None and distro.id() == 'ubuntu':
+            lsb_release_info = distro.lsb_release_info()
+            codename = lsb_release_info.get('codename')
+
+        if codename is None:
+            codename = distro.codename()
+            if codename == u'':
+                codename = None
+
+    return codename
 
 
 def get_all_subclasses(cls):

lib/ansible/module_utils/facts/system/distribution.py

@@ -20,8 +20,9 @@ import os
 import platform
 import re
 
+from ansible.module_utils.common.sys_info import get_distribution, get_distribution_version, \
+    get_distribution_codename
 from ansible.module_utils.facts.utils import get_file_content
-
 from ansible.module_utils.facts.collector import BaseFactCollector
 
 
@@ -111,7 +112,7 @@ class DistributionFiles:
         dist_file_dict = {}
         if name in self.SEARCH_STRING:
             # look for the distribution string in the data and replace according to RELEASE_NAME_MAP
-            # only the distribution name is set, the version is assumed to be correct from platform.dist()
+            # only the distribution name is set, the version is assumed to be correct from distro.linux_distribution()
             if self.SEARCH_STRING[name] in dist_file_content:
                 # this sets distribution=RedHat if 'Red Hat' shows up in data
                 dist_file_dict['distribution'] = name
@@ -152,12 +153,15 @@ class DistributionFiles:
 
     def _guess_distribution(self):
         # try to find out which linux distribution this is
-        dist = platform.dist()
+        dist = (get_distribution(), get_distribution_version(), get_distribution_codename())
         distribution_guess = {}
-        distribution_guess['distribution'] = dist[0].capitalize() or 'NA'
+        distribution_guess['distribution'] = dist[0] or 'NA'
         distribution_guess['distribution_version'] = dist[1] or 'NA'
-        distribution_guess['distribution_major_version'] = dist[1].split('.')[0] or 'NA'
-        distribution_guess['distribution_release'] = dist[2] or 'NA'
+        distribution_guess['distribution_major_version'] = \
+            distribution_guess['distribution_version'].split('.')[0] or 'NA'
+        # distribution_release can be the empty string
+        distribution_guess['distribution_release'] = 'NA' if dist[2] is None else dist[2]
+
         return distribution_guess
 
     def process_dist_files(self):
@@ -362,8 +366,7 @@ class DistributionFiles:
     def parse_distribution_file_Coreos(self, name, data, path, collected_facts):
         coreos_facts = {}
         # FIXME: pass in ro copy of facts for this kind of thing
-        dist = platform.dist()
-        distro = dist[0]
+        distro = get_distribution()
 
         if distro.lower() == 'coreos':
             if not data:

lib/ansible/modules/cloud/misc/xenserver_facts.py

@@ -46,7 +46,6 @@ EXAMPLES = '''
 # }
 '''
 
-import platform
 
 HAVE_XENAPI = False
 try:
@@ -55,6 +54,7 @@ try:
 except ImportError:
     pass
 
+from ansible.module_utils import distro
 from ansible.module_utils.basic import AnsibleModule
 
 
@@ -70,8 +70,7 @@ class XenServerFacts:
 
     @property
     def version(self):
-        # Be aware! Deprecated in Python 2.6!
-        result = platform.dist()[1]
+        result = distro.linux_distribution()[1]
         return result
 
     @property

lib/ansible/modules/system/user.py

@@ -379,7 +379,6 @@ import errno
 import grp
 import os
 import re
-import platform
 import pty
 import pwd
 import select
@@ -388,6 +387,7 @@ import socket
 import subprocess
 import time
 
+from ansible.module_utils import distro
 from ansible.module_utils._text import to_native, to_bytes, to_text
 from ansible.module_utils.basic import load_platform_subclass, AnsibleModule
 
@@ -569,7 +569,7 @@ class User(object):
             # errors from useradd trying to create a group when
             # USERGROUPS_ENAB is set in /etc/login.defs.
             if os.path.exists('/etc/redhat-release'):
-                dist = platform.dist()
+                dist = distro.linux_distribution(full_distribution_name=False)
                 major_release = int(dist[1].split('.')[0])
                 if major_release <= 5:
                     cmd.append('-n')
@@ -578,7 +578,7 @@ class User(object):
             elif os.path.exists('/etc/SuSE-release'):
                 # -N did not exist in useradd before SLE 11 and did not
                 # automatically create a group
-                dist = platform.dist()
+                dist = distro.linux_distribution(full_distribution_name=False)
                 major_release = int(dist[1].split('.')[0])
                 if major_release >= 12:
                     cmd.append('-N')

test/units/executor/module_common/test_recursive_finder.py

@@ -47,6 +47,8 @@ MODULE_UTILS_BASIC_IMPORTS = frozenset((('_text',),
                                         ('common', 'file'),
                                         ('common', 'process'),
                                         ('common', 'sys_info'),
+                                        ('distro', '__init__'),
+                                        ('distro', '_distro'),
                                         ('parsing', '__init__'),
                                         ('parsing', 'convert_bool'),
                                         ('pycompat24',),
@@ -63,6 +65,8 @@ MODULE_UTILS_BASIC_FILES = frozenset(('ansible/module_utils/parsing/__init__.py'
                                       'ansible/module_utils/common/__init__.py',
                                       'ansible/module_utils/common/file.py',
                                       'ansible/module_utils/common/sys_info.py',
+                                      'ansible/module_utils/distro/__init__.py',
+                                      'ansible/module_utils/distro/_distro.py',
                                       'ansible/module_utils/pycompat24.py',
                                       ))
 

test/units/module_utils/basic/test_platform_distribution.py

@@ -1,84 +1,96 @@
 # -*- coding: utf-8 -*-
 # (c) 2012-2014, Michael DeHaan <michael.dehaan@gmail.com>
 # (c) 2016 Toshio Kuratomi <tkuratomi@ansible.com>
-# (c) 2017 Ansible Project
+# (c) 2017-2018 Ansible Project
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 from __future__ import absolute_import, division, print_function
 __metaclass__ = type
 
+import pytest
+
 from units.mock.procenv import ModuleTestCase
 
 from units.compat.mock import patch
+
 from ansible.module_utils.six.moves import builtins
 
+# Functions being tested
+from ansible.module_utils.common.sys_info import get_all_subclasses
+from ansible.module_utils.common.sys_info import get_distribution
+from ansible.module_utils.common.sys_info import get_distribution_version
+from ansible.module_utils.common.sys_info import get_platform
+from ansible.module_utils.common.sys_info import load_platform_subclass
+
+
 realimport = builtins.__import__
 
 
-class TestPlatform(ModuleTestCase):
-    def test_module_utils_basic_get_platform(self):
+@pytest.fixture
+def platform_linux(mocker):
+    mocker.patch('platform.system', return_value='Linux')
+
+
+#
+# get_platform tests
+#
+
+def test_get_platform():
     with patch('platform.system', return_value='foo'):
-            from ansible.module_utils.common.sys_info import get_platform
-            self.assertEqual(get_platform(), 'foo')
+        assert get_platform() == 'foo'
 
-    def test_module_utils_basic_get_distribution(self):
-        from ansible.module_utils.common.sys_info import get_distribution
 
-        with patch('platform.system', return_value='Foo'):
-            self.assertEqual(get_distribution(), None)
+#
+# get_distribution tests
+#
 
-        with patch('platform.system', return_value='Linux'):
-            with patch('platform.linux_distribution', return_value=["foo"]):
-                self.assertEqual(get_distribution(), "Foo")
+def test_get_distribution_not_linux():
+    """If it's not Linux, then it has no distribution"""
+    with patch('platform.system', return_value='Foo'):
+        assert get_distribution() is None
 
-            with patch('os.path.isfile', return_value=True):
-                with patch('platform.linux_distribution', side_effect=[("AmazonFooBar", )]):
-                    self.assertEqual(get_distribution(), "Amazonfoobar")
 
-                with patch('platform.linux_distribution', side_effect=(("", ), ("AmazonFooBam",))):
-                    self.assertEqual(get_distribution(), "Amazon")
+@pytest.mark.usefixtures("platform_linux")
+class TestGetDistribution:
+    """ Tests for get_distribution that have to find somethine"""
+    def test_distro_known(self):
+        with patch('ansible.module_utils.distro.name', return_value="foo"):
+            assert get_distribution() == "Foo"
 
-                with patch('platform.linux_distribution', side_effect=[("", ), ("", )]):
-                    self.assertEqual(get_distribution(), "OtherLinux")
+    def test_distro_unknown(self):
+        with patch('ansible.module_utils.distro.name', return_value=""):
+            assert get_distribution() == "OtherLinux"
 
-                def _dist(distname='', version='', id='', supported_dists=(), full_distribution_name=1):
-                    if supported_dists != ():
-                        return ("Bar", "2", "Two")
-                    else:
-                        return ("", "", "")
+    def test_distro_amazon_part_of_another_name(self):
+        with patch('ansible.module_utils.distro.name', return_value="AmazonFooBar"):
+            assert get_distribution() == "Amazonfoobar"
 
-                with patch('platform.linux_distribution', side_effect=_dist):
-                    self.assertEqual(get_distribution(), "Bar")
+    def test_distro_amazon_linux(self):
+        with patch('ansible.module_utils.distro.name', return_value="Amazon Linux AMI"):
+            assert get_distribution() == "Amazon"
 
-            with patch('platform.linux_distribution', side_effect=Exception("boo")):
-                with patch('platform.dist', return_value=("bar", "2", "Two")):
-                    self.assertEqual(get_distribution(), "Bar")
 
-    def test_module_utils_basic_get_distribution_version(self):
-        from ansible.module_utils.common.sys_info import get_distribution_version
+#
+# get_distribution_version tests
+#
 
+def test_get_distribution_version_not_linux():
+    """If it's not Linux, then it has no distribution"""
     with patch('platform.system', return_value='Foo'):
-            self.assertEqual(get_distribution_version(), None)
+        assert get_distribution_version() is None
 
-        with patch('platform.system', return_value='Linux'):
-            with patch('platform.linux_distribution', return_value=("foo", "1", "One")):
-                self.assertEqual(get_distribution_version(), "1")
 
-            with patch('os.path.isfile', return_value=True):
-                def _dist(distname='', version='', id='', supported_dists=(), full_distribution_name=1):
-                    if supported_dists != ():
-                        return ("AmazonFooBar", "2", "")
-                    else:
-                        return ("", "", "")
+@pytest.mark.usefixtures("platform_linux")
+def test_distro_found():
+    with patch('ansible.module_utils.distro.version', return_value="1"):
+        assert get_distribution_version() == "1"
 
-                with patch('platform.linux_distribution', side_effect=_dist):
-                    self.assertEqual(get_distribution_version(), "2")
 
-            with patch('platform.linux_distribution', side_effect=Exception("boo")):
-                with patch('platform.dist', return_value=("bar", "3", "Three")):
-                    self.assertEqual(get_distribution_version(), "3")
+#
+# Tests for LoadPlatformSubclass
+#
 
-    def test_module_utils_basic_load_platform_subclass(self):
+class TestLoadPlatformSubclass:
     class LinuxTest:
         pass
 
@@ -90,19 +102,58 @@ class TestPlatform(ModuleTestCase):
         platform = "Linux"
         distribution = "Bar"
 
-        from ansible.module_utils.common.sys_info import load_platform_subclass
+    def test_not_linux(self):
+        # if neither match, the fallback should be the top-level class
+        with patch('ansible.module_utils.common.sys_info.get_platform', return_value="Foo"):
+            with patch('ansible.module_utils.common.sys_info.get_distribution', return_value=None):
+                assert isinstance(load_platform_subclass(self.LinuxTest), self.LinuxTest)
 
+    @pytest.mark.usefixtures("platform_linux")
+    def test_get_distribution_none(self):
         # match just the platform class, not a specific distribution
-        with patch('ansible.module_utils.common.sys_info.get_platform', return_value="Linux"):
         with patch('ansible.module_utils.common.sys_info.get_distribution', return_value=None):
-                self.assertIs(type(load_platform_subclass(LinuxTest)), Foo)
+            assert isinstance(load_platform_subclass(self.LinuxTest), self.Foo)
 
+    @pytest.mark.usefixtures("platform_linux")
+    def test_get_distribution_found(self):
         # match both the distribution and platform class
-        with patch('ansible.module_utils.common.sys_info.get_platform', return_value="Linux"):
         with patch('ansible.module_utils.common.sys_info.get_distribution', return_value="Bar"):
-                self.assertIs(type(load_platform_subclass(LinuxTest)), Bar)
+            assert isinstance(load_platform_subclass(self.LinuxTest), self.Bar)
 
-        # if neither match, the fallback should be the top-level class
-        with patch('ansible.module_utils.common.sys_info.get_platform', return_value="Foo"):
-            with patch('ansible.module_utils.common.sys_info.get_distribution', return_value=None):
-                self.assertIs(type(load_platform_subclass(LinuxTest)), LinuxTest)
+
+#
+# Tests for get_all_subclasses
+#
+
+class TestGetAllSubclasses:
+    class Base:
+        pass
+
+    class BranchI(Base):
+        pass
+
+    class BranchII(Base):
+        pass
+
+    class BranchIA(BranchI):
+        pass
+
+    class BranchIB(BranchI):
+        pass
+
+    class BranchIIA(BranchII):
+        pass
+
+    class BranchIIB(BranchII):
+        pass
+
+    def test_bottom_level(self):
+        assert get_all_subclasses(self.BranchIIB) == []
+
+    def test_one_inheritance(self):
+        assert set(get_all_subclasses(self.BranchII)) == set([self.BranchIIA, self.BranchIIB])
+
+    def test_toplevel(self):
+        assert set(get_all_subclasses(self.Base)) == set([self.BranchI, self.BranchII,
+                                                          self.BranchIA, self.BranchIB,
+                                                          self.BranchIIA, self.BranchIIB])

test/units/module_utils/test_distribution_version.py

@@ -7,8 +7,11 @@ __metaclass__ = type
 
 from itertools import product
 
+import mock
 import pytest
 
+from ansible.module_utils.six.moves import builtins
+
 # the module we are actually testing (sort of)
 from ansible.module_utils.facts.system.distribution import DistributionFactCollector
 
@@ -1044,7 +1047,7 @@ def test_distribution_version(am, mocker, testcase):
     * input files that are faked
       * those should be complete and also include "irrelevant" files that might be mistaken as coming from other distributions
       * all files that are not listed here are assumed to not exist at all
-    * the output of pythons platform.dist()
+    * the output of ansible.module_utils.distro.linux_distribution() [called platform.dist() for historical reasons]
     * results for the ansible variables distribution* and os_family
 
     """
@@ -1081,14 +1084,43 @@ def test_distribution_version(am, mocker, testcase):
     def mock_platform_version():
         return testcase.get('platform.version', '')
 
+    def mock_distro_name():
+        return testcase['platform.dist'][0]
+
+    def mock_distro_version():
+        return testcase['platform.dist'][1]
+
+    def mock_distro_codename():
+        return testcase['platform.dist'][2]
+
+    def mock_open(filename, mode='r'):
+        if filename in testcase['input']:
+            file_object = mocker.mock_open(read_data=testcase['input'][filename]).return_value
+            file_object.__iter__.return_value = testcase['input'][filename].splitlines(True)
+        else:
+            file_object = real_open(filename, mode)
+        return file_object
+
+    def mock_os_path_is_file(filename):
+        if filename in testcase['input']:
+            return True
+        return False
+
     mocker.patch('ansible.module_utils.facts.system.distribution.get_file_content', mock_get_file_content)
     mocker.patch('ansible.module_utils.facts.system.distribution.get_uname_version', mock_get_uname_version)
     mocker.patch('ansible.module_utils.facts.system.distribution._file_exists', mock_file_exists)
-    mocker.patch('platform.dist', lambda: testcase['platform.dist'])
+    mocker.patch('ansible.module_utils.distro.name', mock_distro_name)
+    mocker.patch('ansible.module_utils.distro.id', mock_distro_name)
+    mocker.patch('ansible.module_utils.distro.version', mock_distro_version)
+    mocker.patch('ansible.module_utils.distro.codename', mock_distro_codename)
+    mocker.patch('os.path.isfile', mock_os_path_is_file)
     mocker.patch('platform.system', mock_platform_system)
     mocker.patch('platform.release', mock_platform_release)
     mocker.patch('platform.version', mock_platform_version)
 
+    real_open = builtins.open
+    mocker.patch.object(builtins, 'open', new=mock_open)
+
     # run Facts()
     distro_collector = DistributionFactCollector()
     generated_facts = distro_collector.collect(am)

test/units/module_utils/test_distro.py

@@ -16,6 +16,8 @@ import platform
 import pytest
 
 from ansible.module_utils import distro
+from ansible.module_utils.common.sys_info import (get_distribution, get_distribution_version,
+                                                  get_distribution_codename)
 
 
 # Generic test case with minimal assertions about specific returned values.
@@ -42,11 +44,12 @@ class TestDistroCompat():
     _platform_supported_dists = platform._supported_dists
 
     def test_linux_distribution(self):
+        distro_linux_dist = (get_distribution(), get_distribution_version(), get_distribution_codename())
+
         platform_linux_dist = platform.linux_distribution(supported_dists=self._platform_supported_dists)
-        distro_linux_dist = distro.linux_distribution()
 
         assert isinstance(distro_linux_dist, type(platform_linux_dist)), \
-            'linux_distrution() returned type (%s) which is different from platform.linux_distribution type (%s)' % \
+            'linux_distribution() returned type (%s) which is different from platform.linux_distribution type (%s)' % \
             (type(distro_linux_dist), type(platform_linux_dist))
 
         # TODO: add the cases where we expect them to differ
@@ -55,6 +58,9 @@ class TestDistroCompat():
         assert distro_linux_dist[0] == platform_linux_dist[0]
         assert distro_linux_dist[1] == platform_linux_dist[1]
 
-        # FIXME: This fails on at least Fedora
-        # platform returns the 'id', while distro returns the 'codename'
+        if platform_linux_dist[0] == 'Fedora' and 20 < int(platform_linux_dist[1]) < 28:
+            pytest.skip("Fedora versions between 20 and 28 return the variant instead of the code name making this test unreliable")
+            # Fedora considers the platform_linux behaviour to have been a bug as it's finding the
+            # variant, not the code name.  Fedora wants this to be the empty string.
+            platform_linux_dist = platform_linux_dist[:2] + ('',)
         assert distro_linux_dist[2] == platform_linux_dist[2]