Get coredump attributes on demand

3 files changed, 21 insertions, 29 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java
index a3e8d077513..577b4c5eef0 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainer.java
@@ -253,8 +253,7 @@ public class StorageMaintainer {
 
     /** Checks if container has any new coredumps, reports and archives them if so */
     public void handleCoreDumpsForContainer(NodeAgentContext context, Optional<Container> container) {
-        final Map<String, Object> nodeAttributes = getCoredumpNodeAttributes(context, container);
-        coredumpHandler.converge(context, nodeAttributes);
+        coredumpHandler.converge(context, () -> getCoredumpNodeAttributes(context, container));
     }
 
     private Map<String, Object> getCoredumpNodeAttributes(NodeAgentContext context, Optional<Container> container) {
@@ -264,7 +263,7 @@ public class StorageMaintainer {
         attributes.put("environment", context.zoneId().environment().value());
         attributes.put("flavor", context.node().getFlavor());
         attributes.put("kernel_version", System.getProperty("os.version"));
-        attributes.put("cpu_microcode_version", getMicrocodeVersion(context));
+        attributes.put("cpu_microcode_version", getMicrocodeVersion());
 
         container.map(c -> c.image).ifPresent(image -> attributes.put("docker_image", image.asString()));
         context.node().getParentHostname().ifPresent(parent -> attributes.put("parent_hostname", parent));
@@ -292,14 +291,11 @@ public class StorageMaintainer {
         new UnixPath(context.pathOnHostFromPathInNode("/")).deleteRecursively();
     }
 
-    private String getMicrocodeVersion(NodeAgentContext context) {
-        String output = terminal.newCommandLine(context)
-                .add("grep", "microcode", "/proc/cpuinfo")
-                .setTimeout(Duration.ofSeconds(60))
-                .executeSilently()
-                .getOutputLinesStream()
+    private String getMicrocodeVersion() {
+        String output = uncheck(() -> Files.readAllLines(Paths.get("/proc/cpuinfo")).stream()
+                .filter(line -> line.startsWith("microcode"))
                 .findFirst()
-                .orElseThrow(() -> new RuntimeException("No microcode information found in /proc/cpuinfo"));
+                .orElseThrow(() -> new RuntimeException("No microcode information found in /proc/cpuinfo")));
 
         String[] results = output.split(":");
         if (results.length != 2) {
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 19dce1bf75b..95964ec8e7f 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
@@ -68,7 +68,7 @@ public class CoredumpHandler {
     }
 
 
-    public void converge(NodeAgentContext context, Map<String, Object> nodeAttributes) {
+    public void converge(NodeAgentContext context, Supplier<Map<String, Object>> nodeAttributesSupplier) {
         Path containerCrashPathOnHost = context.pathOnHostFromPathInNode(crashPatchInContainer);
         Path containerProcessingPathOnHost = containerCrashPathOnHost.resolve(PROCESSING_DIRECTORY_NAME);
 
@@ -80,7 +80,7 @@ public class CoredumpHandler {
 
         // Check if we have already started to process a core dump or we can enqueue a new core one
         getCoredumpToProcess(containerCrashPathOnHost, containerProcessingPathOnHost)
-                .ifPresent(path -> processAndReportSingleCoredump(context, path, nodeAttributes));
+                .ifPresent(path -> processAndReportSingleCoredump(context, path, nodeAttributesSupplier));
     }
 
     /** @return path to directory inside processing directory that contains a core dump file to process */
@@ -88,8 +88,7 @@ public class CoredumpHandler {
         return FileFinder.directories(containerProcessingPathOnHost).stream()
                 .map(FileFinder.FileAttributes::path)
                 .findAny()
-                .map(Optional::of)
-                .orElseGet(() -> enqueueCoredump(containerCrashPathOnHost, containerProcessingPathOnHost));
+                .or(() -> enqueueCoredump(containerCrashPathOnHost, containerProcessingPathOnHost));
     }
 
     /**
@@ -115,9 +114,9 @@ public class CoredumpHandler {
                 });
     }
 
-    void processAndReportSingleCoredump(NodeAgentContext context, Path coredumpDirectory, Map<String, Object> nodeAttributes) {
+    void processAndReportSingleCoredump(NodeAgentContext context, Path coredumpDirectory, Supplier<Map<String, Object>> nodeAttributesSupplier) {
         try {
-            String metadata = getMetadata(context, coredumpDirectory, nodeAttributes);
+            String metadata = getMetadata(context, coredumpDirectory, nodeAttributesSupplier);
             String coredumpId = coredumpDirectory.getFileName().toString();
             coredumpReporter.reportCoredump(coredumpId, metadata);
             finishProcessing(context, coredumpDirectory);
@@ -131,13 +130,13 @@ public class CoredumpHandler {
      * @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, Path coredumpDirectory, Map<String, Object> nodeAttributes) throws IOException {
+    String getMetadata(NodeAgentContext context, Path coredumpDirectory, Supplier<Map<String, Object>> nodeAttributesSupplier) throws IOException {
         UnixPath metadataPath = new UnixPath(coredumpDirectory.resolve(METADATA_FILE_NAME));
         if (!Files.exists(metadataPath.toPath())) {
             Path coredumpFilePathOnHost = findCoredumpFileInProcessingDirectory(coredumpDirectory);
             Path coredumpFilePathInContainer = context.pathInNodeFromPathOnHost(coredumpFilePathOnHost);
             Map<String, Object> metadata = coreCollector.collect(context, coredumpFilePathInContainer);
-            metadata.putAll(nodeAttributes);
+            metadata.putAll(nodeAttributesSupplier.get());
 
             String metadataFields = objectMapper.writeValueAsString(ImmutableMap.of("fields", metadata));
             metadataPath.writeUtf8File(metadataFields);
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 7779a74ae03..6d3a4dbb553 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,7 +1,6 @@
 // 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 com.google.common.collect.ImmutableMap;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl;
 import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath;
@@ -20,10 +19,8 @@ import java.nio.file.Paths;
 import java.nio.file.attribute.FileTime;
 import java.time.Duration;
 import java.time.Instant;
-import java.util.Arrays;
-import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
+import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 import java.util.Set;
@@ -124,9 +121,9 @@ public class CoredumpHandlerTest {
     public void get_metadata_test() throws IOException {
         Map<String, Object> metadata = new HashMap<>();
         metadata.put("bin_path", "/bin/bash");
-        metadata.put("backtrace", Arrays.asList("call 1", "function 2", "something something"));
+        metadata.put("backtrace", List.of("call 1", "function 2", "something something"));
 
-        Map<String, Object> attributes = ImmutableMap.of(
+        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",
@@ -149,17 +146,17 @@ public class CoredumpHandlerTest {
         when(coreCollector.collect(eq(context), eq(coredumpDirectoryInContainer.resolve("dump_core.456"))))
                 .thenReturn(metadata);
 
-        assertEquals(expectedMetadataStr, coredumpHandler.getMetadata(context, coredumpDirectory, attributes));
+        assertEquals(expectedMetadataStr, coredumpHandler.getMetadata(context, coredumpDirectory, () -> attributes));
         verify(coreCollector, times(1)).collect(any(), any());
 
         // Calling it again will simply read the previously generated metadata from disk
-        assertEquals(expectedMetadataStr, coredumpHandler.getMetadata(context, coredumpDirectory, attributes));
+        assertEquals(expectedMetadataStr, coredumpHandler.getMetadata(context, coredumpDirectory, () -> attributes));
         verify(coreCollector, times(1)).collect(any(), any());
     }
 
     @Test(expected = IllegalStateException.class)
     public void cant_get_metadata_if_no_core_file() throws IOException {
-        coredumpHandler.getMetadata(context, fileSystem.getPath("/fake/path"), Collections.emptyMap());
+        coredumpHandler.getMetadata(context, fileSystem.getPath("/fake/path"), Map::of);
     }
 
     @Test(expected = IllegalStateException.class)
@@ -184,7 +181,7 @@ public class CoredumpHandlerTest {
                     uncheck(() -> Files.createFile(fileSystem.getPath(commandLine.getArguments().get(3))));
                     return new TestChildProcess2(0, "");
                 });
-        coredumpHandler.processAndReportSingleCoredump(context, coredumpDirectory, Collections.emptyMap());
+        coredumpHandler.processAndReportSingleCoredump(context, coredumpDirectory, Map::of);
         verify(coreCollector, never()).collect(any(), any());
         verify(coredumpReporter, times(1)).reportCoredump(eq("id-123"), eq("metadata"));
         assertFalse(Files.exists(coredumpDirectory));
@@ -203,7 +200,7 @@ public class CoredumpHandlerTest {
     }
 
     private static void assertFolderContents(Path pathToFolder, String... filenames) {
-        Set<String> expectedContentsOfFolder = new HashSet<>(Arrays.asList(filenames));
+        Set<String> expectedContentsOfFolder = Set.of(filenames);
         Set<String> actualContentsOfFolder = new UnixPath(pathToFolder)
                 .listContentsOfDirectory().stream()
                 .map(unixPath -> unixPath.toPath().getFileName().toString())