Simplify CoredumpHandler constructor

2 files changed, 43 insertions, 26 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java
index 3de6427e20d..7cfc6a78944 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandler.java
@@ -2,17 +2,21 @@
 package com.yahoo.vespa.hosted.node.admin.maintenance.coredump;
 
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.yahoo.system.ProcessExecuter;
 import com.yahoo.vespa.hosted.node.admin.maintenance.FileHelper;
 import org.apache.http.HttpHeaders;
 import org.apache.http.HttpResponse;
 import org.apache.http.client.HttpClient;
+import org.apache.http.client.config.RequestConfig;
 import org.apache.http.client.methods.HttpPost;
 import org.apache.http.entity.StringEntity;
+import org.apache.http.impl.client.HttpClientBuilder;
 import org.apache.http.util.EntityUtils;
 
 import java.io.BufferedReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
+import java.net.URI;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.time.Duration;
@@ -40,28 +44,29 @@ 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;
-    private final String feedEndpoint;
+    private final URI feedEndpoint;
 
-    public CoredumpHandler(HttpClient httpClient, CoreCollector coreCollector, Path coredumpsPath, Path doneCoredumpsPath,
-                           Map<String, Object> nodeAttributes, String feedEndpoint) {
+    CoredumpHandler(HttpClient httpClient, CoreCollector coreCollector, Path doneCoredumpsPath,
+                    URI feedEndpoint) {
         this.httpClient = httpClient;
         this.coreCollector = coreCollector;
-        this.coredumpsPath = coredumpsPath;
         this.doneCoredumpsPath = doneCoredumpsPath;
-        this.nodeAttributes = nodeAttributes;
         this.feedEndpoint = feedEndpoint;
     }
 
-    public void processAll() throws IOException {
-        removeJavaCoredumps();
-        handleNewCoredumps();
+    public CoredumpHandler(Path doneCoredumpsPath, URI feedEndpoint) {
+        this(createHttpClient(Duration.ofSeconds(5)), new CoreCollector(new ProcessExecuter()),
+                doneCoredumpsPath, feedEndpoint);
+    }
+
+    public void processAll(Path coredumpsPath, Map<String, Object> nodeAttributes) throws IOException {
+        removeJavaCoredumps(coredumpsPath);
+        handleNewCoredumps(coredumpsPath, nodeAttributes);
         removeOldCoredumps();
     }
 
-    private void removeJavaCoredumps() throws IOException {
+    private void removeJavaCoredumps(Path coredumpsPath) throws IOException {
         if (! coredumpsPath.toFile().isDirectory()) return;
         FileHelper.deleteFiles(coredumpsPath, Duration.ZERO, Optional.of("^java_pid.*\\.hprof$"), false);
     }
@@ -71,9 +76,9 @@ public class CoredumpHandler {
         FileHelper.deleteDirectories(doneCoredumpsPath, Duration.ofDays(10), Optional.empty());
     }
 
-    private void handleNewCoredumps() {
-        Path processingCoredumps = enqueueCoredumps();
-        processAndReportCoredumps(processingCoredumps);
+    private void handleNewCoredumps(Path coredumpsPath, Map<String, Object> nodeAttributes) {
+        Path processingCoredumps = enqueueCoredumps(coredumpsPath);
+        processAndReportCoredumps(processingCoredumps, nodeAttributes);
     }
 
 
@@ -81,7 +86,7 @@ public class CoredumpHandler {
      * Moves a coredump to a new directory under the processing/ directory. Limit to only processing
      * one coredump at the time, starting with the oldest.
      */
-    Path enqueueCoredumps() {
+    Path enqueueCoredumps(Path coredumpsPath) {
         Path processingCoredumpsPath = coredumpsPath.resolve(PROCESSING_DIRECTORY_NAME);
         processingCoredumpsPath.toFile().mkdirs();
         if (!FileHelper.listContentsOfDirectory(processingCoredumpsPath).isEmpty()) return processingCoredumpsPath;
@@ -100,7 +105,7 @@ public class CoredumpHandler {
         return processingCoredumpsPath;
     }
 
-    void processAndReportCoredumps(Path processingCoredumpsPath) {
+    void processAndReportCoredumps(Path processingCoredumpsPath, Map<String, Object> nodeAttributes) {
         doneCoredumpsPath.toFile().mkdirs();
 
         FileHelper.listContentsOfDirectory(processingCoredumpsPath).stream()
@@ -168,4 +173,17 @@ public class CoredumpHandler {
         Files.move(coredumpDirectory, doneCoredumpsPath.resolve(coredumpDirectory.getFileName()));
     }
 
+    private static HttpClient createHttpClient(Duration timeout) {
+        int timeoutInMillis = (int) timeout.toMillis();
+        return HttpClientBuilder.create()
+                .setUserAgent("node-admin-core-dump-reporter")
+                .setDefaultRequestConfig(RequestConfig.custom()
+                        .setConnectTimeout(timeoutInMillis)
+                        .setConnectionRequestTimeout(timeoutInMillis)
+                        .setSocketTimeout(timeoutInMillis)
+                        .build())
+                .setMaxConnTotal(100)
+                .setMaxConnPerRoute(10)
+                .build();
+    }
 }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java
index 246ee0ad62b..b3cbdf56219 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpHandlerTest.java
@@ -45,8 +45,6 @@ import static org.mockito.Mockito.when;
  */
 public class CoredumpHandlerTest {
 
-    private final HttpClient httpClient = mock(HttpClient.class);
-    private final CoreCollector coreCollector = mock(CoreCollector.class);
     private static final Map<String, Object> attributes = new LinkedHashMap<>();
     private static final Map<String, Object> metadata = new LinkedHashMap<>();
     private static final String expectedMetadataFileContents = "{\"fields\":{" +
@@ -56,7 +54,7 @@ public class CoredumpHandlerTest {
             "\"vespa_version\":\"6.48.4\"," +
             "\"kernel_version\":\"2.6.32-573.22.1.el6.YAHOO.20160401.10.x86_64\"," +
             "\"docker_image\":\"vespa/ci:6.48.4\"}}";
-    private static final String feedEndpoint = "http://feed-endpoint.hostname.tld/feed";
+    private static final URI feedEndpoint = URI.create("http://feed-endpoint.hostname.tld/feed");
 
     static {
         attributes.put("hostname", "host123.yahoo.com");
@@ -71,6 +69,8 @@ public class CoredumpHandlerTest {
     @Rule
     public TemporaryFolder folder = new TemporaryFolder();
 
+    private final HttpClient httpClient = mock(HttpClient.class);
+    private final CoreCollector coreCollector = mock(CoreCollector.class);
     private CoredumpHandler coredumpHandler;
     private Path crashPath;
     private Path donePath;
@@ -80,14 +80,13 @@ public class CoredumpHandlerTest {
         crashPath = folder.newFolder("crash").toPath();
         donePath = folder.newFolder("done").toPath();
 
-        coredumpHandler = new CoredumpHandler(httpClient, coreCollector, crashPath, donePath, attributes,
-                feedEndpoint);
+        coredumpHandler = new CoredumpHandler(httpClient, coreCollector, donePath, feedEndpoint);
     }
 
     @Test
     public void ignoresIncompleteCoredumps() throws IOException {
         Path coredumpPath = createCoredump(".core.dump", Instant.now());
-        Path processingPath = coredumpHandler.enqueueCoredumps();
+        Path processingPath = coredumpHandler.enqueueCoredumps(crashPath);
 
         // The 'processing' directory should be empty
         assertFolderContents(processingPath);
@@ -120,7 +119,7 @@ public class CoredumpHandlerTest {
         createCoredump(oldestCoredump, startTime.minusSeconds(3600));
         createCoredump("core.dump1", startTime.minusSeconds(1000));
         createCoredump("core.dump2", startTime);
-        Path processingPath = coredumpHandler.enqueueCoredumps();
+        Path processingPath = coredumpHandler.enqueueCoredumps(crashPath);
 
         List<Path> processingCoredumps = Files.list(processingPath).collect(Collectors.toList());
         assertEquals(1, processingCoredumps.size());
@@ -132,7 +131,7 @@ public class CoredumpHandlerTest {
         assertEquals(Collections.singleton(oldestCoredump), filenamesInProcessingDirectory);
 
         // Running enqueueCoredumps should not start processing any new coredumps as we already are processing one
-        coredumpHandler.enqueueCoredumps();
+        coredumpHandler.enqueueCoredumps(crashPath);
         assertEquals(processingCoredumps, Files.list(processingPath).collect(Collectors.toList()));
         filenamesInProcessingDirectory = Files.list(processingCoredumps.get(0))
                 .map(file -> file.getFileName().toString())
@@ -143,7 +142,7 @@ public class CoredumpHandlerTest {
     @Test
     public void coredumpMetadataCollectAndWriteTest() throws IOException {
         createCoredump("core.dump", Instant.now());
-        Path processingPath = coredumpHandler.enqueueCoredumps();
+        Path processingPath = coredumpHandler.enqueueCoredumps(crashPath);
         Path processingCoredumpPath = Files.list(processingPath).findFirst().orElseThrow(() ->
                 new RuntimeException("Expected to find directory with coredump in processing dir"));
         when(coreCollector.collect(eq(processingCoredumpPath.resolve("core.dump")))).thenReturn(metadata);
@@ -182,7 +181,7 @@ public class CoredumpHandlerTest {
         Path metadataPath = createProcessedCoredump(documentId);
 
         setNextHttpResponse(500, Optional.of("Internal server error"));
-        coredumpHandler.processAndReportCoredumps(crashPath.resolve(CoredumpHandler.PROCESSING_DIRECTORY_NAME));
+        coredumpHandler.processAndReportCoredumps(crashPath.resolve(CoredumpHandler.PROCESSING_DIRECTORY_NAME), attributes);
         validateNextHttpPost(documentId, expectedMetadataFileContents);
 
         // The coredump should not have been moved out of 'processing' and into 'done' as the report failed