Merge pull request #277 from fstagni/redhat_release__not

Avoid silent failure when checking the worker node: /etc/redhat-release

Author: aldbr

Pilot/pilotCommands.py

@@ -115,15 +115,19 @@ def execute(self):
         self.log.info("Host FQDN  = %s" % socket.getfqdn())
         self.log.info("WorkingDir = %s" % self.pp.workingDir)  # this could be different than rootPath
 
-        fileName = "/etc/redhat-release"
-        if os.path.exists(fileName):
-            with open(fileName, "r") as f:
-                self.log.info("RedHat Release = %s" % f.read().strip())
-
-        fileName = "/etc/lsb-release"
-        if os.path.isfile(fileName):
-            with open(fileName, "r") as f:
-                self.log.info("Linux release:\n%s" % f.read().strip())
+        for fileName in ["/etc/os-release", "/usr/lib/os-release"]:
+            if os.path.isfile(fileName):
+                try:
+                    with open(fileName, "r") as f:
+                        for line in f:
+                            line = line.strip()
+                            if line.startswith(("NAME=", "VERSION=", "PRETTY_NAME=")):
+                                self.log.info(
+                                    "OS Release = %s" % line.split("=", 1)[1].strip('"')
+                                )
+                    break
+                except (OSError, IOError):
+                    self.log.debug("Could not read %s" % fileName)
 
         fileName = "/proc/cpuinfo"
         if os.path.exists(fileName):

Pilot/tests/Test_Pilot.py

@@ -5,6 +5,8 @@
 import shutil
 import stat
 import sys
+import tempfile
+from unittest import mock
 
 # pylint: disable=protected-access, missing-docstring, invalid-name, line-too-long
 # imports
@@ -33,7 +35,12 @@ def setUp(self):
                             "Version": "v1r1, v2r2",
                         }
                     },
-                    "CEs": {"grid1.example.com": {"GridCEType": "cetype1", "Site": "site.example.com"}},
+                    "CEs": {
+                        "grid1.example.com": {
+                            "GridCEType": "cetype1",
+                            "Site": "site.example.com",
+                        }
+                    },
                     "DefaultSetup": "TestSetup",
                 },
                 fp,
@@ -62,14 +69,33 @@ def tearDown(self):
                 shutil.rmtree("ReplacementCode")
             except OSError:
                 pass
+            try:
+                shutil.rmtree("etc")
+            except OSError:
+                pass
+            try:
+                os.remove(fileProd)
+            except OSError:
+                pass
+            try:
+                shutil.rmtree("ReplacementCode")
+            except OSError:
+                pass
 
 
 class CommandsTestCase(PilotTestCase):
     """Test case for each pilot command"""
 
     def test_InitJSON(self):
         """Test the pilot.json and command line parsing"""
-        sys.argv[1:] = ["--Name", "grid1.example.com", "--commandOptions", "a=1,b=2", "-Z", "c=3"]
+        sys.argv[1:] = [
+            "--Name",
+            "grid1.example.com",
+            "--commandOptions",
+            "a=1,b=2",
+            "-Z",
+            "c=3",
+        ]
         pp = PilotParams()
 
         self.assertEqual(pp.commands, ["x", "y", "z"])
@@ -93,7 +119,13 @@ def test_InitJSON(self):
         self.assertEqual(pp.commandOptions["b"], "2")
         self.assertEqual(pp.commandOptions["c"], "3")
 
-        sys.argv[1:] = ["--Name", "grid1.example.com", "--commandOptions=a = 1,  b=2", "-Z", " c=3"]  # spaces and '=''
+        sys.argv[1:] = [
+            "--Name",
+            "grid1.example.com",
+            "--commandOptions=a = 1,  b=2",
+            "-Z",
+            " c=3",
+        ]  # spaces and '=''
         pp = PilotParams()
 
         self.assertEqual(pp.commandOptions["a"], "1")
@@ -105,6 +137,42 @@ def test_CheckWorkerNode(self):
         pp = PilotParams()
         cwn = CheckWorkerNode(pp)
         self.assertEqual(cwn.execute(), None)
+        with open("pilot.out") as po:
+            s = po.read()
+            self.assertTrue("OS Release =" in s)
+
+    def test_CheckWorkerNode_osrelease_unavailable(self):
+        """Test CheckWorkerNode when os-release is unavailable"""
+        pp = PilotParams()
+        cwn = CheckWorkerNode(pp)
+
+        with mock.patch("pilotCommands.os.path.isfile") as mock_isfile:
+            mock_isfile.return_value = False
+
+            cwn.execute()
+
+            with open("pilot.out") as po:
+                s = po.read()
+                self.assertNotIn("OS Release =", s)
+
+    def test_CheckWorkerNode_osrelease_blocked(self):
+        """Test CheckWorkerNode when os-release access is blocked"""
+        pp = PilotParams()
+        cwn = CheckWorkerNode(pp)
+
+        original_open = open
+
+        def selective_open(path, *args, **kwargs):
+            if path in ["/etc/os-release", "/usr/lib/os-release"]:
+                raise IOError("Permission denied")
+            return original_open(path, *args, **kwargs)
+
+        with mock.patch("pilotCommands.open", side_effect=selective_open):
+            cwn.execute()
+
+            with open("pilot.out") as po:
+                s = po.read()
+                self.assertNotIn("OS Release =", s)
 
     def test_ConfigureSite(self):
         """Test ConfigureSite command"""