diff options
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()); - } - } |