handle-core-dumps deletes old core-dumps

3 files changed, 52 insertions, 33 deletions

diff --git a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/maintainer/CoredumpHandler.java b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/maintainer/CoredumpHandler.java
index 5794e5e5389..8e50feee506 100644
--- a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/maintainer/CoredumpHandler.java
+++ b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/maintainer/CoredumpHandler.java
@@ -37,23 +37,42 @@ public class CoredumpHandler {
 
     private final HttpClient httpClient;
     private final CoreCollector coreCollector;
+    private final Path coredumpsPath;
+    private final Path doneCoredumpsPath;
+    private final Map<String, Object> nodeAttributes;
 
-    public CoredumpHandler(HttpClient httpClient, CoreCollector coreCollector) {
+    public CoredumpHandler(HttpClient httpClient, CoreCollector coreCollector, Path coredumpsPath, Path doneCoredumpsPath,
+                           Map<String, Object> nodeAttributes) {
         this.httpClient = httpClient;
         this.coreCollector = coreCollector;
+        this.coredumpsPath = coredumpsPath;
+        this.doneCoredumpsPath = doneCoredumpsPath;
+        this.nodeAttributes = nodeAttributes;
     }
 
-    public void processAndReportCoredumps(Path coredumpsPath, Path doneCoredumpPath, Map<String, Object> nodeAttributes) throws IOException {
-        Path processingCoredumps = processCoredumps(coredumpsPath, nodeAttributes);
-        reportCoredumps(processingCoredumps, doneCoredumpPath);
+    public void processAll() throws IOException {
+        removeJavaCoredumps();
+        processAndReportCoredumps();
+        removeOldCoredumps();
     }
 
-    public void removeJavaCoredumps(Path javaCoredumpsPath) throws IOException {
-        if (! javaCoredumpsPath.toFile().isDirectory()) return;
-        FileHelper.deleteFiles(javaCoredumpsPath, Duration.ZERO, Optional.of("^java_pid.*\\.hprof$"), false);
+    private void removeJavaCoredumps() throws IOException {
+        if (! coredumpsPath.toFile().isDirectory()) return;
+        FileHelper.deleteFiles(coredumpsPath, Duration.ZERO, Optional.of("^java_pid.*\\.hprof$"), false);
     }
 
-    Path processCoredumps(Path coredumpsPath, Map<String, Object> nodeAttributes) throws IOException {
+    private void removeOldCoredumps() throws IOException {
+        if (! doneCoredumpsPath.toFile().isDirectory()) return;
+        FileHelper.deleteDirectories(doneCoredumpsPath, Duration.ofDays(10), Optional.empty());
+    }
+
+    private void processAndReportCoredumps() throws IOException {
+        Path processingCoredumps = processCoredumps();
+        reportCoredumps(processingCoredumps);
+    }
+
+
+    Path processCoredumps() throws IOException {
         Path processingCoredumpsPath = coredumpsPath.resolve(PROCESSING_DIRECTORY_NAME);
         processingCoredumpsPath.toFile().mkdirs();
 
@@ -75,7 +94,7 @@ public class CoredumpHandler {
         return processingCoredumpsPath;
     }
 
-    void reportCoredumps(Path processingCoredumpsPath, Path doneCoredumpsPath) throws IOException {
+    void reportCoredumps(Path processingCoredumpsPath) throws IOException {
         doneCoredumpsPath.toFile().mkdirs();
 
         Files.list(processingCoredumpsPath)
@@ -83,7 +102,7 @@ public class CoredumpHandler {
                 .forEach(coredumpDirectory -> {
                     try {
                         report(coredumpDirectory);
-                        finishProcessing(coredumpDirectory, doneCoredumpsPath);
+                        finishProcessing(coredumpDirectory);
                     } catch (Throwable e) {
                         logger.log(Level.WARNING, "Failed to report coredump " + coredumpDirectory, e);
                     }
@@ -128,7 +147,7 @@ public class CoredumpHandler {
         logger.info("Successfully reported coredump " + documentId);
     }
 
-    void finishProcessing(Path coredumpDirectory, Path doneCoredumpsPath) throws IOException {
+    void finishProcessing(Path coredumpDirectory) throws IOException {
         Files.move(coredumpDirectory, doneCoredumpsPath.resolve(coredumpDirectory.getFileName()));
     }
 }
diff --git a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/maintainer/Maintainer.java b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/maintainer/Maintainer.java
index 4c9e80aaa8c..8447e04eb4b 100644
--- a/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/maintainer/Maintainer.java
+++ b/node-maintainer/src/main/java/com/yahoo/vespa/hosted/node/maintainer/Maintainer.java
@@ -6,6 +6,7 @@ import com.yahoo.slime.Inspector;
 import com.yahoo.slime.Type;
 import com.yahoo.system.ProcessExecuter;
 import com.yahoo.vespa.config.SlimeUtils;
+import org.apache.http.client.HttpClient;
 import org.apache.http.impl.client.HttpClientBuilder;
 
 import java.io.IOException;
@@ -21,7 +22,7 @@ import java.util.Optional;
  */
 public class Maintainer {
     private static final CoreCollector coreCollector = new CoreCollector(new ProcessExecuter());
-    private static final CoredumpHandler coredumpHandler = new CoredumpHandler(HttpClientBuilder.create().build(), coreCollector);
+    private static final HttpClient httpClient = HttpClientBuilder.create().build();
 
     public static void main(String[] args) {
         if (args.length != 1) {
@@ -125,8 +126,8 @@ public class Maintainer {
         Map<String, Object> attributesMap = parseMap(arguments);
 
         try {
-            coredumpHandler.removeJavaCoredumps(coredumpsPath);
-            coredumpHandler.processAndReportCoredumps(coredumpsPath, doneCoredumpsPath, attributesMap);
+            CoredumpHandler coredumpHandler = new CoredumpHandler(httpClient, coreCollector, coredumpsPath, doneCoredumpsPath, attributesMap);
+            coredumpHandler.processAll();
         } catch (IOException e) {
             throw new RuntimeException("Failed processing coredumps at " + coredumpsPath.toAbsolutePath() +
                     ", moving fished dumps to " + doneCoredumpsPath.toAbsolutePath(), e);
diff --git a/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/maintainer/CoredumpHandlerTest.java b/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/maintainer/CoredumpHandlerTest.java
index 36c582dcd5f..6d55b915fbe 100644
--- a/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/maintainer/CoredumpHandlerTest.java
+++ b/node-maintainer/src/test/java/com/yahoo/vespa/hosted/node/maintainer/CoredumpHandlerTest.java
@@ -9,6 +9,7 @@ import org.apache.http.client.methods.HttpPost;
 import org.apache.http.entity.StringEntity;
 import org.apache.http.impl.DefaultHttpResponseFactory;
 import org.apache.http.message.BasicStatusLine;
+import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
@@ -61,17 +62,25 @@ public class CoredumpHandlerTest {
         metadata.put("backtrace", Arrays.asList("call 1", "function 2", "something something"));
     }
 
-    private final CoredumpHandler coredumpHandler = new CoredumpHandler(httpClient, coreCollector);
+    @Rule
+    public TemporaryFolder folder = new TemporaryFolder();
 
+    private CoredumpHandler coredumpHandler;
+    private Path crashPath;
+    private Path donePath;
 
-    @Rule
-    public TemporaryFolder folder= new TemporaryFolder();
+    @Before
+    public void setup() throws IOException {
+        crashPath = folder.newFolder("crash").toPath();
+        donePath = folder.newFolder("done").toPath();
+
+        coredumpHandler = new CoredumpHandler(httpClient, coreCollector, crashPath, donePath, attributes);
+    }
 
     @Test
     public void ignoresIncompleteCoredumps() throws IOException {
         Path coredumpPath = createCoredump(".core.dump");
-        Path crashPath = coredumpPath.getParent();
-        Path processingPath = coredumpHandler.processCoredumps(crashPath, attributes);
+        Path processingPath = coredumpHandler.processCoredumps();
 
         // The 'processing' directory should be empty
         assertFolderContents(processingPath);
@@ -83,7 +92,6 @@ public class CoredumpHandlerTest {
     @Test
     public void startProcessingTest() throws IOException {
         Path coredumpPath = createCoredump("core.dump");
-        Path crashPath = coredumpPath.getParent();
         Path processingPath = crashPath.resolve("processing_dir");
         coredumpHandler.startProcessing(coredumpPath, crashPath.resolve("processing_dir"));
 
@@ -101,9 +109,8 @@ public class CoredumpHandlerTest {
     @Test
     public void coredumpMetadataCollectAndWriteTest() throws IOException, InterruptedException {
         when(coreCollector.collect(any())).thenReturn(metadata);
-        Path coredumpPath = createCoredump("core.dump");
-        Path crashPath = coredumpPath.getParent();
-        Path processingPath = coredumpHandler.processCoredumps(crashPath, attributes);
+        createCoredump("core.dump");
+        Path processingPath = coredumpHandler.processCoredumps();
 
         // Inside 'processing' directory, there should be a new directory containing 'metadata.json' file
         List<Path> processedCoredumps = Files.list(processingPath).collect(Collectors.toList());
@@ -125,13 +132,10 @@ public class CoredumpHandlerTest {
     @Test
     public void reportFailCoredumpTest() throws IOException, URISyntaxException {
         final String documentId = "UIDD-ABCD-EFGH";
-
         Path metadataPath = createProcessedCoredump(documentId);
-        Path crashPath = metadataPath.getParent().getParent().getParent();
-        Path donePath = folder.newFolder("done").toPath();
 
         setNextHttpResponse(500, Optional.of("Internal server error"));
-        coredumpHandler.reportCoredumps(crashPath.resolve(CoredumpHandler.PROCESSING_DIRECTORY_NAME), donePath);
+        coredumpHandler.reportCoredumps(crashPath.resolve(CoredumpHandler.PROCESSING_DIRECTORY_NAME));
         validateNextHttpPost(documentId, expectedMetadataFileContents);
 
         // The coredump should not have been moved out of 'processing' and into 'done' as the report failed
@@ -144,10 +148,7 @@ public class CoredumpHandlerTest {
         final String documentId = "UIDD-ABCD-EFGH";
 
         Path coredumpPath = createProcessedCoredump(documentId);
-        Path crashPath = coredumpPath.getParent().getParent().getParent();
-        Path donePath = folder.newFolder("done").toPath();
-
-        coredumpHandler.finishProcessing(coredumpPath.getParent(), donePath);
+        coredumpHandler.finishProcessing(coredumpPath.getParent());
 
         // The coredump should've been moved out of 'processing' and into 'done'
         assertFolderContents(crashPath.resolve(CoredumpHandler.PROCESSING_DIRECTORY_NAME));
@@ -164,14 +165,12 @@ public class CoredumpHandlerTest {
     }
 
     private Path createCoredump(String coredumpName) throws IOException {
-        Path crashPath = folder.newFolder("crash").toPath();
         Path coredumpPath = crashPath.resolve(coredumpName);
         coredumpPath.toFile().createNewFile();
         return coredumpPath;
     }
 
     private Path createProcessedCoredump(String documentId) throws IOException {
-        Path crashPath = folder.newFolder("crash").toPath();
         Path coredumpPath = crashPath
                 .resolve(CoredumpHandler.PROCESSING_DIRECTORY_NAME)
                 .resolve(documentId)