Merge pull request #25483 from vespa-engine/freva/fix-paths

Use TestFileSystem in test

8 files changed, 21 insertions, 69 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 6f3d2d74bc6..f0182ae36e4 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
@@ -27,27 +27,21 @@ import com.yahoo.vespa.hosted.node.admin.task.util.process.Terminal;
 import java.net.URI;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.time.Clock;
 import java.time.Duration;
 import java.time.Instant;
 import java.time.ZoneOffset;
 import java.time.format.DateTimeFormatter;
 import java.util.ArrayList;
-import java.util.Collections;
 import java.util.Comparator;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 import java.util.function.Function;
 import java.util.logging.Level;
 import java.util.logging.Logger;
-import java.util.stream.Collectors;
 
 import static com.yahoo.vespa.hosted.node.admin.maintenance.disk.DiskCleanupRule.Priority;
-import static com.yahoo.yolean.Exceptions.uncheck;
 
 /**
  * @author freva
@@ -172,33 +166,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, boolean throwIfCoreBeingWritten) {
         if (context.isDisabled(NodeAgentTask.CoreDumps)) return;
-        coredumpHandler.converge(context, () -> getCoredumpNodeAttributes(context, container),
-                                 container.map(Container::image), throwIfCoreBeingWritten);
-    }
-
-    private Map<String, Object> getCoredumpNodeAttributes(NodeAgentContext context, Optional<Container> container) {
-        Map<String, String> attributes = new HashMap<>();
-        attributes.put("hostname", context.node().hostname());
-        attributes.put("system", context.zone().getSystemName().value());
-        attributes.put("region", context.zone().getRegionName().value());
-        attributes.put("environment", context.zone().getEnvironment().value());
-        attributes.put("flavor", context.node().flavor());
-        attributes.put("kernel_version", System.getProperty("os.version"));
-        attributes.put("cpu_microcode_version", getMicrocodeVersion());
-
-        container.map(c -> c.image().asString()).ifPresent(image -> attributes.put("docker_image", image));
-        container.flatMap(c -> c.image().tag()).ifPresent(version -> attributes.put("vespa_version", version));
-        context.node().parentHostname().ifPresent(parent -> attributes.put("parent_hostname", parent));
-        context.node().owner().ifPresent(owner -> {
-            attributes.put("tenant", owner.tenant().value());
-            attributes.put("application", owner.application().value());
-            attributes.put("instance", owner.instance().value());
-        });
-        context.node().membership().ifPresent(membership -> {
-            attributes.put("cluster_id", membership.clusterId());
-            attributes.put("cluster_type", membership.type().value());
-        });
-        return Collections.unmodifiableMap(attributes);
+        coredumpHandler.converge(context, container.map(Container::image), throwIfCoreBeingWritten);
     }
 
     /**
@@ -225,18 +193,4 @@ public class StorageMaintainer {
         if (context.nodeType() != NodeType.tenant)
             context.paths().of("/").getFileSystem().createRoot();
     }
-
-    private String getMicrocodeVersion() {
-        String output = uncheck(() -> Files.readAllLines(Paths.get("/proc/cpuinfo")).stream()
-                .filter(line -> line.startsWith("microcode"))
-                .findFirst()
-                .orElse("microcode : UNKNOWN"));
-
-        String[] results = output.split(":");
-        if (results.length != 2) {
-            throw ConvergenceException.ofError("Result from detect microcode command not as expected: " + output);
-        }
-
-        return results[1].trim();
-    }
 }
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 e2da984fa10..bfc4c09cf9e 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
@@ -4,7 +4,6 @@ package com.yahoo.vespa.hosted.node.admin.maintenance.coredump;
 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.FetchVector;
 import com.yahoo.vespa.flags.FlagSource;
 import com.yahoo.vespa.flags.Flags;
@@ -29,11 +28,9 @@ import java.io.OutputStream;
 import java.io.UncheckedIOException;
 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.List;
-import java.util.Map;
 import java.util.Optional;
 import java.util.UUID;
 import java.util.function.Predicate;
@@ -101,8 +98,7 @@ public class CoredumpHandler {
     }
 
 
-    public void converge(NodeAgentContext context, Supplier<Map<String, Object>> nodeAttributesSupplier,
-                         Optional<DockerImage> dockerImage, boolean throwIfCoreBeingWritten) {
+    public void converge(NodeAgentContext context, Optional<DockerImage> dockerImage, boolean throwIfCoreBeingWritten) {
         ContainerPath containerCrashPath = context.paths().of(crashPatchInContainer, context.users().vespa());
         ContainerPath containerProcessingPath = containerCrashPath.resolve(PROCESSING_DIRECTORY_NAME);
 
@@ -328,7 +324,7 @@ public class CoredumpHandler {
     }
 
     private String getMicrocodeVersion() {
-        String output = uncheck(() -> Files.readAllLines(Paths.get("/proc/cpuinfo")).stream()
+        String output = uncheck(() -> Files.readAllLines(doneCoredumpsPath.getFileSystem().getPath("/proc/cpuinfo")).stream()
                                            .filter(line -> line.startsWith("microcode"))
                                            .findFirst()
                                            .orElse("microcode : UNKNOWN"));
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/DefaultEnvWriter.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/DefaultEnvWriter.java
index c5a28c26786..1889332ee49 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/DefaultEnvWriter.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/DefaultEnvWriter.java
@@ -5,7 +5,6 @@ import com.yahoo.vespa.hosted.node.admin.component.TaskContext;
 
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.List;
@@ -62,7 +61,7 @@ public class DefaultEnvWriter {
             return false;
         } else {
             context.log(logger, "Updating " + defaultEnvFile.toString());
-            Path tempFile = Paths.get(defaultEnvFile + ".tmp");
+            Path tempFile = defaultEnvFile.resolveSibling(defaultEnvFile.getFileName() + ".tmp");
             uncheck(() -> Files.write(tempFile, newDefaultEnvLines));
             uncheck(() -> Files.move(tempFile, defaultEnvFile, ATOMIC_MOVE));
             return true;
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java
index ac5035216e9..fbef3def446 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/file/UnixPath.java
@@ -12,7 +12,6 @@ import java.nio.file.NoSuchFileException;
 import java.nio.file.NotDirectoryException;
 import java.nio.file.OpenOption;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.nio.file.StandardCopyOption;
 import java.nio.file.StandardOpenOption;
 import java.nio.file.attribute.FileAttribute;
@@ -45,7 +44,7 @@ public class UnixPath {
     private final Path path;
 
     public UnixPath(Path path) { this.path = path; }
-    public UnixPath(String path) { this(Paths.get(path)); }
+    public UnixPath(String path) { this(Path.of(path)); }
 
     public Path toPath() { return path; }
     public UnixPath resolve(String relativeOrAbsolutePath) { return new UnixPath(path.resolve(relativeOrAbsolutePath)); }
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java
index 65b62848d4b..daae19478ed 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/StorageMaintainerTest.java
@@ -21,7 +21,6 @@ import java.io.IOException;
 import java.nio.file.FileSystem;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.time.Duration;
 import java.time.Instant;
 import java.util.List;
@@ -63,7 +62,7 @@ public class StorageMaintainerTest {
 
     @Test
     void testNonExistingDiskUsed() {
-        DiskSize size = storageMaintainer.getDiskUsed(null, Paths.get("/fake/path"));
+        DiskSize size = storageMaintainer.getDiskUsed(null, Path.of("/fake/path"));
         assertEquals(DiskSize.ZERO, size);
     }
 
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 fd7c366fb8b..33d785eb04e 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
@@ -160,6 +160,8 @@ public class CoredumpHandlerTest {
                                              .setCoreDumpPath(context.paths().of("/home/docker/dumps/container-123/id-123/dump_core.456"))
                                              .setDockerImage(DockerImage.fromString("example.com/vespa/ci:6.48.4"));
 
+        new UnixPath(fileSystem.getPath("/proc/cpuinfo")).createParents().writeUtf8File("microcode\t: 0xf0");
+
         ContainerPath coredumpDirectory = context.paths().of("/var/crash/id-123");
         Files.createDirectories(coredumpDirectory.pathOnHost());
         Files.createFile(coredumpDirectory.resolve("dump_core.456"));
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/DefaultEnvWriterTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/DefaultEnvWriterTest.java
index bc461af0eb3..bd523a16705 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/DefaultEnvWriterTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/DefaultEnvWriterTest.java
@@ -9,11 +9,12 @@ import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.util.logging.Logger;
 
 import static java.nio.file.StandardCopyOption.REPLACE_EXISTING;
-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.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
@@ -27,8 +28,8 @@ public class DefaultEnvWriterTest {
     @TempDir
     public File temporaryFolder;
 
-    private static final Path EXAMPLE_FILE = Paths.get("src/test/resources/default-env-example.txt");
-    private static final Path EXPECTED_RESULT_FILE = Paths.get("src/test/resources/default-env-rewritten.txt");
+    private static final Path EXAMPLE_FILE = Path.of("src/test/resources/default-env-example.txt");
+    private static final Path EXPECTED_RESULT_FILE = Path.of("src/test/resources/default-env-rewritten.txt");
 
     private final TaskContext context = mock(TaskContext.class);
 
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileFinderTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileFinderTest.java
index ce193059fb2..bbc549230c1 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileFinderTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/file/FileFinderTest.java
@@ -13,7 +13,6 @@ import java.nio.file.FileSystem;
 import java.nio.file.Files;
 import java.nio.file.NoSuchFileException;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.nio.file.attribute.BasicFileAttributes;
 import java.nio.file.attribute.FileTime;
 import java.time.Duration;
@@ -26,7 +25,10 @@ import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
 import static java.util.Set.of;
-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.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
@@ -78,7 +80,7 @@ public class FileFinderTest {
         @Test
         void throws_if_prune_path_not_under_base_path() {
             assertThrows(IllegalArgumentException.class, () -> {
-                FileFinder.files(Paths.get("/some/path")).prune(Paths.get("/other/path"));
+                FileFinder.files(Path.of("/some/path")).prune(Path.of("/other/path"));
             });
         }
 
@@ -193,7 +195,7 @@ public class FileFinderTest {
 
         @Test
         void age_filter_test() {
-            Path path = Paths.get("/my/fake/path");
+            Path path = Path.of("/my/fake/path");
             when(attributes.lastModifiedTime()).thenReturn(FileTime.from(Instant.now().minus(Duration.ofHours(1))));
             FileFinder.FileAttributes fileAttributes = new FileFinder.FileAttributes(path, attributes);
 
@@ -206,7 +208,7 @@ public class FileFinderTest {
 
         @Test
         void size_filters() {
-            Path path = Paths.get("/my/fake/path");
+            Path path = Path.of("/my/fake/path");
             when(attributes.size()).thenReturn(100L);
             FileFinder.FileAttributes fileAttributes = new FileFinder.FileAttributes(path, attributes);
 
@@ -219,7 +221,7 @@ public class FileFinderTest {
 
         @Test
         void filename_filters() {
-            Path path = Paths.get("/my/fake/path/some-12352-file.json");
+            Path path = Path.of("/my/fake/path/some-12352-file.json");
             FileFinder.FileAttributes fileAttributes = new FileFinder.FileAttributes(path, attributes);
 
             assertTrue(FileFinder.nameStartsWith("some-").test(fileAttributes));