From 4bb061c335e0276db5e9946056734cdbd9b185c0 Mon Sep 17 00:00:00 2001 From: Harald Musum Date: Fri, 1 May 2020 18:26:29 +0200 Subject: Revert "Mount /etc/machine-id and /var/log/journal" This reverts commit 59abe4286a6b9c550c83c59b0a114c37b01db726. --- .../node/admin/docker/DockerOperationsImpl.java | 115 +++++++-------------- .../node/admin/nodeagent/NodeAgentContextImpl.java | 2 +- .../admin/docker/DockerOperationsImplTest.java | 11 +- .../node/admin/integrationTests/DockerTester.java | 2 +- 4 files changed, 42 insertions(+), 88 deletions(-) (limited to 'node-admin/src') 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 c47d6713095..d7f248250a8 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,20 +46,15 @@ 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, FileSystem fileSystem) { + public DockerOperationsImpl(Docker docker, Terminal terminal, IPAddresses ipAddresses, FlagSource flagSource) { this.docker = docker; this.terminal = terminal; this.ipAddresses = ipAddresses; - this.fileSystem = fileSystem; } @Override @@ -124,13 +119,6 @@ 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); @@ -178,7 +166,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(fileSystem.getPath("/etc/hosts"), etcHosts.toString()); + containerData.addFile(Paths.get("/etc/hosts"), etcHosts.toString()); } @Override @@ -279,42 +267,46 @@ public class DockerOperationsImpl implements DockerOperations { return docker.getContainerStats(context.containerName()); } - private void addMounts(NodeAgentContext context, Docker.CreateContainerCommand command) { - var volumes = new VolumeHelper(context, fileSystem, command); + private static void addMounts(NodeAgentContext context, Docker.CreateContainerCommand command) { + Path varLibSia = Paths.get("/var/lib/sia"); // Paths unique to each container - 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"); + List 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 + )); if (context.nodeType() == NodeType.proxy) { - volumes.addPrivateVolumes("logs/nginx", "var/vespa-hosted/routing"); + paths.add(context.pathInNodeUnderVespaHome("logs/nginx")); + paths.add(context.pathInNodeUnderVespaHome("var/vespa-hosted/routing")); } else if (context.nodeType() == NodeType.tenant) - volumes.addPrivateVolumes("/var/lib/sia"); + paths.add(varLibSia); + + paths.forEach(path -> command.withVolume( + context.pathOnHostFromPathInNode(path), + context.rewritePathInNodeForWantedDockerImage(path))); + // Shared paths if (isInfrastructureHost(context.nodeType())) - volumes.addSharedVolumeMap("/var/lib/sia", "/var/lib/sia"); + command.withSharedVolume(varLibSia, varLibSia); boolean isMain = context.zone().getSystemName() == SystemName.cd || context.zone().getSystemName() == SystemName.main; if (isMain && context.nodeType() == NodeType.tenant) - volumes.addSharedVolumeMap("/var/zpe", "var/zpe"); + command.withSharedVolume(Paths.get("/var/zpe"), context.pathInNodeUnderVespaHome("var/zpe")); } @Override @@ -334,41 +326,4 @@ 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 9a021b7ade3..4585d72d6de 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 relativePath.getFileSystem().getPath(pathToVespaHome.resolve(relativePath.toString()).toString()); + return pathToVespaHome.resolve(relativePath); } @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 bacebcc42c0..1c6a82d0bef 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,6 +3,7 @@ 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; @@ -13,12 +14,11 @@ 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.FileSystem; +import java.nio.file.Paths; import java.util.Optional; import java.util.OptionalLong; @@ -36,9 +36,8 @@ 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, fileSystem); + docker, terminal, ipAddresses, new InMemoryFlagSource()); @Test public void processResultFromNodeProgramWhenSuccess() { @@ -99,7 +98,7 @@ public class DockerOperationsImplTest { dockerOperations.addEtcHosts(containerData, hostname, Optional.empty(), Optional.of(ipV6Local)); verify(containerData, times(1)).addFile( - fileSystem.getPath("/etc/hosts"), + Paths.get("/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" + @@ -112,7 +111,7 @@ public class DockerOperationsImplTest { dockerOperations.addEtcHosts(containerData, hostname, Optional.of(ipV4Local), Optional.of(ipV6Local)); verify(containerData, times(1)).addFile( - fileSystem.getPath("/etc/hosts"), + Paths.get("/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 19f991b2ce8..a1ea3558504 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, fileSystem); + DockerOperations dockerOperations = new DockerOperationsImpl(docker, terminal, ipAddresses, flagSource); NodeAgentFactory nodeAgentFactory = (contextSupplier, nodeContext) -> new NodeAgentImpl( contextSupplier, nodeRepository, orchestrator, dockerOperations, storageMaintainer, flagSource, -- cgit v1.2.3