Make _parse_os_release() always callable

To simplify running unit tests against os_data(), make the
_parse_os_release() function always callable to avoid needing to mock
os.path.isfile().

Signed-off-by: Benjamin Drung <benjamin.drung@profitbricks.com>

salt/grains/core.py

@@ -1218,19 +1218,22 @@ def _get_interfaces():
     return _INTERFACES
 
 
-def _parse_os_release():
+def _parse_os_release(os_release_files):
     '''
-    Parse /etc/os-release and return a parameter dictionary
+    Parse os-release and return a parameter dictionary
 
     See http://www.freedesktop.org/software/systemd/man/os-release.html
     for specification of the file format.
     '''
 
-    filename = '/etc/os-release'
-    if not os.path.isfile(filename):
-        filename = '/usr/lib/os-release'
-
     data = dict()
+    for filename in os_release_files:
+        if os.path.isfile(filename):
+            break
+    else:
+        # None of the specified os-release files exist
+        return data
+
     with salt.utils.fopen(filename) as ifile:
         regex = re.compile('^([\\w]+)=(?:\'|")?(.*?)(?:\'|")?$')
         for line in ifile:
@@ -1414,8 +1417,8 @@ def os_data():
                 # to be incorrectly set to "Arch".
                 grains['osfullname'] = 'Antergos Linux'
             elif 'lsb_distrib_id' not in grains:
-                if os.path.isfile('/etc/os-release') or os.path.isfile('/usr/lib/os-release'):
-                    os_release = _parse_os_release()
+                os_release = _parse_os_release(['/etc/os-release', '/usr/lib/os-release'])
+                if os_release:
                     if 'NAME' in os_release:
                         grains['lsb_distrib_id'] = os_release['NAME'].strip()
                     if 'VERSION_ID' in os_release:

tests/unit/grains/test_core.py

@@ -147,9 +147,6 @@ class CoreGrainsTestCase(TestCase, LoaderModuleMockMixin):
         _path_exists_map = {
             '/proc/1/cmdline': False
         }
-        _path_isfile_map = {
-            '/etc/os-release': True,
-        }
         _os_release_map = {
             'NAME': 'SLES',
             'VERSION': '12-SP1',
@@ -161,9 +158,6 @@ class CoreGrainsTestCase(TestCase, LoaderModuleMockMixin):
         }
 
         path_exists_mock = MagicMock(side_effect=lambda x: _path_exists_map[x])
-        path_isfile_mock = MagicMock(
-            side_effect=lambda x: _path_isfile_map.get(x, False)
-        )
         empty_mock = MagicMock(return_value={})
         osarch_mock = MagicMock(return_value="amd64")
         os_release_mock = MagicMock(return_value=_os_release_map)
@@ -191,7 +185,7 @@ class CoreGrainsTestCase(TestCase, LoaderModuleMockMixin):
                     with patch('{0}.__import__'.format(built_in),
                                side_effect=_import_mock):
                         # Skip all the /etc/*-release stuff (not pertinent)
-                        with patch.object(os.path, 'isfile', path_isfile_mock):
+                        with patch.object(os.path, 'isfile', MagicMock(return_value=False)):
                             with patch.object(core, '_parse_os_release', os_release_mock):
                                 # Mock linux_distribution to give us the OS
                                 # name that we want.
@@ -210,10 +204,10 @@ class CoreGrainsTestCase(TestCase, LoaderModuleMockMixin):
         self.assertEqual(os_grains.get('os'), 'SUSE')
 
     def _run_os_grains_tests(self, os_release_map):
-        path_isfile_mock = MagicMock(side_effect=lambda x: x in os_release_map['files'])
+        path_isfile_mock = MagicMock(side_effect=lambda x: x in os_release_map.get('files', []))
         empty_mock = MagicMock(return_value={})
         osarch_mock = MagicMock(return_value="amd64")
-        os_release_mock = MagicMock(return_value=os_release_map.get('os_release_file'))
+        os_release_mock = MagicMock(return_value=os_release_map.get('os_release_file', {}))
 
         orig_import = __import__
         if six.PY2:
@@ -309,7 +303,6 @@ PATCHLEVEL = 3
             'osrelease': '11.4',
             'osrelease_info': [11, 4],
             'osmajorrelease': 11,
-            'files': ["/etc/os-release"],
         }
         self._run_suse_os_grains_tests(_os_release_map)
 
@@ -333,7 +326,6 @@ PATCHLEVEL = 3
             'osrelease': '12',
             'osrelease_info': [12],
             'osmajorrelease': 12,
-            'files': ["/etc/os-release"],
         }
         self._run_suse_os_grains_tests(_os_release_map)
 
@@ -357,7 +349,6 @@ PATCHLEVEL = 3
             'osrelease': '12.1',
             'osrelease_info': [12, 1],
             'osmajorrelease': 12,
-            'files': ["/etc/os-release"],
         }
         self._run_suse_os_grains_tests(_os_release_map)
 
@@ -381,7 +372,6 @@ PATCHLEVEL = 3
             'osrelease': '42.1',
             'osrelease_info': [42, 1],
             'osmajorrelease': 42,
-            'files': ["/etc/os-release"],
         }
         self._run_suse_os_grains_tests(_os_release_map)
 
@@ -405,7 +395,6 @@ PATCHLEVEL = 3
             'osrelease': '20160504',
             'osrelease_info': [20160504],
             'osmajorrelease': 20160504,
-            'files': ["/etc/os-release"],
         }
         self._run_suse_os_grains_tests(_os_release_map)
 
@@ -431,7 +420,6 @@ PATCHLEVEL = 3
             'osrelease_info': [16, 4],
             'osmajorrelease': 16,
             'osfinger': 'Ubuntu-16.04',
-            'files': '/etc/os-release',
         }
         self._run_os_grains_tests(_os_release_map)