Merge pull request #13127 from vespa-engine/hakonhall/mount-etcmachine-id-and-varlogjournal

Mount /etc/machine-id and /var/log/journal

4 files changed, 88 insertions, 42 deletions

diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
index d7f248250a8..c47d6713095 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImpl.java
@@ -6,32 +6,32 @@ import com.yahoo.config.provision.DockerImage;
 import com.yahoo.config.provision.HostName;
 import com.yahoo.config.provision.NodeType;
 import com.yahoo.config.provision.SystemName;
-import com.yahoo.vespa.flags.FlagSource;
 import com.yahoo.vespa.hosted.dockerapi.Container;
 import com.yahoo.vespa.hosted.dockerapi.ContainerResources;
 import com.yahoo.vespa.hosted.dockerapi.ContainerStats;
 import com.yahoo.vespa.hosted.dockerapi.Docker;
 import com.yahoo.vespa.hosted.dockerapi.ProcessResult;
-import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeMembership;
 import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.ContainerData;
 import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext;
+import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath;
 import com.yahoo.vespa.hosted.node.admin.task.util.network.IPAddresses;
 import com.yahoo.vespa.hosted.node.admin.task.util.network.IPVersion;
 import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult;
 import com.yahoo.vespa.hosted.node.admin.task.util.process.Terminal;
 
 import java.net.InetAddress;
+import java.nio.file.FileSystem;
 import java.nio.file.Path;
-import java.nio.file.Paths;
 import java.time.Duration;
-import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Optional;
 import java.util.OptionalLong;
+import java.util.Random;
 import java.util.Set;
 import java.util.logging.Logger;
+import java.util.stream.Stream;
 
 /**
  * Class that wraps the Docker class and have some tools related to running programs in docker.
@@ -46,15 +46,20 @@ public class DockerOperationsImpl implements DockerOperations {
 
     private static final InetAddress IPV6_NPT_PREFIX = InetAddresses.forString("fd00::");
     private static final InetAddress IPV4_NPT_PREFIX = InetAddresses.forString("172.17.0.0");
+    public static final String ETC_MACHINE_ID = "/etc/machine-id";
+
+    private static final Random random = new Random(System.nanoTime());
 
     private final Docker docker;
     private final Terminal terminal;
     private final IPAddresses ipAddresses;
+    private final FileSystem fileSystem;
 
-    public DockerOperationsImpl(Docker docker, Terminal terminal, IPAddresses ipAddresses, FlagSource flagSource) {
+    public DockerOperationsImpl(Docker docker, Terminal terminal, IPAddresses ipAddresses, FileSystem fileSystem) {
         this.docker = docker;
         this.terminal = terminal;
         this.ipAddresses = ipAddresses;
+        this.fileSystem = fileSystem;
     }
 
     @Override
@@ -119,6 +124,13 @@ public class DockerOperationsImpl implements DockerOperations {
             command.withIpAddress(ipv4Address);
         }
 
+        UnixPath machineIdPath = new UnixPath(context.pathOnHostFromPathInNode(ETC_MACHINE_ID));
+        if (!machineIdPath.exists()) {
+            String machineId = String.format("%16x%16x\n", random.nextLong(), random.nextLong());
+            machineIdPath.createParents().writeUtf8File(machineId);
+            context.log(logger, "Wrote " + machineId + " to " + machineIdPath);
+        }
+
         addMounts(context, command);
 
         logger.info("Creating new container with args: " + command);
@@ -166,7 +178,7 @@ public class DockerOperationsImpl implements DockerOperations {
         ipV6Local.ifPresent(ipv6 -> etcHosts.append(ipv6.getHostAddress()).append('\t').append(hostname).append('\n'));
         ipV4Local.ifPresent(ipv4 -> etcHosts.append(ipv4.getHostAddress()).append('\t').append(hostname).append('\n'));
 
-        containerData.addFile(Paths.get("/etc/hosts"), etcHosts.toString());
+        containerData.addFile(fileSystem.getPath("/etc/hosts"), etcHosts.toString());
     }
 
     @Override
@@ -267,46 +279,42 @@ public class DockerOperationsImpl implements DockerOperations {
         return docker.getContainerStats(context.containerName());
     }
 
-    private static void addMounts(NodeAgentContext context, Docker.CreateContainerCommand command) {
-        Path varLibSia = Paths.get("/var/lib/sia");
+    private void addMounts(NodeAgentContext context, Docker.CreateContainerCommand command) {
+        var volumes = new VolumeHelper(context, fileSystem, command);
 
         // Paths unique to each container
-        List<Path> paths = new ArrayList<>(List.of(
-                Paths.get("/etc/vespa/flags"), // local file db, to use flags before connection to cfg is established
-                Paths.get("/etc/yamas-agent"), // metrics check configuration
-                Paths.get("/opt/splunkforwarder/var/log"),  // VESPA-14917, thin pool leakage
-                Paths.get("/var/log"),                      // VESPA-14917, thin pool leakage
-                Paths.get("/var/spool/postfix/maildrop"),   // VESPA-14917, thin pool leakage
-                context.pathInNodeUnderVespaHome("logs/vespa"),
-                context.pathInNodeUnderVespaHome("logs/ysar"),
-                context.pathInNodeUnderVespaHome("tmp"),
-                context.pathInNodeUnderVespaHome("var/crash"), // core dumps
-                context.pathInNodeUnderVespaHome("var/container-data"),
-                context.pathInNodeUnderVespaHome("var/db/vespa"),
-                context.pathInNodeUnderVespaHome("var/jdisc_container"),
-                context.pathInNodeUnderVespaHome("var/vespa"),
-                context.pathInNodeUnderVespaHome("var/yca"),
-                context.pathInNodeUnderVespaHome("var/zookeeper") // Tenant content nodes, config server and controller
-        ));
+        volumes.addPrivateVolumes(
+                ETC_MACHINE_ID,  // VESPA-18110, rotation of journal
+                "/etc/vespa/flags", // local file db, to use flags before connection to cfg is established
+                "/etc/yamas-agent", // metrics check configuration
+                "/opt/splunkforwarder/var/log",  // VESPA-14917, thin pool leakage
+                "/var/log",                      // VESPA-14917, thin pool leakage
+                "/var/log/journal",              // VESPA-18110, rotation of journal, must map exact path
+                "/var/spool/postfix/maildrop",
+
+                // Under VESPA_HOME in container
+                "logs/ysar",
+                "tmp",
+                "var/crash", // core dumps
+                "var/container-data",
+                "var/db/vespa",
+                "var/jdisc_container",
+                "var/vespa",
+                "var/yca",
+                "var/zookeeper");
 
         if (context.nodeType() == NodeType.proxy) {
-            paths.add(context.pathInNodeUnderVespaHome("logs/nginx"));
-            paths.add(context.pathInNodeUnderVespaHome("var/vespa-hosted/routing"));
+            volumes.addPrivateVolumes("logs/nginx", "var/vespa-hosted/routing");
         } else if (context.nodeType() == NodeType.tenant)
-            paths.add(varLibSia);
-
-        paths.forEach(path -> command.withVolume(
-                context.pathOnHostFromPathInNode(path),
-                context.rewritePathInNodeForWantedDockerImage(path)));
-
+            volumes.addPrivateVolumes("/var/lib/sia");
 
         // Shared paths
         if (isInfrastructureHost(context.nodeType()))
-            command.withSharedVolume(varLibSia, varLibSia);
+            volumes.addSharedVolumeMap("/var/lib/sia", "/var/lib/sia");
 
         boolean isMain = context.zone().getSystemName() == SystemName.cd || context.zone().getSystemName() == SystemName.main;
         if (isMain && context.nodeType() == NodeType.tenant)
-            command.withSharedVolume(Paths.get("/var/zpe"), context.pathInNodeUnderVespaHome("var/zpe"));
+            volumes.addSharedVolumeMap("/var/zpe", "var/zpe");
     }
 
     @Override
@@ -326,4 +334,41 @@ public class DockerOperationsImpl implements DockerOperations {
                 nodeType == NodeType.controller;
     }
 
+    private static class VolumeHelper {
+        private final NodeAgentContext context;
+        private final FileSystem fileSystem;
+        private final Docker.CreateContainerCommand command;
+
+        public VolumeHelper(NodeAgentContext context, FileSystem fileSystem, Docker.CreateContainerCommand command) {
+            this.context = context;
+            this.fileSystem = fileSystem;
+            this.command = command;
+        }
+
+        /**
+         * Resolve each path to an absolute relative the container's vespa home directory.
+         * Mounts the resulting path, under the container's storage directory as path in the container.
+         */
+        public void addPrivateVolumes(String... pathsInNode) {
+            Stream.of(pathsInNode).forEach(pathString -> {
+                Path pathInNode = context.rewritePathInNodeForWantedDockerImage(resolveNodePath(pathString));
+                Path pathOnHost = context.pathOnHostFromPathInNode(pathInNode.toString());
+                command.withVolume(pathOnHost, pathInNode);
+            });
+        }
+
+        /**
+         * Mounts pathOnHost on the host as pathInNode in the container.  Use for paths that
+         * might be shared with other containers.
+         */
+        public void addSharedVolumeMap(String pathOnHost, String pathInNode) {
+            command.withSharedVolume(resolveNodePath(pathOnHost), resolveNodePath(pathInNode));
+        }
+
+        private Path resolveNodePath(String pathString) {
+            Path path = fileSystem.getPath(pathString);
+            return path.isAbsolute() ? path : context.pathInNodeUnderVespaHome(path);
+        }
+    }
+
 }
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java
index 4585d72d6de..9a021b7ade3 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImpl.java
@@ -143,7 +143,7 @@ public class NodeAgentContextImpl implements NodeAgentContext {
         if (relativePath.isAbsolute())
             throw new IllegalArgumentException("Expected a relative path to the Vespa home, got: " + relativePath);
 
-        return pathToVespaHome.resolve(relativePath);
+        return relativePath.getFileSystem().getPath(pathToVespaHome.resolve(relativePath.toString()).toString());
     }
 
     @Override
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java
index 1c6a82d0bef..bacebcc42c0 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/docker/DockerOperationsImplTest.java
@@ -3,7 +3,6 @@ package com.yahoo.vespa.hosted.node.admin.docker;
 
 import com.google.common.net.InetAddresses;
 import com.yahoo.config.provision.DockerImage;
-import com.yahoo.vespa.flags.InMemoryFlagSource;
 import com.yahoo.vespa.hosted.dockerapi.Container;
 import com.yahoo.vespa.hosted.dockerapi.ContainerName;
 import com.yahoo.vespa.hosted.dockerapi.Docker;
@@ -14,11 +13,12 @@ import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl;
 import com.yahoo.vespa.hosted.node.admin.task.util.network.IPAddresses;
 import com.yahoo.vespa.hosted.node.admin.task.util.network.IPAddressesMock;
 import com.yahoo.vespa.hosted.node.admin.task.util.process.TestTerminal;
+import com.yahoo.vespa.test.file.TestFileSystem;
 import org.junit.Test;
 import org.mockito.InOrder;
 
 import java.net.InetAddress;
-import java.nio.file.Paths;
+import java.nio.file.FileSystem;
 import java.util.Optional;
 import java.util.OptionalLong;
 
@@ -36,8 +36,9 @@ public class DockerOperationsImplTest {
     private final Docker docker = mock(Docker.class);
     private final TestTerminal terminal = new TestTerminal();
     private final IPAddresses ipAddresses = new IPAddressesMock();
+    private final FileSystem fileSystem = TestFileSystem.create();
     private final DockerOperationsImpl dockerOperations = new DockerOperationsImpl(
-            docker, terminal, ipAddresses, new InMemoryFlagSource());
+            docker, terminal, ipAddresses, fileSystem);
 
     @Test
     public void processResultFromNodeProgramWhenSuccess() {
@@ -98,7 +99,7 @@ public class DockerOperationsImplTest {
         dockerOperations.addEtcHosts(containerData, hostname, Optional.empty(), Optional.of(ipV6Local));
 
         verify(containerData, times(1)).addFile(
-                Paths.get("/etc/hosts"),
+                fileSystem.getPath("/etc/hosts"),
                 "# This file was generated by com.yahoo.vespa.hosted.node.admin.docker.DockerOperationsImpl\n" +
                         "127.0.0.1	localhost\n" +
                         "::1	localhost ip6-localhost ip6-loopback\n" +
@@ -111,7 +112,7 @@ public class DockerOperationsImplTest {
         dockerOperations.addEtcHosts(containerData, hostname, Optional.of(ipV4Local), Optional.of(ipV6Local));
 
         verify(containerData, times(1)).addFile(
-                Paths.get("/etc/hosts"),
+                fileSystem.getPath("/etc/hosts"),
                 "# This file was generated by com.yahoo.vespa.hosted.node.admin.docker.DockerOperationsImpl\n" +
                         "127.0.0.1	localhost\n" +
                         "::1	localhost ip6-localhost ip6-loopback\n" +
diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
index a1ea3558504..19f991b2ce8 100644
--- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
+++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integrationTests/DockerTester.java
@@ -81,7 +81,7 @@ public class DockerTester implements AutoCloseable {
         Clock clock = Clock.systemUTC();
         Metrics metrics = new Metrics();
         FileSystem fileSystem = TestFileSystem.create();
-        DockerOperations dockerOperations = new DockerOperationsImpl(docker, terminal, ipAddresses, flagSource);
+        DockerOperations dockerOperations = new DockerOperationsImpl(docker, terminal, ipAddresses, fileSystem);
 
         NodeAgentFactory nodeAgentFactory = (contextSupplier, nodeContext) -> new NodeAgentImpl(
                 contextSupplier, nodeRepository, orchestrator, dockerOperations, storageMaintainer, flagSource,