feat: Make PilotLoggingAgent truly remote to TornadoPilotLogginghandl…

…er (delete by agent + tests)

src/DIRAC/WorkloadManagementSystem/Agent/PilotLoggingAgent.py

@@ -129,7 +129,7 @@ def executeForVO(self, vo):
 
         if not resDict["OK"]:
             return resDict
-        # get all successful filename and corresponding logs, write them to temporary files and upload:
+        # get all successful filenames and corresponding logs, write them to temporary files and upload:
         toBeDeleted = []
         with tempfile.TemporaryDirectory() as tmpdirname:
             if not resDict["Value"]["Successful"]:
@@ -147,27 +147,6 @@ def executeForVO(self, vo):
                         self.log.verbose("File uploaded: ", f"LFN = {res['Value']}")
                         toBeDeleted.append(key)
         client.deleteLogs(toBeDeleted, vo)
+        # delete old logs
+        client.clearLogs(self.clearPilotsDelay, vo)
         return S_OK()
-
-    def clearOldPilotLogs(self, pilotLogPath):
-        """
-        Delete old pilot log files unconditionally. Assumes that pilotLogPath exists.
-
-        :param str pilotLogPath: log files directory
-        :return: None
-        :rtype: None
-        """
-
-        files = os.listdir(pilotLogPath)
-        seconds = int(self.clearPilotsDelay) * 86400
-        currentTime = time.time()
-
-        for file in files:
-            fullpath = os.path.join(pilotLogPath, file)
-            modifTime = os.stat(fullpath).st_mtime
-            if modifTime < currentTime - seconds:
-                self.log.debug(f" Deleting old log : {fullpath}")
-                try:
-                    os.remove(fullpath)
-                except Exception as excp:
-                    self.log.exception(f"Cannot remove an old log file after {fullpath}", lException=excp)

src/DIRAC/WorkloadManagementSystem/Agent/test/Test_Agent_PilotLoggingAgent.py

@@ -141,12 +141,12 @@ def test_executeForVO(pla, ppath, files, result):
     assert res == S_OK()
 
 
-def test_executeForVOMetaFails(pla):
+def test_executeForVOGetLogsFails(pla):
     opsHelperValues = {"OK": True, "Value": {"UploadSE": "testUploadSE", "UploadPath": "/gridpp/uploadPath"}}
     mockOperations.return_value.getOptionsDict.return_value = opsHelperValues
     pla.opsHelper = mockOperations.return_value
-    # getMetadata call fails.
-    mockTornadoClient.return_value.getMetadata.return_value = {"OK": False, "Message": "Failed, sorry.."}
+    # getLogs call fails.
+    mockTornadoClient.return_value.getLogs.return_value = {"OK": False, "Message": "Failed, sorry.."}
     res = pla.executeForVO(vo="anything")
     assert res["OK"] is False
 

src/DIRAC/WorkloadManagementSystem/Client/PilotLoggingPlugins/FileCacheLoggingPlugin.py

@@ -5,6 +5,7 @@
 import json
 import re
 import time
+from pathlib import Path
 from DIRAC import S_OK, S_ERROR, gLogger
 from DIRAC.WorkloadManagementSystem.Client.PilotLoggingPlugins.PilotLoggingPlugin import PilotLoggingPlugin
 
@@ -117,29 +118,38 @@ def getLogs(self, vo):
 
     def deleteLogs(self, filelist, vo):
         """
-        Delete log files from the server cache.
+        Delete log files from the server cache. The file has to be in a subdirectory of a vo-specific root.
+
         :param list filelist: list of pilot log files to be deleted
         :param str vo: VO name
-        :return: Dirac S_OK
+        :return: Dirac S_OK containing successful nad failed upload lists: {"Successful": [...], "Failed": [...]}
         :rtype: dict
         """
 
+        resultDict = {"Successful": [], "Failed": []}
         for elem in filelist:
-            fullpath = os.path.join(self._logPath, vo, elem)
-            sLog.debug(f" Deleting pilot log : {fullpath}")
+            topdir = os.path.join(self._logPath, vo)
+            fullpath = os.path.join(topdir, elem)
             try:
+                # if Path(topdir) not in Path(fullpath).resolve().parents:
+                if not Path(fullpath).resolve().is_relative_to(Path(topdir)):
+                    raise Exception
+                sLog.debug(f" Deleting pilot log : {fullpath}")
                 os.remove(fullpath)
+                resultDict["Successful"].append(fullpath)
             except Exception as excp:
                 sLog.exception(f"Cannot remove a log file {fullpath}", lException=excp)
-        return S_OK()
+                resultDict["Failed"].append(fullpath)
+        return S_OK(resultDict)
 
     def clearLogs(self, clearPilotsDelay, vo):
         """
         Delete old pilot log files if older that clearPilotsDelay days
 
-        :param int pilotLogPath: maximum file age.
-        :return: None
-        :rtype: None
+        :param int clearPilotsDelay: maximum file age.
+        :param str vo: VO name
+        :return: Dirac S_OK()
+        :rtype: dict
         """
 
         seconds = int(clearPilotsDelay) * 86400
@@ -149,11 +159,11 @@ def clearLogs(self, clearPilotsDelay, vo):
             fullpath = os.path.join(self._logPath, vo, elem)
             modifTime = os.stat(fullpath).st_mtime
             if modifTime < currentTime - seconds:
-                self.log.debug(f" Deleting old log : {fullpath}")
+                sLog.debug(f" Deleting old log : {fullpath}")
                 try:
                     os.remove(fullpath)
                 except Exception as excp:
-                    self.log.exception(f"Cannot remove an old log file after {fullpath}", lException=excp)
+                    sLog.exception(f"Cannot remove an old log file {fullpath}", lException=excp)
         return S_OK()
 
     def getLog(self, logfile, vo):

src/DIRAC/WorkloadManagementSystem/Service/TornadoPilotLoggingHandler.py

@@ -105,16 +105,17 @@ def export_deleteLogs(self, filelist, vo):
 
         return self.loggingPlugin.deleteLogs(filelist, vo)
 
-    def export_clearLogs(self, vo):
+    def export_clearLogs(self, age, vo):
         """
-        Clear all logs for a given VO.
+        Clear all logs for a given VO (age based).
 
+        :param int age: file age in days
         :param str vo: VO name
         :return: S_OK or S_ERROR
         :rtype: dict
         """
 
-        return self.loggingPlugin.clearLogs(vo)
+        return self.loggingPlugin.clearLogs(age, vo)
 
     def export_getLog(self, logfile, vo):
         """