Dist version fix for Red Hat and more tests (#15663)

* add tests for centos6, rhel6 and rhel7

* gen_distribution_version_testcase with python2.6

* remove unused imports

* fix redhat/vmware/... parsing

* add centos7 test case

3 files changed, 157 insertions, 81 deletions

diff --git a/hacking/tests/gen_distribution_version_testcase.py b/hacking/tests/gen_distribution_version_testcase.py
index d7355040cd..f221100b71 100755
--- a/hacking/tests/gen_distribution_version_testcase.py
+++ b/hacking/tests/gen_distribution_version_testcase.py
@@ -46,11 +46,14 @@ dist = platform.dist()
 
 
 facts = ['distribution', 'distribution_version', 'distribution_release', 'distribution_major_version']
-ansible_out = subprocess.check_output(['ansible', 'localhost', '-m', 'setup'])
+ansible_out = subprocess.Popen(['ansible', 'localhost', '-m', 'setup'], stdout=subprocess.PIPE).communicate()[0]
 parsed = json.loads(ansible_out[ansible_out.index('{'):])
 ansible_facts = {}
 for fact in facts:
-    ansible_facts[fact] = parsed['ansible_facts']['ansible_'+fact]
+    try:
+        ansible_facts[fact] = parsed['ansible_facts']['ansible_'+fact]
+    except:
+        ansible_facts[fact] = "N/A"
 
 nicename = ansible_facts['distribution'] + ' ' + ansible_facts['distribution_version']
 
diff --git a/lib/ansible/module_utils/facts.py b/lib/ansible/module_utils/facts.py
index ce8a9cc5af..c3377aa0e5 100644
--- a/lib/ansible/module_utils/facts.py
+++ b/lib/ansible/module_utils/facts.py
@@ -605,25 +605,31 @@ class Distribution(object):
     """
 
     OSDIST_LIST = (
-        ('/etc/oracle-release', 'OracleLinux'),
-        ('/etc/slackware-version', 'Slackware'),
-        ('/etc/redhat-release', 'RedHat'),
-        ('/etc/vmware-release', 'VMwareESX'),
-        ('/etc/openwrt_release', 'OpenWrt'),
-        ('/etc/system-release', 'OtherLinux'),
-        ('/etc/alpine-release', 'Alpine'),
-        ('/etc/release', 'Solaris'),
-        ('/etc/arch-release', 'Archlinux'),
-        ('/etc/os-release', 'SuSE'),
-        ('/etc/SuSE-release', 'SuSE'),
-        ('/etc/gentoo-release', 'Gentoo'),
-        ('/etc/os-release', 'Debian'),
-        ('/etc/lsb-release', 'Mandriva'),
-        ('/etc/altlinux-release', 'Altlinux'),
-        ('/etc/os-release', 'NA'),
-        ('/etc/coreos/update.conf', 'Coreos'),
+        {'path': '/etc/oracle-release', 'name': 'OracleLinux'},
+        {'path': '/etc/slackware-version', 'name': 'Slackware'},
+        {'path': '/etc/redhat-release', 'name': 'RedHat'},
+        {'path': '/etc/vmware-release', 'name': 'VMwareESX', 'allowempty': True},
+        {'path': '/etc/openwrt_release', 'name': 'OpenWrt'},
+        {'path': '/etc/system-release', 'name': 'Amazon'},
+        {'path': '/etc/alpine-release', 'name': 'Alpine'},
+        {'path': '/etc/release', 'name': 'Solaris'},
+        {'path': '/etc/arch-release', 'name': 'Archlinux', 'allowempty': True},
+        {'path': '/etc/os-release', 'name': 'SuSE'},
+        {'path': '/etc/SuSE-release', 'name': 'SuSE'},
+        {'path': '/etc/gentoo-release', 'name': 'Gentoo'},
+        {'path': '/etc/os-release', 'name': 'Debian'},
+        {'path': '/etc/lsb-release', 'name': 'Mandriva'},
+        {'path': '/etc/altlinux-release', 'name': 'Altlinux'},
+        {'path': '/etc/os-release', 'name': 'NA'},
+        {'path': '/etc/coreos/update.conf', 'name': 'Coreos'},
     )
 
+    SEARCH_STRING = {
+        'OracleLinux': 'Oracle Linux',
+        'RedHat': 'Red Hat',
+        'Altlinux': 'ALT Linux',
+    }
+
     # A list with OS Family members
     OS_FAMILY = dict(
         RedHat = 'RedHat', Fedora = 'RedHat', CentOS = 'RedHat', Scientific = 'RedHat',
@@ -675,36 +681,47 @@ class Distribution(object):
             self.facts['distribution_release'] = dist[2] or 'NA'
             # Try to handle the exceptions now ...
             # self.facts['distribution_debug'] = []
-            for (path, name) in self.OSDIST_LIST:
+            for ddict in self.OSDIST_LIST:
+                name = ddict['name']
+                path = ddict['path']
 
-                # TODO: remove this hack if tested with Fedora and Altlinux
-                if self.facts['distribution'] in ('Fedora', 'Altlinux', ):
-                    # Once we determine the value is one of these distros
-                    # we trust the values are always correct
-                    break
                 if not os.path.exists(path):
                     continue
                 if os.path.getsize(path) == 0:
-                    continue
+                    if 'allowempty' in ddict and ddict['allowempty']:
+                        self.facts['distribution'] = name
+                        break
+                    else:
+                        continue
 
-                try:
+                data = get_file_content(path)
+                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()
+                    if self.SEARCH_STRING[name] in data:
+                        # this sets distribution=RedHat if 'Red Hat' shows up in data
+                        self.facts['distribution'] = name
+                    else:
+                        # this sets distribution to what's in the data, e.g. CentOS, Scientific, ...
+                        self.facts['distribution'] = data.split()[0]
+                    break
+                else:
+                    # call a dedicated function for parsing the file content
                     distfunc = getattr(self, 'get_distribution_' + name)
-                    data = get_file_content(path)
                     parsed = distfunc(name, data, path)
                     if parsed is None or parsed:
+                        # distfunc return False if parsing failed
                         # break only if parsing was succesful
                         # otherwise continue with other distributions
-
-                        # to debug multiple matching release files, one can use:
-                        # self.facts['distribution_debug'].append({path + ' ' + name:
-                        #         (parsed,
-                        #          self.facts['distribution'],
-                        #          self.facts['distribution_version'],
-                        #          self.facts['distribution_release'],
-                        #          )})
                         break
-                except AttributeError:
-                    pass
+
+                    # to debug multiple matching release files, one can use:
+                    # self.facts['distribution_debug'].append({path + ' ' + name:
+                    #         (parsed,
+                    #          self.facts['distribution'],
+                    #          self.facts['distribution_version'],
+                    #          self.facts['distribution_release'],
+                    #          )})
 
         self.facts['os_family'] = self.facts['distribution']
         distro = self.facts['distribution'].replace(' ', '_')
@@ -738,47 +755,30 @@ class Distribution(object):
         else:
             self.facts['distribution_version'] = 'release'
 
-    def get_distribution_Archlinux(self, name, data, path):
-        if not 'Arch Linux' in data:
-            self.facts['distribution'] = data.split()[0]
-
     def get_distribution_Slackware(self, name, data, path):
-        if 'Slackware' in data:
-            self.facts['distribution'] = name
-            version = re.findall('\w+[.]\w+', data)
-            if version:
-                self.facts['distribution_version'] = version[0]
-
-    def get_distribution_OracleLinux(self, name, data, path):
-        if not 'Oracle Linux' in data:
-            self.facts['distribution'] = data.split()[0]
-
-    def get_distribution_RedHat(self, name, data, path):
-        if not 'Red Hat' in data:
-            self.facts['distribution'] = data.split()[0]
-
-    def get_distribution_Altlinux(self, name, data, path):
-        if not 'ALT Linux' in data:
-            self.facts['distribution'] = data.split()[0]
-
-    def get_distribution_OtherLinux(self, name, data, path):
-        if 'Amazon' in data:
-            self.facts['distribution'] = 'Amazon'
-            self.facts['distribution_version'] = data.split()[-1]
-        else:
-            return False  # TODO: remove if tested without this
+        if 'Slackware' not in data:
+            return False  # TODO: remove
+        self.facts['distribution'] = name
+        version = re.findall('\w+[.]\w+', data)
+        if version:
+            self.facts['distribution_version'] = version[0]
+
+    def get_distribution_Amazon(self, name, data, path):
+        if 'Amazon' not in data:
+            return False  # TODO: remove
+        self.facts['distribution'] = 'Amazon'
+        self.facts['distribution_version'] = data.split()[-1]
 
     def get_distribution_OpenWrt(self, name, data, path):
-        if 'OpenWrt' in data:
-            self.facts['distribution'] = name
-            version = re.search('DISTRIB_RELEASE="(.*)"', data)
-            if version:
-                self.facts['distribution_version'] = version.groups()[0]
-            release = re.search('DISTRIB_CODENAME="(.*)"', data)
-            if release:
-                self.facts['distribution_release'] = release.groups()[0]
-        else:
-            return False  # TODO: remove if tested without this
+        if 'OpenWrt' not in data:
+            return False  # TODO: remove
+        self.facts['distribution'] = name
+        version = re.search('DISTRIB_RELEASE="(.*)"', data)
+        if version:
+            self.facts['distribution_version'] = version.groups()[0]
+        release = re.search('DISTRIB_CODENAME="(.*)"', data)
+        if release:
+            self.facts['distribution_release'] = release.groups()[0]
 
     def get_distribution_Alpine(self, name, data, path):
         self.facts['distribution'] = 'Alpine'
diff --git a/test/units/module_utils/test_distribution_version.py b/test/units/module_utils/test_distribution_version.py
index e3e5f3d391..3ade3b0c3a 100644
--- a/test/units/module_utils/test_distribution_version.py
+++ b/test/units/module_utils/test_distribution_version.py
@@ -22,9 +22,6 @@ import sys
 
 # to work around basic.py reading stdin
 import json
-from io import BytesIO, StringIO
-from ansible.compat.six import PY3
-from ansible.utils.unicode import to_bytes
 
 from units.mock.procenv import swap_stdin_and_argv
 
@@ -38,6 +35,82 @@ from ansible.compat.tests.mock import patch
 # to generate the testcase data, you can use the script gen_distribution_version_testcase.py in hacking/tests
 TESTSETS = [
     {
+    "platform.dist": [
+        "centos",
+        "7.2.1511",
+        "Core"
+    ],
+    "input": {
+        "/etc/redhat-release": "CentOS Linux release 7.2.1511 (Core) \n",
+        "/etc/os-release": "NAME=\"CentOS Linux\"\nVERSION=\"7 (Core)\"\nID=\"centos\"\nID_LIKE=\"rhel fedora\"\nVERSION_ID=\"7\"\nPRETTY_NAME=\"CentOS Linux 7 (Core)\"\nANSI_COLOR=\"0;31\"\nCPE_NAME=\"cpe:/o:centos:centos:7\"\nHOME_URL=\"https://www.centos.org/\"\nBUG_REPORT_URL=\"https://bugs.centos.org/\"\n\nCENTOS_MANTISBT_PROJECT=\"CentOS-7\"\nCENTOS_MANTISBT_PROJECT_VERSION=\"7\"\nREDHAT_SUPPORT_PRODUCT=\"centos\"\nREDHAT_SUPPORT_PRODUCT_VERSION=\"7\"\n\n",
+        "/etc/system-release": "CentOS Linux release 7.2.1511 (Core) \n"
+    },
+    "name": "CentOS 7.2.1511",
+    "result": {
+        "distribution_release": "Core",
+        "distribution": "CentOS",
+        "distribution_major_version": "7",
+        "distribution_version": "7.2.1511"
+    }
+},
+    {
+    "name": "CentOS 6.7",
+    "platform.dist": [
+        "centos",
+        "6.7",
+        "Final"
+    ],
+    "input": {
+        "/etc/redhat-release": "CentOS release 6.7 (Final)\n",
+        "/etc/lsb-release": "LSB_VERSION=base-4.0-amd64:base-4.0-noarch:core-4.0-amd64:core-4.0-noarch:graphics-4.0-amd64:graphics-4.0-noarch:printing-4.0-amd64:printing-4.0-noarch\n",
+        "/etc/system-release": "CentOS release 6.7 (Final)\n"
+    },
+    "result": {
+        "distribution_release": "Final",
+        "distribution": "CentOS",
+        "distribution_major_version": "6",
+        "distribution_version": "6.7"
+    }
+},
+    {
+    "name": "RedHat 7.2",
+    "platform.dist": [
+        "redhat",
+        "7.2",
+        "Maipo"
+    ],
+    "input": {
+        "/etc/redhat-release": "Red Hat Enterprise Linux Server release 7.2 (Maipo)\n",
+        "/etc/os-release": "NAME=\"Red Hat Enterprise Linux Server\"\nVERSION=\"7.2 (Maipo)\"\nID=\"rhel\"\nID_LIKE=\"fedora\"\nVERSION_ID=\"7.2\"\nPRETTY_NAME=\"Red Hat Enterprise Linux Server 7.2 (Maipo)\"\nANSI_COLOR=\"0;31\"\nCPE_NAME=\"cpe:/o:redhat:enterprise_linux:7.2:GA:server\"\nHOME_URL=\"https://www.redhat.com/\"\nBUG_REPORT_URL=\"https://bugzilla.redhat.com/\"\n\nREDHAT_BUGZILLA_PRODUCT=\"Red Hat Enterprise Linux 7\"\nREDHAT_BUGZILLA_PRODUCT_VERSION=7.2\nREDHAT_SUPPORT_PRODUCT=\"Red Hat Enterprise Linux\"\nREDHAT_SUPPORT_PRODUCT_VERSION=\"7.2\"\n",
+        "/etc/system-release": "Red Hat Enterprise Linux Server release 7.2 (Maipo)\n"
+    },
+    "result": {
+        "distribution_release": "Maipo",
+        "distribution": "RedHat",
+        "distribution_major_version": "7",
+        "distribution_version": "7.2"
+    }
+},
+{
+    "name": "RedHat 6.7",
+    "platform.dist": [
+        "redhat",
+        "6.7",
+        "Santiago"
+    ],
+    "input": {
+        "/etc/redhat-release": "Red Hat Enterprise Linux Server release 6.7 (Santiago)\n",
+        "/etc/lsb-release": "LSB_VERSION=base-4.0-amd64:base-4.0-noarch:core-4.0-amd64:core-4.0-noarch:graphics-4.0-amd64:graphics-4.0-noarch:printing-4.0-amd64:printing-4.0-noarch\n",
+        "/etc/system-release": "Red Hat Enterprise Linux Server release 6.7 (Santiago)\n"
+    },
+    "result": {
+        "distribution_release": "Santiago",
+        "distribution": "RedHat",
+        "distribution_major_version": "6",
+        "distribution_version": "6.7"
+    }
+},
+{
         "name" : "openSUSE Leap 42.1",
         "input": {
             "/etc/os-release":
@@ -339,7 +412,7 @@ def test_distribution_version():
 
 def _test_one_distribution(facts, module, testcase):
     """run the test on one distribution testcase
-    
+
     * prepare some mock functions to get the testdata in
     * run Facts()
     * compare with the expected output
@@ -373,7 +446,7 @@ def _test_one_distribution(facts, module, testcase):
     @patch('platform.system', lambda: 'Linux')
     def get_facts(testcase):
         return facts.Facts(module).populate()
-    
+
     generated_facts = get_facts(testcase)
 
     # testcase['result'] has a list of variables and values it expects Facts() to set