Create CoredumpReporter

4 files changed, 70 insertions, 144 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 e063425e4ae..1cdf97960ff 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
@@ -3,21 +3,9 @@ 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.maintainer.FileHelper;
-import org.apache.http.Header;
-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 com.yahoo.vespa.hosted.node.admin.maintenance.FileHelper;
+
 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;
@@ -26,10 +14,8 @@ import java.util.HashMap;
 import java.util.Map;
 import java.util.Optional;
 import java.util.UUID;
-import java.util.function.Supplier;
 import java.util.logging.Level;
 import java.util.logging.Logger;
-import java.util.stream.Collectors;
 
 /**
  * Finds coredumps, collects metadata and reports them
@@ -44,24 +30,18 @@ public class CoredumpHandler {
     private final Logger logger = Logger.getLogger(CoredumpHandler.class.getName());
     private final ObjectMapper objectMapper = new ObjectMapper();
 
-    private final HttpClient httpClient;
     private final CoreCollector coreCollector;
     private final Path doneCoredumpsPath;
-    private final URI feedEndpoint;
-    private final Optional<Supplier<Header>> requestHeaderSupplier;
+    private final CoredumpReporter coredumpReporter;
 
-    CoredumpHandler(HttpClient httpClient, CoreCollector coreCollector, Path doneCoredumpsPath,
-                    URI feedEndpoint, Optional<Supplier<Header>> requestHeaderSupplier) {
-        this.httpClient = httpClient;
-        this.coreCollector = coreCollector;
-        this.doneCoredumpsPath = doneCoredumpsPath;
-        this.feedEndpoint = feedEndpoint;
-        this.requestHeaderSupplier = requestHeaderSupplier;
+    public CoredumpHandler(CoredumpReporter coredumpReporter, Path doneCoredumpsPath) {
+        this(new CoreCollector(new ProcessExecuter()), coredumpReporter, doneCoredumpsPath);
     }
 
-    public CoredumpHandler(Path doneCoredumpsPath, URI feedEndpoint, Optional<Supplier<Header>> requestHeaderSupplier) {
-        this(createHttpClient(Duration.ofSeconds(5)), new CoreCollector(new ProcessExecuter()),
-                doneCoredumpsPath, feedEndpoint, requestHeaderSupplier);
+    CoredumpHandler(CoreCollector coreCollector, CoredumpReporter coredumpReporter, Path doneCoredumpsPath) {
+        this.coreCollector = coreCollector;
+        this.coredumpReporter = coredumpReporter;
+        this.doneCoredumpsPath = doneCoredumpsPath;
     }
 
     public void processAll(Path coredumpsPath, Map<String, Object> nodeAttributes) throws IOException {
@@ -81,8 +61,8 @@ public class CoredumpHandler {
     }
 
     private void handleNewCoredumps(Path coredumpsPath, Map<String, Object> nodeAttributes) {
-        Path processingCoredumps = enqueueCoredumps(coredumpsPath);
-        processAndReportCoredumps(processingCoredumps, nodeAttributes);
+        enqueueCoredumps(coredumpsPath);
+        processAndReportCoredumps(coredumpsPath, nodeAttributes);
     }
 
 
@@ -90,10 +70,10 @@ 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 coredumpsPath) {
-        Path processingCoredumpsPath = coredumpsPath.resolve(PROCESSING_DIRECTORY_NAME);
+    void enqueueCoredumps(Path coredumpsPath) {
+        Path processingCoredumpsPath = getProcessingCoredumpsPath(coredumpsPath);
         processingCoredumpsPath.toFile().mkdirs();
-        if (!FileHelper.listContentsOfDirectory(processingCoredumpsPath).isEmpty()) return processingCoredumpsPath;
+        if (!FileHelper.listContentsOfDirectory(processingCoredumpsPath).isEmpty()) return;
 
         FileHelper.listContentsOfDirectory(coredumpsPath).stream()
                 .filter(path -> path.toFile().isFile() && ! path.getFileName().toString().startsWith("."))
@@ -105,8 +85,6 @@ public class CoredumpHandler {
                         logger.log(Level.WARNING, "Failed to process coredump " + coredumpPath, e);
                     }
                 });
-
-        return processingCoredumpsPath;
     }
 
     void processAndReportCoredumps(Path processingCoredumpsPath, Map<String, Object> nodeAttributes) {
@@ -114,25 +92,29 @@ public class CoredumpHandler {
 
         FileHelper.listContentsOfDirectory(processingCoredumpsPath).stream()
                 .filter(path -> path.toFile().isDirectory())
-                .forEach(coredumpDirectory -> {
-                    try {
-                        String metadata = collectMetadata(coredumpDirectory, nodeAttributes);
-                        report(coredumpDirectory, metadata);
-                        finishProcessing(coredumpDirectory);
-                    } catch (Throwable e) {
-                        logger.log(Level.WARNING, "Failed to report coredump " + coredumpDirectory, e);
-                    }
-                });
+                .forEach(coredumpDirectory -> processAndReportSingleCoredump(coredumpDirectory, nodeAttributes));
     }
 
-    Path enqueueCoredumpForProcessing(Path coredumpPath, Path processingCoredumpsPath) throws IOException {
+    private void processAndReportSingleCoredump(Path coredumpDirectory, Map<String, Object> nodeAttributes) {
+        try {
+            String metadata = collectMetadata(coredumpDirectory, nodeAttributes);
+            String coredumpId = coredumpDirectory.getFileName().toString();
+            coredumpReporter.reportCoredump(coredumpId, metadata);
+            finishProcessing(coredumpDirectory);
+            logger.info("Successfully reported coredump " + coredumpId);
+        } catch (Throwable e) {
+            logger.log(Level.WARNING, "Failed to report coredump " + coredumpDirectory, e);
+        }
+    }
+
+    private void enqueueCoredumpForProcessing(Path coredumpPath, Path processingCoredumpsPath) throws IOException {
         // Make coredump readable
         coredumpPath.toFile().setReadable(true, false);
 
         // Create new directory for this coredump and move it into it
         Path folder = processingCoredumpsPath.resolve(UUID.randomUUID().toString());
         folder.toFile().mkdirs();
-        return Files.move(coredumpPath, folder.resolve(coredumpPath.getFileName()));
+        Files.move(coredumpPath, folder.resolve(coredumpPath.getFileName()));
     }
 
     String collectMetadata(Path coredumpDirectory, Map<String, Object> nodeAttributes) throws IOException {
@@ -154,41 +136,14 @@ public class CoredumpHandler {
         }
     }
 
-    void report(Path coredumpDirectory, String metadata) throws IOException {
-        // Use core dump UUID as document ID
-        String documentId = coredumpDirectory.getFileName().toString();
-
-        HttpPost post = new HttpPost(feedEndpoint + "/" + documentId);
-        post.setHeader(HttpHeaders.CONTENT_TYPE, "application/json");
-        post.setEntity(new StringEntity(metadata));
-        requestHeaderSupplier.map(Supplier::get).ifPresent(post::addHeader);
-
-        HttpResponse response = httpClient.execute(post);
-        if (response.getStatusLine().getStatusCode() / 100 != 2) {
-            String result = new BufferedReader(new InputStreamReader(response.getEntity().getContent()))
-                    .lines().collect(Collectors.joining("\n"));
-            throw new RuntimeException("POST to " + post.getURI() + " failed with HTTP: " +
-                    response.getStatusLine().getStatusCode() + " [" + result + "]");
-        }
-        EntityUtils.consume(response.getEntity());
-        logger.info("Successfully reported coredump " + documentId);
-    }
-
-    void finishProcessing(Path coredumpDirectory) throws IOException {
+    private void finishProcessing(Path coredumpDirectory) throws IOException {
         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();
+    /**
+     * @return Path to directory where coredumps are temporarily moved while still being processed
+     */
+    static Path getProcessingCoredumpsPath(Path coredumpsPath) {
+        return coredumpsPath.resolve(PROCESSING_DIRECTORY_NAME);
     }
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpReporter.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpReporter.java
new file mode 100644
index 00000000000..5babfc26acb
--- /dev/null
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpReporter.java
@@ -0,0 +1,10 @@
+package com.yahoo.vespa.hosted.node.admin.maintenance.coredump;
+
+/**
+ * @author valerijf
+ */
+public interface CoredumpReporter {
+
+    /** Report a coredump with a given ID and given metadata */
+    void reportCoredump(String id, String metadata);
+}
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/package-info.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/package-info.java
new file mode 100644
index 00000000000..0184ff1189f
--- /dev/null
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/package-info.java
@@ -0,0 +1,5 @@
+// Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+@ExportPackage
+package com.yahoo.vespa.hosted.node.admin.maintenance.coredump;
+
+import com.yahoo.osgi.annotation.ExportPackage;
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 54787108e64..d61c1e4b815 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
@@ -1,25 +1,12 @@
 // Copyright 2018 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
 package com.yahoo.vespa.hosted.node.admin.maintenance.coredump;
 
-import org.apache.http.HttpHeaders;
-import org.apache.http.HttpResponse;
-import org.apache.http.HttpVersion;
-import org.apache.http.client.HttpClient;
-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;
-import org.mockito.ArgumentCaptor;
 
-import java.io.BufferedReader;
 import java.io.IOException;
-import java.io.InputStreamReader;
-import java.net.URI;
-import java.net.URISyntaxException;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.time.Instant;
@@ -28,13 +15,13 @@ import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
 import static org.junit.Assert.assertEquals;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.eq;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyZeroInteractions;
@@ -54,7 +41,6 @@ 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 URI feedEndpoint = URI.create("http://feed-endpoint.hostname.tld/feed");
 
     static {
         attributes.put("hostname", "host123.yahoo.com");
@@ -69,24 +55,26 @@ public class CoredumpHandlerTest {
     @Rule
     public TemporaryFolder folder = new TemporaryFolder();
 
-    private final HttpClient httpClient = mock(HttpClient.class);
     private final CoreCollector coreCollector = mock(CoreCollector.class);
+    private final CoredumpReporter coredumpReporter = mock(CoredumpReporter.class);
     private CoredumpHandler coredumpHandler;
     private Path crashPath;
     private Path donePath;
+    private Path processingPath;
 
     @Before
     public void setup() throws IOException {
         crashPath = folder.newFolder("crash").toPath();
         donePath = folder.newFolder("done").toPath();
+        processingPath = CoredumpHandler.getProcessingCoredumpsPath(crashPath);
 
-        coredumpHandler = new CoredumpHandler(httpClient, coreCollector, donePath, feedEndpoint, Optional.empty());
+        coredumpHandler = new CoredumpHandler(coreCollector, coredumpReporter, donePath);
     }
 
     @Test
     public void ignoresIncompleteCoredumps() throws IOException {
         Path coredumpPath = createCoredump(".core.dump", Instant.now());
-        Path processingPath = coredumpHandler.enqueueCoredumps(crashPath);
+        coredumpHandler.enqueueCoredumps(crashPath);
 
         // The 'processing' directory should be empty
         assertFolderContents(processingPath);
@@ -98,8 +86,7 @@ public class CoredumpHandlerTest {
     @Test
     public void startProcessingTest() throws IOException {
         Path coredumpPath = createCoredump("core.dump", Instant.now());
-        Path processingPath = crashPath.resolve("processing_dir");
-        coredumpHandler.enqueueCoredumpForProcessing(coredumpPath, processingPath);
+        coredumpHandler.enqueueCoredumps(crashPath);
 
         // Contents of 'crash' should be only the 'processing' directory
         assertFolderContents(crashPath, processingPath.getFileName().toString());
@@ -119,7 +106,7 @@ public class CoredumpHandlerTest {
         createCoredump(oldestCoredump, startTime.minusSeconds(3600));
         createCoredump("core.dump1", startTime.minusSeconds(1000));
         createCoredump("core.dump2", startTime);
-        Path processingPath = coredumpHandler.enqueueCoredumps(crashPath);
+        coredumpHandler.enqueueCoredumps(crashPath);
 
         List<Path> processingCoredumps = Files.list(processingPath).collect(Collectors.toList());
         assertEquals(1, processingCoredumps.size());
@@ -142,7 +129,7 @@ public class CoredumpHandlerTest {
     @Test
     public void coredumpMetadataCollectAndWriteTest() throws IOException {
         createCoredump("core.dump", Instant.now());
-        Path processingPath = coredumpHandler.enqueueCoredumps(crashPath);
+        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);
@@ -166,42 +153,32 @@ public class CoredumpHandlerTest {
     }
 
     @Test
-    public void reportSuccessCoredumpTest() throws IOException, URISyntaxException {
+    public void reportSuccessCoredumpTest() throws IOException {
         final String documentId = "UIDD-ABCD-EFGH";
-        Path coredumpPath = createProcessedCoredump(documentId);
+        createProcessedCoredump(documentId);
 
-        setNextHttpResponse(200, Optional.empty());
-        coredumpHandler.report(coredumpPath.getParent(), expectedMetadataFileContents);
-        validateNextHttpPost(documentId, expectedMetadataFileContents);
+        coredumpHandler.processAndReportCoredumps(crashPath.resolve(CoredumpHandler.PROCESSING_DIRECTORY_NAME), attributes);
+        verify(coredumpReporter).reportCoredump(eq(documentId), eq(expectedMetadataFileContents));
+
+        // The coredump should not have been moved out of 'processing' and into 'done' as the report failed
+        assertFolderContents(crashPath.resolve(CoredumpHandler.PROCESSING_DIRECTORY_NAME));
+        assertFolderContents(donePath.resolve(documentId), CoredumpHandler.METADATA_FILE_NAME);
     }
 
     @Test
-    public void reportFailCoredumpTest() throws IOException, URISyntaxException {
+    public void reportFailCoredumpTest() throws IOException {
         final String documentId = "UIDD-ABCD-EFGH";
         Path metadataPath = createProcessedCoredump(documentId);
 
-        setNextHttpResponse(500, Optional.of("Internal server error"));
+        doThrow(new RuntimeException()).when(coredumpReporter).reportCoredump(any(), any());
         coredumpHandler.processAndReportCoredumps(crashPath.resolve(CoredumpHandler.PROCESSING_DIRECTORY_NAME), attributes);
-        validateNextHttpPost(documentId, expectedMetadataFileContents);
+        verify(coredumpReporter).reportCoredump(eq(documentId), eq(expectedMetadataFileContents));
 
         // The coredump should not have been moved out of 'processing' and into 'done' as the report failed
         assertFolderContents(donePath);
         assertFolderContents(metadataPath.getParent(), CoredumpHandler.METADATA_FILE_NAME);
     }
 
-    @Test
-    public void finishProcessingTest() throws IOException {
-        final String documentId = "UIDD-ABCD-EFGH";
-
-        Path coredumpPath = createProcessedCoredump(documentId);
-        coredumpHandler.finishProcessing(coredumpPath.getParent());
-
-        // The coredump should've been moved out of 'processing' and into 'done'
-        assertFolderContents(crashPath.resolve(CoredumpHandler.PROCESSING_DIRECTORY_NAME));
-        assertFolderContents(donePath.resolve(documentId), CoredumpHandler.METADATA_FILE_NAME);
-    }
-
-
     private static void assertFolderContents(Path pathToFolder, String... filenames) throws IOException {
         Set<Path> expectedContentsOfFolder = Arrays.stream(filenames)
                 .map(pathToFolder::resolve)
@@ -225,25 +202,4 @@ public class CoredumpHandlerTest {
         coredumpPath.getParent().toFile().mkdirs();
         return Files.write(coredumpPath, expectedMetadataFileContents.getBytes());
     }
-
-    private void setNextHttpResponse(int code, Optional<String> message) throws IOException {
-        DefaultHttpResponseFactory responseFactory = new DefaultHttpResponseFactory();
-        HttpResponse httpResponse = responseFactory.newHttpResponse(
-                new BasicStatusLine(HttpVersion.HTTP_1_1, code, null), null);
-        if (message.isPresent()) httpResponse.setEntity(new StringEntity(message.get()));
-
-        when(httpClient.execute(any())).thenReturn(httpResponse);
-    }
-
-    private void validateNextHttpPost(String documentId, String expectedBody) throws IOException, URISyntaxException {
-        ArgumentCaptor<HttpPost> capturedPost = ArgumentCaptor.forClass(HttpPost.class);
-        verify(httpClient).execute(capturedPost.capture());
-
-        URI expectedURI = new URI(feedEndpoint + "/" + documentId);
-        assertEquals(expectedURI, capturedPost.getValue().getURI());
-        assertEquals("application/json", capturedPost.getValue().getHeaders(HttpHeaders.CONTENT_TYPE)[0].getValue());
-        assertEquals(expectedBody,
-                new BufferedReader(new InputStreamReader(capturedPost.getValue().getEntity().getContent())).readLine());
-    }
-
 }