diff options
author | Håkon Hallingstad <hakon@yahooinc.com> | 2022-12-01 16:26:19 +0100 |
---|---|---|
committer | Håkon Hallingstad <hakon@yahooinc.com> | 2022-12-01 16:26:19 +0100 |
commit | 90b483b03b0681d60fccfa7edcddb3aa62dff555 (patch) | |
tree | 3ce2faca7306ed299bdc61fe5f23dd18b5451673 /node-admin | |
parent | c04788cb28e3b72e19b4ad11c87031ff85f17681 (diff) |
Stop using report-cores-via-cfg
Diffstat (limited to 'node-admin')
5 files changed, 67 insertions, 233 deletions
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollector.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollector.java index d99b45dbbbc..cfb2d240bfa 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollector.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollector.java @@ -10,9 +10,7 @@ import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult; import java.nio.file.Path; import java.util.Arrays; -import java.util.HashMap; import java.util.List; -import java.util.Map; import java.util.logging.Level; import java.util.logging.Logger; import java.util.regex.Matcher; @@ -32,9 +30,6 @@ public class CoreCollector { private static final Pattern FROM_PATH_PATTERN = Pattern.compile("^.* from '(?<path>.*?)'"); static final String GDB_PATH_RHEL8 = "/opt/rh/gcc-toolset-11/root/bin/gdb"; - static final Map<String, Object> JAVA_HEAP_DUMP_METADATA = - Map.of("bin_path", "java", "backtrace", List.of("Heap dump, no backtrace available")); - private final ContainerOperations container; public CoreCollector(ContainerOperations container) { @@ -100,36 +95,7 @@ public class CoreCollector { return List.of(result.getOutput().split("\n")); } - /** - * Collects metadata about a given core dump - * @param context context of the NodeAgent that owns the core dump - * @param coredumpPath path to core dump file inside the container - * @return map of relevant metadata about the core dump - */ - Map<String, Object> collect(NodeAgentContext context, ContainerPath coredumpPath) { - if (JAVA_HEAP_DUMP_PATTERN.matcher(coredumpPath.getFileName().toString()).find()) - return JAVA_HEAP_DUMP_METADATA; - - Map<String, Object> data = new HashMap<>(); - try { - String binPath = readBinPath(context, coredumpPath); - - data.put("bin_path", binPath); - if (Path.of(binPath).getFileName().toString().equals("java")) { - data.put("backtrace_all_threads", readJstack(context, coredumpPath, binPath)); - } else { - data.put("backtrace", readBacktrace(context, coredumpPath, binPath, false)); - data.put("backtrace_all_threads", readBacktrace(context, coredumpPath, binPath, true)); - } - } catch (ConvergenceException e) { - context.log(logger, Level.WARNING, "Failed to extract backtrace: " + e.getMessage()); - } catch (RuntimeException e) { - context.log(logger, Level.WARNING, "Failed to extract backtrace", e); - } - return data; - } - - CoreDumpMetadata collect2(NodeAgentContext context, ContainerPath coredumpPath) { + CoreDumpMetadata collect(NodeAgentContext context, ContainerPath coredumpPath) { var metadata = new CoreDumpMetadata(); if (JAVA_HEAP_DUMP_PATTERN.matcher(coredumpPath.getFileName().toString()).find()) { 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 4a8353d92ec..94f402d5332 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 @@ -1,14 +1,10 @@ // Copyright Yahoo. 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 com.fasterxml.jackson.core.JsonProcessingException; -import com.fasterxml.jackson.databind.ObjectMapper; -import com.fasterxml.jackson.databind.node.ObjectNode; import com.yahoo.config.provision.DockerImage; import com.yahoo.security.KeyId; import com.yahoo.security.SecretSharedKey; import com.yahoo.security.SharedKeyGenerator; -import com.yahoo.vespa.flags.BooleanFlag; import com.yahoo.vespa.flags.FetchVector; import com.yahoo.vespa.flags.FlagSource; import com.yahoo.vespa.flags.Flags; @@ -26,19 +22,16 @@ import com.yahoo.vespa.hosted.node.admin.task.util.file.FileDeleter; import com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder; import com.yahoo.vespa.hosted.node.admin.task.util.file.FileMover; import com.yahoo.vespa.hosted.node.admin.task.util.file.MakeDirectory; -import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; import java.io.IOException; import java.io.OutputStream; import java.io.UncheckedIOException; -import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; import java.time.Clock; import java.util.Comparator; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -61,64 +54,49 @@ import static com.yahoo.yolean.Exceptions.uncheck; */ public class CoredumpHandler { + public static final String COREDUMP_FILENAME_PREFIX = "dump_"; + + private static final Logger logger = Logger.getLogger(CoredumpHandler.class.getName()); private static final Pattern HS_ERR_PATTERN = Pattern.compile("hs_err_pid[0-9]+\\.log"); private static final String PROCESSING_DIRECTORY_NAME = "processing"; - private static final String METADATA_FILE_NAME = "metadata.json"; private static final String METADATA2_FILE_NAME = "metadata2.json"; private static final String COMPRESSED_EXTENSION = ".zst"; private static final String ENCRYPTED_EXTENSION = ".enc"; - public static final String COREDUMP_FILENAME_PREFIX = "dump_"; - - private final Logger logger = Logger.getLogger(CoredumpHandler.class.getName()); - private final ObjectMapper objectMapper = new ObjectMapper(); private final CoreCollector coreCollector; private final Cores cores; - private final CoredumpReporter coredumpReporter; private final String crashPatchInContainer; private final Path doneCoredumpsPath; private final Metrics metrics; private final Clock clock; private final Supplier<String> coredumpIdSupplier; private final SecretSharedKeySupplier secretSharedKeySupplier; - private final BooleanFlag reportCoresViaCfgFlag; private final StringFlag coreEncryptionPublicKeyIdFlag; /** * @param crashPathInContainer path inside the container where core dump are dumped * @param doneCoredumpsPath path on host where processed core dumps are stored */ - public CoredumpHandler(CoreCollector coreCollector, Cores cores, CoredumpReporter coredumpReporter, - String crashPathInContainer, Path doneCoredumpsPath, Metrics metrics, - FlagSource flagSource) { - this(coreCollector, cores, coredumpReporter, crashPathInContainer, doneCoredumpsPath, - metrics, Clock.systemUTC(), () -> UUID.randomUUID().toString(), (ctx) -> Optional.empty() /*TODO*/, - flagSource); - } - - // TODO remove redundant constructor once internal callsite has been updated - public CoredumpHandler(CoreCollector coreCollector, Cores cores, CoredumpReporter coredumpReporter, + public CoredumpHandler(CoreCollector coreCollector, Cores cores, String crashPathInContainer, Path doneCoredumpsPath, Metrics metrics, SecretSharedKeySupplier secretSharedKeySupplier, FlagSource flagSource) { - this(coreCollector, cores, coredumpReporter, crashPathInContainer, doneCoredumpsPath, + this(coreCollector, cores, crashPathInContainer, doneCoredumpsPath, metrics, Clock.systemUTC(), () -> UUID.randomUUID().toString(), secretSharedKeySupplier, flagSource); } - CoredumpHandler(CoreCollector coreCollector, Cores cores, CoredumpReporter coredumpReporter, + CoredumpHandler(CoreCollector coreCollector, Cores cores, String crashPathInContainer, Path doneCoredumpsPath, Metrics metrics, Clock clock, Supplier<String> coredumpIdSupplier, SecretSharedKeySupplier secretSharedKeySupplier, FlagSource flagSource) { this.coreCollector = coreCollector; this.cores = cores; - this.coredumpReporter = coredumpReporter; this.crashPatchInContainer = crashPathInContainer; this.doneCoredumpsPath = doneCoredumpsPath; this.metrics = metrics; this.clock = clock; this.coredumpIdSupplier = coredumpIdSupplier; this.secretSharedKeySupplier = secretSharedKeySupplier; - this.reportCoresViaCfgFlag = Flags.REPORT_CORES_VIA_CFG.bindTo(flagSource); this.coreEncryptionPublicKeyIdFlag = Flags.CORE_ENCRYPTION_PUBLIC_KEY_ID.bindTo(flagSource); } @@ -143,13 +121,7 @@ public class CoredumpHandler { // Check if we have already started to process a core dump or we can enqueue a new core one getCoredumpToProcess(context, containerCrashPath, containerProcessingPath) - .ifPresent(path -> { - if (reportCoresViaCfgFlag.with(FetchVector.Dimension.NODE_TYPE, context.nodeType().name()).value()) { - processAndReportSingleCoreDump2(context, path, dockerImage); - } else { - processAndReportSingleCoredump(context, path, nodeAttributesSupplier); - } - }); + .ifPresent(path -> processAndReportSingleCoreDump(context, path, dockerImage)); } /** @return path to directory inside processing directory that contains a core dump file to process */ @@ -212,65 +184,6 @@ public class CoredumpHandler { return coreEncryptionPublicKeyIdFlag.with(FetchVector.Dimension.NODE_TYPE, context.nodeType().name()).value(); } - void processAndReportSingleCoredump(NodeAgentContext context, ContainerPath coredumpDirectory, Supplier<Map<String, Object>> nodeAttributesSupplier) { - try { - Optional<SecretSharedKey> sharedCoreKey = Optional.of(corePublicKeyFlagValue(context)) - .filter(k -> !k.isEmpty()) - .map(KeyId::ofString) - .flatMap(secretSharedKeySupplier::create); - Optional<String> decryptionToken = sharedCoreKey.map(k -> k.sealedSharedKey().toTokenString()); - String metadata = getMetadata(context, coredumpDirectory, nodeAttributesSupplier, decryptionToken); - String coredumpId = coredumpDirectory.getFileName().toString(); - coredumpReporter.reportCoredump(coredumpId, metadata); - finishProcessing(context, coredumpDirectory, sharedCoreKey); - context.log(logger, "Successfully reported coredump " + coredumpId); - } catch (Exception e) { - throw new RuntimeException("Failed to process coredump " + coredumpDirectory, e); - } - } - - /** - * @return coredump metadata from metadata.json if present, otherwise attempts to get metadata using - * {@link CoreCollector} and stores it to metadata.json - */ - String getMetadata(NodeAgentContext context, ContainerPath coredumpDirectory, Supplier<Map<String, Object>> nodeAttributesSupplier, Optional<String> decryptionToken) throws IOException { - UnixPath metadataPath = new UnixPath(coredumpDirectory.resolve(METADATA_FILE_NAME)); - if (!metadataPath.exists()) { - ContainerPath coredumpFile = findCoredumpFileInProcessingDirectory(coredumpDirectory); - Map<String, Object> metadata = new HashMap<>(coreCollector.collect(context, coredumpFile)); - metadata.putAll(nodeAttributesSupplier.get()); - metadata.put("coredump_path", doneCoredumpsPath - .resolve(context.containerName().asString()) - .resolve(coredumpDirectory.getFileName().toString()) - .resolve(coredumpFile.getFileName().toString()).toString()); - decryptionToken.ifPresent(token -> metadata.put("decryption_token", token)); - - String metadataFields = objectMapper.writeValueAsString(Map.of("fields", metadata)); - metadataPath.writeUtf8File(metadataFields); - return metadataFields; - } else { - if (decryptionToken.isPresent()) { - // Since encryption keys are single-use and generated for each core dump processing invocation, - // we must ensure we store and report the token associated with the _latest_ (i.e. current) - // attempt at processing the core dump. Patch and rewrite the file with a new token, if present. - String metadataFields = metadataWithPatchedTokenValue(metadataPath, decryptionToken.get()); - metadataPath.deleteIfExists(); - metadataPath.writeUtf8File(metadataFields); - return metadataFields; - } else { - return metadataPath.readUtf8File(); - } - } - } - - private String metadataWithPatchedTokenValue(UnixPath metadataPath, String decryptionToken) throws JsonProcessingException { - var jsonRoot = objectMapper.readTree(metadataPath.readUtf8File()); - if (jsonRoot.path("fields").isObject()) { - ((ObjectNode)jsonRoot.get("fields")).put("decryption_token", decryptionToken); - } // else: unit testing case without real metadata - return objectMapper.writeValueAsString(jsonRoot); - } - static OutputStream maybeWrapWithEncryption(OutputStream wrappedStream, Optional<SecretSharedKey> sharedCoreKey) { return sharedCoreKey .map(key -> SharedKeyGenerator.makeAesGcmEncryptionCipher(key).wrapOutputStream(wrappedStream)) @@ -372,8 +285,8 @@ public class CoredumpHandler { return clock.instant().minusSeconds(60).isAfter(fileAttributes.lastModifiedTime()); } - void processAndReportSingleCoreDump2(NodeAgentContext context, ContainerPath coreDumpDirectory, - Optional<DockerImage> dockerImage) { + void processAndReportSingleCoreDump(NodeAgentContext context, ContainerPath coreDumpDirectory, + Optional<DockerImage> dockerImage) { CoreDumpMetadata metadata = gatherMetadata(context, coreDumpDirectory); dockerImage.ifPresent(metadata::setDockerImage); dockerImage.flatMap(DockerImage::tag).ifPresent(metadata::setVespaVersion); @@ -390,20 +303,17 @@ public class CoredumpHandler { finishProcessing(context, coreDumpDirectory, sharedCoreKey); } - private CoreDumpMetadata gatherMetadata(NodeAgentContext context, ContainerPath coreDumpDirectory) { + CoreDumpMetadata gatherMetadata(NodeAgentContext context, ContainerPath coreDumpDirectory) { ContainerPath metadataPath = coreDumpDirectory.resolve(METADATA2_FILE_NAME); Optional<ReportCoreDumpRequest> request = ReportCoreDumpRequest.load(metadataPath); if (request.isPresent()) { - return request.map(requestInstance -> { - var metadata = new CoreDumpMetadata(); - requestInstance.populateMetadata(metadata, FileSystems.getDefault()); - return metadata; - }) - .get(); + var metadata = new CoreDumpMetadata(); + request.get().populateMetadata(metadata, doneCoredumpsPath.getFileSystem()); + return metadata; } ContainerPath coreDumpFile = findCoredumpFileInProcessingDirectory(coreDumpDirectory); - CoreDumpMetadata metadata = coreCollector.collect2(context, coreDumpFile); + CoreDumpMetadata metadata = coreCollector.collect(context, coreDumpFile); metadata.setCpuMicrocodeVersion(getMicrocodeVersion()) .setKernelVersion(System.getProperty("os.version")) .setCoreDumpPath(doneCoredumpsPath.resolve(context.containerName().asString()) 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 deleted file mode 100644 index 0e2bb724f50..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoredumpReporter.java +++ /dev/null @@ -1,11 +0,0 @@ -// Copyright Yahoo. 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; - -/** - * @author freva - */ -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/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java index ea73fd4af6f..8f251bf72c5 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/coredump/CoreCollectorTest.java @@ -1,6 +1,7 @@ // Copyright Yahoo. 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 com.yahoo.vespa.hosted.node.admin.configserver.cores.CoreDumpMetadata; import com.yahoo.vespa.hosted.node.admin.container.ContainerOperations; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl; @@ -10,10 +11,8 @@ import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.jupiter.api.Test; import java.util.List; -import java.util.Map; import static com.yahoo.vespa.hosted.node.admin.maintenance.coredump.CoreCollector.GDB_PATH_RHEL8; -import static com.yahoo.vespa.hosted.node.admin.maintenance.coredump.CoreCollector.JAVA_HEAP_DUMP_METADATA; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.fail; import static org.mockito.Mockito.mock; @@ -23,7 +22,6 @@ import static org.mockito.Mockito.when; * @author freva */ public class CoreCollectorTest { - private final String JDK_PATH = "/path/to/jdk/java"; private final ContainerOperations docker = mock(ContainerOperations.class); private final CoreCollector coreCollector = new CoreCollector(docker); private final NodeAgentContext context = NodeAgentContextImpl.builder("container-123.domain.tld") @@ -144,11 +142,10 @@ public class CoreCollectorTest { "/usr/bin/program", "/tmp/core.1234"}, String.join("\n", GDB_BACKTRACE)); - Map<String, Object> expectedData = Map.of( - "bin_path", TEST_BIN_PATH, - "backtrace", GDB_BACKTRACE, - "backtrace_all_threads", GDB_BACKTRACE); - assertEquals(expectedData, coreCollector.collect(context, TEST_CORE_PATH)); + var expected = new CoreDumpMetadata().setBinPath(TEST_BIN_PATH) + .setBacktrace(GDB_BACKTRACE) + .setBacktraceAllThreads(GDB_BACKTRACE); + assertEquals(expected, coreCollector.collect(context, TEST_CORE_PATH)); } @Test @@ -159,8 +156,8 @@ public class CoreCollectorTest { mockExec(new String[]{GDB_PATH_RHEL8 + " -n -ex set print frame-arguments none -ex bt -batch /usr/bin/program /tmp/core.1234"}, "", "Failure"); - Map<String, Object> expectedData = Map.of("bin_path", TEST_BIN_PATH); - assertEquals(expectedData, coreCollector.collect(context, TEST_CORE_PATH)); + var expected = new CoreDumpMetadata().setBinPath(TEST_BIN_PATH); + assertEquals(expected, coreCollector.collect(context, TEST_CORE_PATH)); } @Test @@ -168,22 +165,25 @@ public class CoreCollectorTest { mockExec(new String[]{"file", TEST_CORE_PATH.pathInContainer()}, "dump.core.5954: ELF 64-bit LSB core file x86-64, version 1 (SYSV), too many program header sections (33172)"); + String jdkPath = "/path/to/jdk/java"; mockExec(new String[]{GDB_PATH_RHEL8, "-n", "-batch", "-core", "/tmp/core.1234"}, - "Core was generated by `" + JDK_PATH + " -Dconfig.id=default/container.11 -XX:+Pre'."); + "Core was generated by `" + jdkPath + " -Dconfig.id=default/container.11 -XX:+Pre'."); String jstack = "jstack11"; - mockExec(new String[]{"jhsdb", "jstack", "--exe", JDK_PATH, "--core", "/tmp/core.1234"}, + mockExec(new String[]{"jhsdb", "jstack", "--exe", jdkPath, "--core", "/tmp/core.1234"}, jstack); - Map<String, Object> expectedData = Map.of( - "bin_path", JDK_PATH, - "backtrace_all_threads", List.of(jstack)); - assertEquals(expectedData, coreCollector.collect(context, TEST_CORE_PATH)); + var expected = new CoreDumpMetadata().setBinPath(jdkPath) + .setBacktraceAllThreads(List.of(jstack)); + assertEquals(expected, coreCollector.collect(context, TEST_CORE_PATH)); } @Test void metadata_for_java_heap_dump() { - assertEquals(JAVA_HEAP_DUMP_METADATA, coreCollector.collect(context, context.paths().of("/dump_java_pid123.hprof"))); + var expected = new CoreDumpMetadata().setBinPath("java") + .setBacktrace(List.of("Heap dump, no backtrace available")); + + assertEquals(expected, coreCollector.collect(context, context.paths().of("/dump_java_pid123.hprof"))); } private void mockExec(String[] cmd, String output) { 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 b096db2eaf4..1fd688558a0 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,12 +1,14 @@ // Copyright Yahoo. 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 com.yahoo.config.provision.DockerImage; import com.yahoo.security.KeyId; import com.yahoo.security.SealedSharedKey; import com.yahoo.security.SecretSharedKey; import com.yahoo.test.ManualClock; import com.yahoo.vespa.flags.Flags; import com.yahoo.vespa.flags.InMemoryFlagSource; +import com.yahoo.vespa.hosted.node.admin.configserver.cores.CoreDumpMetadata; import com.yahoo.vespa.hosted.node.admin.configserver.cores.Cores; import com.yahoo.vespa.hosted.node.admin.container.metrics.DimensionMetrics; import com.yahoo.vespa.hosted.node.admin.container.metrics.Metrics; @@ -26,7 +28,6 @@ import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.attribute.FileTime; import java.time.Duration; -import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; @@ -36,9 +37,12 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import static com.yahoo.yolean.Exceptions.uncheck; -import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.never; import static org.mockito.Mockito.times; @@ -57,7 +61,6 @@ public class CoredumpHandlerTest { private final CoreCollector coreCollector = mock(CoreCollector.class); private final Cores cores = mock(Cores.class); - private final CoredumpReporter coredumpReporter = mock(CoredumpReporter.class); private final Metrics metrics = new Metrics(); private final ManualClock clock = new ManualClock(); @SuppressWarnings("unchecked") @@ -65,7 +68,7 @@ public class CoredumpHandlerTest { private final SecretSharedKeySupplier secretSharedKeySupplier = mock(SecretSharedKeySupplier.class); private final InMemoryFlagSource flagSource = new InMemoryFlagSource(); private final CoredumpHandler coredumpHandler = - new CoredumpHandler(coreCollector, cores, coredumpReporter, containerCrashPath.pathInContainer(), + new CoredumpHandler(coreCollector, cores, containerCrashPath.pathInContainer(), doneCoredumpsPath, metrics, clock, coredumpIdSupplier, secretSharedKeySupplier, flagSource); @@ -148,31 +151,14 @@ public class CoredumpHandlerTest { verify(coredumpIdSupplier, times(1)).get(); } - private static String buildExpectedMetadataString(Optional<String> decryptionToken) { - return "{\"fields\":{" + - "\"hostname\":\"host123.yahoo.com\"," + - "\"kernel_version\":\"3.10.0-862.9.1.el7.x86_64\"," + - "\"backtrace\":[\"call 1\",\"function 2\",\"something something\"]," + - "\"vespa_version\":\"6.48.4\"," + - "\"bin_path\":\"/bin/bash\"," + - "\"coredump_path\":\"/home/docker/dumps/container-123/id-123/dump_core.456\"," + - "\"docker_image\":\"vespa/ci:6.48.4\"" + - decryptionToken.map(",\"decryption_token\":\"%s\""::formatted).orElse("") + - "}}"; - } - - void do_get_metadata_test(Optional<String> oldDecryptionToken, Optional<String> newDecryptionToken) throws IOException { - Map<String, Object> metadata = new HashMap<>(); - metadata.put("bin_path", "/bin/bash"); - metadata.put("backtrace", List.of("call 1", "function 2", "something something")); - - Map<String, Object> attributes = Map.of( - "hostname", "host123.yahoo.com", - "vespa_version", "6.48.4", - "kernel_version", "3.10.0-862.9.1.el7.x86_64", - "docker_image", "vespa/ci:6.48.4"); - - String expectedMetadataStr = buildExpectedMetadataString(oldDecryptionToken); + @Test + void gather_metadata_test() throws IOException { + var metadata = new CoreDumpMetadata().setKernelVersion("3.10.0-862.9.1.el7.x86_64") + .setBacktrace(List.of("call 1", "function 2", "something something")) + .setVespaVersion("6.48.4") + .setBinPath("/bin/bash") + .setCoreDumpPath(context.paths().of("/home/docker/dumps/container-123/id-123/dump_core.456")) + .setDockerImage(DockerImage.fromString("example.com/vespa/ci:6.48.4")); ContainerPath coredumpDirectory = context.paths().of("/var/crash/id-123"); Files.createDirectories(coredumpDirectory.pathOnHost()); @@ -180,40 +166,20 @@ public class CoredumpHandlerTest { when(coreCollector.collect(eq(context), eq(coredumpDirectory.resolve("dump_core.456")))) .thenReturn(metadata); - assertEquals(expectedMetadataStr, coredumpHandler.getMetadata(context, coredumpDirectory, () -> attributes, oldDecryptionToken)); + assertEquals(metadata, coredumpHandler.gatherMetadata(context, coredumpDirectory)); verify(coreCollector, times(1)).collect(any(), any()); - // Calling it again will read the previously generated metadata from disk and selectively - // patch in an updated decryption token value, if one is provided. - // This avoids having to re-run a potentially expensive collector step. - expectedMetadataStr = buildExpectedMetadataString(newDecryptionToken); - assertEquals(expectedMetadataStr, coredumpHandler.getMetadata(context, coredumpDirectory, () -> attributes, newDecryptionToken)); + // On second invocation the test already exist, so times(1) is not incremented + assertEquals(metadata, coredumpHandler.gatherMetadata(context, coredumpDirectory)); + doThrow(new IllegalStateException("Should not be invoked")) + .when(coreCollector).collect(any(), any()); verify(coreCollector, times(1)).collect(any(), any()); } @Test - void get_metadata_test_without_encryption() throws IOException { - do_get_metadata_test(Optional.empty(), Optional.empty()); // No token in metadata - } - - @Test - void get_metadata_test_with_encryption() throws IOException { - when(secretSharedKeySupplier.create(any())).thenReturn(Optional.of(makeFixedSecretSharedKey())); - do_get_metadata_test(Optional.of("AVeryCoolToken"), Optional.of("AnEvenCoolerToken")); - } - - @Test - void get_metadata_test_without_encryption_then_with_encryption() throws IOException { - // Edge where encryption was enabled between attempted processing runs. - // We don't bother with testing the opposite edge case (encryption -> no encryption), since - // in that case the core dump itself won't be encrypted so the token will be a no-op. - do_get_metadata_test(Optional.empty(), Optional.of("TheSwaggestToken")); - } - - @Test void cant_get_metadata_if_no_core_file() { assertThrows(IllegalStateException.class, () -> { - coredumpHandler.getMetadata(context, context.paths().of("/fake/path"), Map::of, Optional.empty()); + coredumpHandler.gatherMetadata(context, context.paths().of("/fake/path")); }); } @@ -228,31 +194,34 @@ public class CoredumpHandlerTest { }); } - void do_process_single_coredump_test(String expectedCoreFileName) throws IOException { + void do_process_single_coredump_test(String expectedCoreFileName, boolean encrypt) throws IOException { ContainerPath coredumpDirectory = context.paths().of("/path/to/coredump/proccessing/id-123"); Files.createDirectories(coredumpDirectory); - Files.write(coredumpDirectory.resolve("metadata.json"), "{\"test-metadata\":{}}".getBytes()); + Files.write(coredumpDirectory.resolve("metadata2.json"), "{\"test-metadata\":{}}".getBytes()); Files.createFile(coredumpDirectory.resolve("dump_bash.core.431")); - assertFolderContents(coredumpDirectory, "metadata.json", "dump_bash.core.431"); + assertFolderContents(coredumpDirectory, "metadata2.json", "dump_bash.core.431"); + CoreDumpMetadata expectedMetadata = new CoreDumpMetadata(); + if (encrypt) + expectedMetadata.setDecryptionToken("XWTN195KaZecNCloqRICq9YwxwWJEKGqyMYcTgHtwaZfnXtzHzd7bPrPPR60VdxxzA"); - coredumpHandler.processAndReportSingleCoredump(context, coredumpDirectory, Map::of); + coredumpHandler.processAndReportSingleCoreDump(context, coredumpDirectory, Optional.empty()); verify(coreCollector, never()).collect(any(), any()); - verify(coredumpReporter, times(1)).reportCoredump(eq("id-123"), eq("{\"test-metadata\":{}}")); + verify(cores, times(1)).report(eq(context.hostname()), eq("id-123"), eq(expectedMetadata)); assertFalse(Files.exists(coredumpDirectory)); assertFolderContents(doneCoredumpsPath.resolve("container-123"), "id-123"); - assertFolderContents(doneCoredumpsPath.resolve("container-123").resolve("id-123"), "metadata.json", expectedCoreFileName); + assertFolderContents(doneCoredumpsPath.resolve("container-123").resolve("id-123"), "metadata2.json", expectedCoreFileName); } @Test void process_single_coredump_test_without_encryption() throws IOException { - do_process_single_coredump_test("dump_bash.core.431.zst"); + do_process_single_coredump_test("dump_bash.core.431.zst", false); } @Test void process_single_coredump_test_with_encryption() throws IOException { flagSource.withStringFlag(Flags.CORE_ENCRYPTION_PUBLIC_KEY_ID.id(), "bar-key"); when(secretSharedKeySupplier.create(KeyId.ofString("bar-key"))).thenReturn(Optional.of(makeFixedSecretSharedKey())); - do_process_single_coredump_test("dump_bash.core.431.zst.enc"); + do_process_single_coredump_test("dump_bash.core.431.zst.enc", true); } // TODO fail closed instead of open @@ -260,7 +229,7 @@ public class CoredumpHandlerTest { void encryption_disabled_when_no_public_key_set_in_feature_flag() throws IOException { flagSource.withStringFlag(Flags.CORE_ENCRYPTION_PUBLIC_KEY_ID.id(), ""); // empty -> not set verify(secretSharedKeySupplier, never()).create(any()); - do_process_single_coredump_test("dump_bash.core.431.zst"); // No .enc suffix; not encrypted + do_process_single_coredump_test("dump_bash.core.431.zst", false); // No .enc suffix; not encrypted } // TODO fail closed instead of open @@ -268,7 +237,7 @@ public class CoredumpHandlerTest { void encryption_disabled_when_no_key_returned_for_key_id_specified_by_feature_flag() throws IOException { flagSource.withStringFlag(Flags.CORE_ENCRYPTION_PUBLIC_KEY_ID.id(), "baz-key"); when(secretSharedKeySupplier.create(KeyId.ofString("baz-key"))).thenReturn(Optional.empty()); - do_process_single_coredump_test("dump_bash.core.431.zst"); // No .enc suffix; not encrypted + do_process_single_coredump_test("dump_bash.core.431.zst", false); // No .enc suffix; not encrypted } @Test @@ -278,7 +247,7 @@ public class CoredumpHandlerTest { Files.createFile(containerCrashPath.resolve("dump-2")); Files.createFile(containerCrashPath.resolve("hs_err_pid2.log")); Files.createDirectory(processingPath); - Files.createFile(processingPath.resolve("metadata.json")); + Files.createFile(processingPath.resolve("metadata2.json")); Files.createFile(processingPath.resolve("dump-3")); new UnixPath(doneCoredumpsPath.resolve("container-123").resolve("dump-3-folder").resolve("dump-3")) |