From 4d5118af963dd985a417e5af45ab9af879e21ba3 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Thu, 14 Oct 2021 16:20:33 +0200 Subject: Use ContainerPath --- .../node/admin/container/ContainerOperations.java | 2 +- .../node/admin/maintenance/StorageMaintainer.java | 32 ++++---- .../admin/maintenance/coredump/CoreCollector.java | 19 +++-- .../maintenance/coredump/CoredumpHandler.java | 58 ++++++------- .../identity/AthenzCredentialsMaintainer.java | 41 +++++----- .../maintenance/servicedump/AbstractProducer.java | 6 +- .../admin/maintenance/servicedump/Artifact.java | 22 ++--- .../maintenance/servicedump/ArtifactProducer.java | 7 +- .../admin/maintenance/servicedump/JvmDumper.java | 32 ++++---- .../maintenance/servicedump/PerfReporter.java | 15 ++-- .../maintenance/servicedump/PmapReporter.java | 9 +- .../maintenance/servicedump/VespaLogDumper.java | 8 +- .../servicedump/VespaServiceDumperImpl.java | 49 +++++------ .../hosted/node/admin/nodeagent/ContainerData.java | 27 +++--- .../node/admin/nodeagent/NodeAgentContext.java | 39 +-------- .../node/admin/nodeagent/NodeAgentContextImpl.java | 91 +++++++-------------- .../hosted/node/admin/nodeagent/NodeAgentImpl.java | 7 +- .../hosted/node/admin/nodeagent/UserNamespace.java | 34 +------- .../node/admin/task/util/fs/package-info.java | 5 ++ .../admin/maintenance/StorageMaintainerTest.java | 30 ++++--- .../maintenance/coredump/CoreCollectorTest.java | 14 ++-- .../maintenance/coredump/CoredumpHandlerTest.java | 95 +++++++++++----------- .../admin/nodeagent/NodeAgentContextImplTest.java | 21 ++--- 23 files changed, 280 insertions(+), 383 deletions(-) create mode 100644 node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/package-info.java (limited to 'node-admin') diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java index acac2098d53..fd38d38b381 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerOperations.java @@ -141,7 +141,7 @@ public class ContainerOperations { } private String executeNodeCtlInContainer(NodeAgentContext context, String program) { - String[] command = new String[] {context.pathInNodeUnderVespaHome("bin/vespa-nodectl").toString(), program}; + String[] command = new String[] {context.containerPathUnderVespaHome("bin/vespa-nodectl").pathInContainer(), program}; return executeCommandInContainerAsRoot(context, command).getOutput(); } 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 340f43b4671..9328eb232a6 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 @@ -22,6 +22,7 @@ import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentTask; import com.yahoo.vespa.hosted.node.admin.task.util.file.DiskSize; import com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder; import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; +import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; import com.yahoo.vespa.hosted.node.admin.task.util.process.Terminal; import java.net.URI; @@ -85,7 +86,7 @@ public class StorageMaintainer { if (archiveUri.isEmpty()) return false; ApplicationId owner = context.node().owner().orElseThrow(); - List syncFileInfos = FileFinder.files(pathOnHostUnderContainerVespaHome(context, "logs/vespa")) + List syncFileInfos = FileFinder.files(context.containerPathUnderVespaHome("logs/vespa")) .maxDepth(2) .stream() .sorted(Comparator.comparing(FileFinder.FileAttributes::lastModifiedTime)) @@ -100,7 +101,7 @@ public class StorageMaintainer { DiskSize cachedDiskUsage = diskUsage.getIfPresent(context.containerName()); if (cachedDiskUsage != null) return Optional.of(cachedDiskUsage); - DiskSize diskUsageBytes = getDiskUsed(context, context.pathOnHostFromPathInNode("/")); + DiskSize diskUsageBytes = getDiskUsed(context, context.containerPath("/").pathOnHost()); diskUsage.put(context.containerName(), diskUsageBytes); return Optional.of(diskUsageBytes); } catch (Exception e) { @@ -109,11 +110,11 @@ public class StorageMaintainer { } } - DiskSize getDiskUsed(TaskContext context, Path path) { - if (!Files.exists(path)) return DiskSize.ZERO; + DiskSize getDiskUsed(TaskContext context, Path pathOnHost) { + if (!Files.exists(pathOnHost)) return DiskSize.ZERO; String output = terminal.newCommandLine(context) - .add("du", "-xsk", path.toString()) + .add("du", "-xsk", pathOnHost.toString()) .setTimeout(Duration.ofSeconds(60)) .executeSilently() .getOutput(); @@ -149,18 +150,18 @@ public class StorageMaintainer { Function monthNormalizer = instant -> Duration.between(instant, start).getSeconds() / oneMonthSeconds; List rules = new ArrayList<>(); - rules.add(CoredumpCleanupRule.forContainer(pathOnHostUnderContainerVespaHome(context, "var/crash"))); + rules.add(CoredumpCleanupRule.forContainer(context.containerPathUnderVespaHome("var/crash"))); if (context.node().membership().map(m -> m.type().hasContainer()).orElse(false)) - rules.add(new LinearCleanupRule(() -> FileFinder.files(pathOnHostUnderContainerVespaHome(context, "logs/vespa/qrs")).list(), + rules.add(new LinearCleanupRule(() -> FileFinder.files(context.containerPathUnderVespaHome("logs/vespa/qrs")).list(), fa -> monthNormalizer.apply(fa.lastModifiedTime()), Priority.LOWEST, Priority.HIGHEST)); if (context.nodeType() == NodeType.tenant && context.node().membership().map(m -> m.type().isAdmin()).orElse(false)) - rules.add(new LinearCleanupRule(() -> FileFinder.files(pathOnHostUnderContainerVespaHome(context, "logs/vespa/logarchive")).list(), + rules.add(new LinearCleanupRule(() -> FileFinder.files(context.containerPathUnderVespaHome("logs/vespa/logarchive")).list(), fa -> monthNormalizer.apply(fa.lastModifiedTime()), Priority.LOWEST, Priority.HIGHEST)); if (context.nodeType() == NodeType.proxy) - rules.add(new LinearCleanupRule(() -> FileFinder.files(pathOnHostUnderContainerVespaHome(context, "logs/nginx")).list(), + rules.add(new LinearCleanupRule(() -> FileFinder.files(context.containerPathUnderVespaHome("logs/nginx")).list(), fa -> monthNormalizer.apply(fa.lastModifiedTime()), Priority.LOWEST, Priority.MEDIUM)); return rules; @@ -202,16 +203,17 @@ public class StorageMaintainer { * Removes old files, reports coredumps and archives container data, runs when container enters state "dirty" */ public void archiveNodeStorage(NodeAgentContext context) { - Path logsDirInContainer = context.pathInNodeUnderVespaHome("logs"); + ContainerPath logsDirInContainer = context.containerPathUnderVespaHome("logs"); Path containerLogsInArchiveDir = archiveContainerStoragePath - .resolve(context.containerName().asString() + "_" + DATE_TIME_FORMATTER.format(clock.instant()) + logsDirInContainer); - UnixPath containerLogsOnHost = new UnixPath(context.pathOnHostFromPathInNode(logsDirInContainer)); + .resolve(context.containerName().asString() + "_" + DATE_TIME_FORMATTER.format(clock.instant()) + logsDirInContainer.pathInContainer()); + // Files.move() does not support moving non-empty directories across providers, move using host paths + UnixPath containerLogsOnHost = new UnixPath(logsDirInContainer.pathOnHost()); if (containerLogsOnHost.exists()) { new UnixPath(containerLogsInArchiveDir).createParents(); containerLogsOnHost.moveIfExists(containerLogsInArchiveDir); } - new UnixPath(context.pathOnHostFromPathInNode("/")).deleteRecursively(); + new UnixPath(context.containerPath("/")).deleteRecursively(); } private String getMicrocodeVersion() { @@ -235,8 +237,4 @@ public class StorageMaintainer { .orElse("") ); } - - private static Path pathOnHostUnderContainerVespaHome(NodeAgentContext context, String path) { - return context.pathOnHostFromPathInNode(context.pathInNodeUnderVespaHome(path)); - } } 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 2c0fa439000..c5c8d0e121d 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 @@ -4,6 +4,7 @@ package com.yahoo.vespa.hosted.node.admin.maintenance.coredump; import com.yahoo.vespa.hosted.node.admin.container.ContainerOperations; import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; +import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult; import java.nio.file.Path; @@ -56,8 +57,8 @@ public class CoreCollector { return GDB_PATH_RHEL8; } - String readBinPathFallback(NodeAgentContext context, Path coredumpPath) { - String command = getGdbPath(context) + " -n -batch -core " + coredumpPath + " | grep \'^Core was generated by\'"; + String readBinPathFallback(NodeAgentContext context, ContainerPath coredumpPath) { + String command = getGdbPath(context) + " -n -batch -core " + coredumpPath.pathInContainer() + " | grep \'^Core was generated by\'"; String[] wrappedCommand = {"/bin/sh", "-c", command}; CommandResult result = docker.executeCommandInContainerAsRoot(context, wrappedCommand); @@ -69,8 +70,8 @@ public class CoreCollector { return matcher.group("path").split(" ")[0]; } - String readBinPath(NodeAgentContext context, Path coredumpPath) { - String[] command = {"file", coredumpPath.toString()}; + String readBinPath(NodeAgentContext context, ContainerPath coredumpPath) { + String[] command = {"file", coredumpPath.pathInContainer()}; try { CommandResult result = docker.executeCommandInContainerAsRoot(context, command); if (result.getExitCode() != 0) { @@ -94,9 +95,9 @@ public class CoreCollector { return readBinPathFallback(context, coredumpPath); } - List readBacktrace(NodeAgentContext context, Path coredumpPath, String binPath, boolean allThreads) { + List readBacktrace(NodeAgentContext context, ContainerPath coredumpPath, String binPath, boolean allThreads) { String threads = allThreads ? "thread apply all bt" : "bt"; - String[] command = {getGdbPath(context), "-n", "-ex", threads, "-batch", binPath, coredumpPath.toString()}; + String[] command = {getGdbPath(context), "-n", "-ex", threads, "-batch", binPath, coredumpPath.pathInContainer()}; CommandResult result = docker.executeCommandInContainerAsRoot(context, command); if (result.getExitCode() != 0) @@ -105,8 +106,8 @@ public class CoreCollector { return List.of(result.getOutput().split("\n")); } - List readJstack(NodeAgentContext context, Path coredumpPath, String binPath) { - String[] command = {"jhsdb", "jstack", "--exe", binPath, "--core", coredumpPath.toString()}; + List readJstack(NodeAgentContext context, ContainerPath coredumpPath, String binPath) { + String[] command = {"jhsdb", "jstack", "--exe", binPath, "--core", coredumpPath.pathInContainer()}; CommandResult result = docker.executeCommandInContainerAsRoot(context, command); if (result.getExitCode() != 0) @@ -121,7 +122,7 @@ public class CoreCollector { * @param coredumpPath path to core dump file inside the container * @return map of relevant metadata about the core dump */ - Map collect(NodeAgentContext context, Path coredumpPath) { + Map collect(NodeAgentContext context, ContainerPath coredumpPath) { if (JAVA_HEAP_DUMP_PATTERN.matcher(coredumpPath.getFileName().toString()).find()) return JAVA_HEAP_DUMP_METADATA; 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 4c0ffa3dce9..0594c5ee016 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 @@ -9,6 +9,7 @@ import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder; import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; +import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; import com.yahoo.vespa.hosted.node.admin.task.util.process.Terminal; import java.io.IOException; @@ -86,13 +87,13 @@ public class CoredumpHandler { public void converge(NodeAgentContext context, Supplier> nodeAttributesSupplier, boolean throwIfCoreBeingWritten) { - Path containerCrashPathOnHost = context.pathOnHostFromPathInNode(crashPatchInContainer); - Path containerProcessingPathOnHost = containerCrashPathOnHost.resolve(PROCESSING_DIRECTORY_NAME); + ContainerPath containerCrashPath = context.containerPath(crashPatchInContainer); + ContainerPath containerProcessingPath = containerCrashPath.resolve(PROCESSING_DIRECTORY_NAME); - updateMetrics(context, containerCrashPathOnHost); + updateMetrics(context, containerCrashPath); if (throwIfCoreBeingWritten) { - List pendingCores = FileFinder.files(containerCrashPathOnHost) + List pendingCores = FileFinder.files(containerCrashPath) .match(fileAttributes -> !isReadyForProcessing(fileAttributes)) .maxDepth(1).stream() .map(FileFinder.FileAttributes::filename) @@ -103,16 +104,17 @@ 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) + getCoredumpToProcess(containerCrashPath, containerProcessingPath) .ifPresent(path -> processAndReportSingleCoredump(context, path, nodeAttributesSupplier)); } /** @return path to directory inside processing directory that contains a core dump file to process */ - Optional getCoredumpToProcess(Path containerCrashPathOnHost, Path containerProcessingPathOnHost) { - return FileFinder.directories(containerProcessingPathOnHost).stream() + Optional getCoredumpToProcess(ContainerPath containerCrashPath, ContainerPath containerProcessingPath) { + return FileFinder.directories(containerProcessingPath).stream() .map(FileFinder.FileAttributes::path) .findAny() - .or(() -> enqueueCoredump(containerCrashPathOnHost, containerProcessingPathOnHost)); + .map(ContainerPath.class::cast) + .or(() -> enqueueCoredump(containerCrashPath, containerProcessingPath)); } /** @@ -124,8 +126,8 @@ public class CoredumpHandler { * * @return path to directory inside processing directory which contains the enqueued core dump file */ - Optional enqueueCoredump(Path containerCrashPathOnHost, Path containerProcessingPathOnHost) { - List toProcess = FileFinder.files(containerCrashPathOnHost) + Optional enqueueCoredump(ContainerPath containerCrashPath, ContainerPath containerProcessingPath) { + List toProcess = FileFinder.files(containerCrashPath) .match(this::isReadyForProcessing) .maxDepth(1) .stream() @@ -141,7 +143,7 @@ public class CoredumpHandler { // Either there are no files in crash directory, or all the files are hs_err files. if (coredumpIndex == -1) return Optional.empty(); - Path enqueuedDir = uncheck(() -> Files.createDirectories(containerProcessingPathOnHost.resolve(coredumpIdSupplier.get()))); + ContainerPath enqueuedDir = (ContainerPath) uncheck(() -> Files.createDirectories(containerProcessingPath.resolve(coredumpIdSupplier.get()))); IntStream.range(0, coredumpIndex + 1) .forEach(i -> { Path path = toProcess.get(i); @@ -151,7 +153,7 @@ public class CoredumpHandler { return Optional.of(enqueuedDir); } - void processAndReportSingleCoredump(NodeAgentContext context, Path coredumpDirectory, Supplier> nodeAttributesSupplier) { + void processAndReportSingleCoredump(NodeAgentContext context, ContainerPath coredumpDirectory, Supplier> nodeAttributesSupplier) { try { String metadata = getMetadata(context, coredumpDirectory, nodeAttributesSupplier); String coredumpId = coredumpDirectory.getFileName().toString(); @@ -167,17 +169,16 @@ 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, Supplier> nodeAttributesSupplier) throws IOException { + String getMetadata(NodeAgentContext context, ContainerPath coredumpDirectory, Supplier> 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 metadata = new HashMap<>(coreCollector.collect(context, coredumpFilePathInContainer)); + ContainerPath coredumpFile = findCoredumpFileInProcessingDirectory(coredumpDirectory); + Map metadata = new HashMap<>(coreCollector.collect(context, coredumpFile)); metadata.putAll(nodeAttributesSupplier.get()); metadata.put("coredump_path", doneCoredumpsPath .resolve(context.containerName().asString()) - .resolve(coredumpDirectory.getFileName()) - .resolve(coredumpFilePathOnHost.getFileName()).toString()); + .resolve(coredumpDirectory.getFileName().toString()) + .resolve(coredumpFile.getFileName().toString()).toString()); String metadataFields = objectMapper.writeValueAsString(Map.of("fields", metadata)); metadataPath.writeUtf8File(metadataFields); @@ -191,23 +192,24 @@ public class CoredumpHandler { * Compresses core file (and deletes the uncompressed core), then moves the entire core dump processing * directory to {@link #doneCoredumpsPath} for archive */ - private void finishProcessing(NodeAgentContext context, Path coredumpDirectory) throws IOException { - Path coreFile = findCoredumpFileInProcessingDirectory(coredumpDirectory); - Path compressedCoreFile = coreFile.getParent().resolve(coreFile.getFileName() + ".lz4"); + private void finishProcessing(NodeAgentContext context, ContainerPath coredumpDirectory) throws IOException { + ContainerPath coreFile = findCoredumpFileInProcessingDirectory(coredumpDirectory); + ContainerPath compressedCoreFile = coreFile.resolveSibling(coreFile.getFileName() + ".lz4"); terminal.newCommandLine(context) - .add(LZ4_PATH, "-f", coreFile.toString(), compressedCoreFile.toString()) + .add(LZ4_PATH, "-f", coreFile.pathOnHost().toString(), compressedCoreFile.pathOnHost().toString()) .setTimeout(Duration.ofMinutes(30)) .execute(); - new UnixPath(compressedCoreFile).setGroupId(operatorGroupId).setPermissions("rw-r-----"); + new UnixPath(compressedCoreFile.pathOnHost()).setGroupId(operatorGroupId).setPermissions("rw-r-----"); Files.delete(coreFile); Path newCoredumpDirectory = doneCoredumpsPath.resolve(context.containerName().asString()); uncheck(() -> Files.createDirectories(newCoredumpDirectory)); - Files.move(coredumpDirectory, newCoredumpDirectory.resolve(coredumpDirectory.getFileName())); + // Files.move() does not support moving non-empty directories across providers, move using host paths + Files.move(coredumpDirectory.pathOnHost(), newCoredumpDirectory.resolve(coredumpDirectory.getFileName().toString())); } - Path findCoredumpFileInProcessingDirectory(Path coredumpProccessingDirectory) { - return FileFinder.files(coredumpProccessingDirectory) + ContainerPath findCoredumpFileInProcessingDirectory(ContainerPath coredumpProccessingDirectory) { + return (ContainerPath) FileFinder.files(coredumpProccessingDirectory) .match(nameStartsWith(COREDUMP_FILENAME_PREFIX).and(nameEndsWith(".lz4").negate())) .maxDepth(1) .stream() @@ -217,11 +219,11 @@ public class CoredumpHandler { "No coredump file found in processing directory " + coredumpProccessingDirectory)); } - void updateMetrics(NodeAgentContext context, Path containerCrashPathOnHost) { + void updateMetrics(NodeAgentContext context, ContainerPath containerCrashPath) { Dimensions dimensions = generateDimensions(context); // Unprocessed coredumps - int numberOfUnprocessedCoredumps = FileFinder.files(containerCrashPathOnHost) + int numberOfUnprocessedCoredumps = FileFinder.files(containerCrashPath) .match(nameStartsWith(".").negate()) .match(nameMatches(HS_ERR_PATTERN).negate()) .match(nameEndsWith(".lz4").negate()) diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java index 280ebd86d13..c5cb6020e1c 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java @@ -26,17 +26,15 @@ import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentTask; import com.yahoo.vespa.hosted.node.admin.task.util.file.FileFinder; import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; +import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; import javax.net.ssl.HostnameVerifier; import javax.net.ssl.SSLContext; import java.io.IOException; import java.io.UncheckedIOException; import java.net.URI; -import java.nio.file.FileSystem; -import java.nio.file.FileSystems; import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.Paths; import java.security.KeyPair; import java.security.PrivateKey; import java.security.cert.X509Certificate; @@ -110,10 +108,10 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { try { context.log(logger, Level.FINE, "Checking certificate"); - Path containerSiaDirectory = context.pathOnHostFromPathInNode(CONTAINER_SIA_DIRECTORY); - Path privateKeyFile = SiaUtils.getPrivateKeyFile(containerSiaDirectory, context.identity()); - Path certificateFile = SiaUtils.getCertificateFile(containerSiaDirectory, context.identity()); - Path identityDocumentFile = containerSiaDirectory.resolve("vespa-node-identity-document.json"); + ContainerPath containerSiaDirectory = context.containerPath(CONTAINER_SIA_DIRECTORY); + ContainerPath privateKeyFile = (ContainerPath) SiaUtils.getPrivateKeyFile(containerSiaDirectory, context.identity()); + ContainerPath certificateFile = (ContainerPath) SiaUtils.getCertificateFile(containerSiaDirectory, context.identity()); + ContainerPath identityDocumentFile = containerSiaDirectory.resolve("vespa-node-identity-document.json"); if (!Files.exists(privateKeyFile) || !Files.exists(certificateFile) || !Files.exists(identityDocumentFile)) { context.log(logger, "Certificate/private key/identity document file does not exist"); Files.createDirectories(privateKeyFile.getParent()); @@ -154,15 +152,15 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { } public void clearCredentials(NodeAgentContext context) { - FileFinder.files(context.pathOnHostFromPathInNode(CONTAINER_SIA_DIRECTORY)) + FileFinder.files(context.containerPath(CONTAINER_SIA_DIRECTORY)) .deleteRecursively(context); lastRefreshAttempt.remove(context.containerName()); } @Override public Duration certificateLifetime(NodeAgentContext context) { - Path containerSiaDirectory = context.pathOnHostFromPathInNode(CONTAINER_SIA_DIRECTORY); - Path certificateFile = SiaUtils.getCertificateFile(containerSiaDirectory, context.identity()); + ContainerPath containerSiaDirectory = context.containerPath(CONTAINER_SIA_DIRECTORY); + ContainerPath certificateFile = (ContainerPath) SiaUtils.getCertificateFile(containerSiaDirectory, context.identity()); try { X509Certificate certificate = readCertificateFromFile(certificateFile); Instant now = clock.instant(); @@ -190,7 +188,7 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { now)) > 0; } - private void registerIdentity(NodeAgentContext context, Path privateKeyFile, Path certificateFile, Path identityDocumentFile) { + private void registerIdentity(NodeAgentContext context, ContainerPath privateKeyFile, ContainerPath certificateFile, ContainerPath identityDocumentFile) { KeyPair keyPair = KeyUtils.generateKeypair(KeyAlgorithm.RSA); SignedIdentityDocument signedIdentityDocument = identityDocumentClient.getNodeIdentityDocument(context.hostname().value()); Pkcs10Csr csr = csrGenerator.generateInstanceCsr( @@ -208,13 +206,13 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { EntityBindingsMapper.toAttestationData(signedIdentityDocument), csr); EntityBindingsMapper.writeSignedIdentityDocumentToFile(identityDocumentFile, signedIdentityDocument); - writePrivateKeyAndCertificate(context.userNamespace().vespaUserIdOnHost(), privateKeyFile, keyPair.getPrivate(), + writePrivateKeyAndCertificate(privateKeyFile, keyPair.getPrivate(), certificateFile, instanceIdentity.certificate()); context.log(logger, "Instance successfully registered and credentials written to file"); } } - private void refreshIdentity(NodeAgentContext context, Path privateKeyFile, Path certificateFile, Path identityDocumentFile) { + private void refreshIdentity(NodeAgentContext context, ContainerPath privateKeyFile, ContainerPath certificateFile, ContainerPath identityDocumentFile) { SignedIdentityDocument identityDocument = EntityBindingsMapper.readSignedIdentityDocumentFromFile(identityDocumentFile); KeyPair keyPair = KeyUtils.generateKeypair(KeyAlgorithm.RSA); Pkcs10Csr csr = csrGenerator.generateInstanceCsr( @@ -236,7 +234,7 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { context.identity(), identityDocument.providerUniqueId().asDottedString(), csr); - writePrivateKeyAndCertificate(context.userNamespace().vespaUserIdOnHost(), privateKeyFile, keyPair.getPrivate(), + writePrivateKeyAndCertificate(privateKeyFile, keyPair.getPrivate(), certificateFile, instanceIdentity.certificate()); context.log(logger, "Instance successfully refreshed and credentials written to file"); } catch (ZtsClientException e) { @@ -253,23 +251,22 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { } - private static void writePrivateKeyAndCertificate(int vespaUidOnHost, - Path privateKeyFile, + private static void writePrivateKeyAndCertificate(ContainerPath privateKeyFile, PrivateKey privateKey, - Path certificateFile, + ContainerPath certificateFile, X509Certificate certificate) { - writeFile(privateKeyFile, vespaUidOnHost, KeyUtils.toPem(privateKey)); - writeFile(certificateFile, vespaUidOnHost, X509CertificateUtils.toPem(certificate)); + writeFile(privateKeyFile, KeyUtils.toPem(privateKey)); + writeFile(certificateFile, X509CertificateUtils.toPem(certificate)); } - private static void writeFile(Path path, int vespaUidOnHost, String utf8Content) { + private static void writeFile(ContainerPath path, String utf8Content) { new UnixPath(path.resolveSibling(path.getFileName() + ".tmp")) .writeUtf8File(utf8Content, "r--------") - .setOwnerId(vespaUidOnHost) + .setOwner("vespa") .atomicMove(path); } - private static X509Certificate readCertificateFromFile(Path certificateFile) throws IOException { + private static X509Certificate readCertificateFromFile(ContainerPath certificateFile) throws IOException { String pemEncodedCertificate = new String(Files.readAllBytes(certificateFile)); return X509CertificateUtils.fromPem(pemEncodedCertificate); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/AbstractProducer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/AbstractProducer.java index 6bcf41b89c2..1756b81f795 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/AbstractProducer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/AbstractProducer.java @@ -3,10 +3,10 @@ package com.yahoo.vespa.hosted.node.admin.maintenance.servicedump; 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.task.util.fs.ContainerPath; import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult; import java.io.IOException; -import java.nio.file.Path; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; @@ -48,8 +48,8 @@ abstract class AbstractProducer implements ArtifactProducer { } protected int findVespaServicePid(NodeAgentContext ctx, String configId) throws IOException { - Path findPidBinary = ctx.pathInNodeUnderVespaHome("libexec/vespa/find-pid"); - CommandResult findPidResult = executeCommand(ctx, List.of(findPidBinary.toString(), configId), true); + ContainerPath findPidBinary = ctx.containerPathUnderVespaHome("libexec/vespa/find-pid"); + CommandResult findPidResult = executeCommand(ctx, List.of(findPidBinary.pathInContainer(), configId), true); return Integer.parseInt(findPidResult.getOutput()); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/Artifact.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/Artifact.java index edab90afea7..6f29a9c2558 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/Artifact.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/Artifact.java @@ -1,7 +1,8 @@ // 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.servicedump; -import java.nio.file.Path; +import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; + import java.util.Optional; /** @@ -21,18 +22,14 @@ class Artifact { } private final Classification classification; - private final Path fileInNode; - private final Path fileOnHost; + private final ContainerPath file; private final boolean compressOnUpload; private Artifact(Builder builder) { - if (builder.fileOnHost == null && builder.fileInNode == null) { + if (builder.file == null) { throw new IllegalArgumentException("No file specified"); - } else if (builder.fileOnHost != null && builder.fileInNode != null) { - throw new IllegalArgumentException("Only a single file can be specified"); } - this.fileInNode = builder.fileInNode; - this.fileOnHost = builder.fileOnHost; + this.file = builder.file; this.classification = builder.classification; this.compressOnUpload = Boolean.TRUE.equals(builder.compressOnUpload); } @@ -40,21 +37,18 @@ class Artifact { static Builder newBuilder() { return new Builder(); } Optional classification() { return Optional.ofNullable(classification); } - Optional fileInNode() { return Optional.ofNullable(fileInNode); } - Optional fileOnHost() { return Optional.ofNullable(fileOnHost); } + ContainerPath file() { return file; } boolean compressOnUpload() { return compressOnUpload; } static class Builder { private Classification classification; - private Path fileInNode; - private Path fileOnHost; + private ContainerPath file; private Boolean compressOnUpload; private Builder() {} Builder classification(Classification c) { this.classification = c; return this; } - Builder fileInNode(Path f) { this.fileInNode = f; return this; } - Builder fileOnHost(Path f) { this.fileOnHost = f; return this; } + Builder file(ContainerPath f) { this.file = f; return this; } Builder compressOnUpload() { this.compressOnUpload = true; return this; } Artifact build() { return new Artifact(this); } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducer.java index e4a9e6aeea5..0394756cc77 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/ArtifactProducer.java @@ -1,9 +1,9 @@ // 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.servicedump; +import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult; -import java.nio.file.Path; import java.util.List; import java.util.OptionalDouble; @@ -22,9 +22,8 @@ interface ArtifactProducer { String serviceId(); int servicePid(); CommandResult executeCommandInNode(List command, boolean logOutput); - Path outputDirectoryInNode(); - Path pathInNodeUnderVespaHome(String relativePath); - Path pathOnHostFromPathInNode(Path pathInNode); + ContainerPath outputContainerPath(); + ContainerPath containerPathUnderVespaHome(String relativePath); Options options(); interface Options { diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JvmDumper.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JvmDumper.java index cf206918568..8b6ca1384e9 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JvmDumper.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JvmDumper.java @@ -1,9 +1,9 @@ // 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.servicedump; +import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; import com.yahoo.yolean.concurrent.Sleeper; -import java.nio.file.Path; import java.time.Duration; import java.util.List; @@ -22,12 +22,12 @@ class JvmDumper { @Override public List produceArtifacts(Context ctx) { - Path heapDumpFile = ctx.outputDirectoryInNode().resolve("jvm-heap-dump.bin"); + ContainerPath heapDumpFile = ctx.outputContainerPath().resolve("jvm-heap-dump.bin"); List cmd = List.of( - "jmap", "-dump:live,format=b,file=" + heapDumpFile, Integer.toString(ctx.servicePid())); + "jmap", "-dump:live,format=b,file=" + heapDumpFile.pathInContainer(), Integer.toString(ctx.servicePid())); ctx.executeCommandInNode(cmd, true); return List.of( - Artifact.newBuilder().classification(CONFIDENTIAL).fileInNode(heapDumpFile).compressOnUpload().build()); + Artifact.newBuilder().classification(CONFIDENTIAL).file(heapDumpFile).compressOnUpload().build()); } } @@ -37,10 +37,10 @@ class JvmDumper { @Override public List produceArtifacts(Context ctx) { - Path jmapReport = ctx.outputDirectoryInNode().resolve("jvm-jmap.txt"); - List cmd = List.of("bash", "-c", "jhsdb jmap --heap --pid " + ctx.servicePid() + " > " + jmapReport); + ContainerPath jmapReport = ctx.outputContainerPath().resolve("jvm-jmap.txt"); + List cmd = List.of("bash", "-c", "jhsdb jmap --heap --pid " + ctx.servicePid() + " > " + jmapReport.pathInContainer()); ctx.executeCommandInNode(cmd, true); - return List.of(Artifact.newBuilder().classification(INTERNAL).fileInNode(jmapReport).build()); + return List.of(Artifact.newBuilder().classification(INTERNAL).file(jmapReport).build()); } } @@ -50,10 +50,10 @@ class JvmDumper { @Override public List produceArtifacts(Context ctx) { - Path jstatReport = ctx.outputDirectoryInNode().resolve("jvm-jstat.txt"); - List cmd = List.of("bash", "-c", "jstat -gcutil " + ctx.servicePid() + " > " + jstatReport); + ContainerPath jstatReport = ctx.outputContainerPath().resolve("jvm-jstat.txt"); + List cmd = List.of("bash", "-c", "jstat -gcutil " + ctx.servicePid() + " > " + jstatReport.pathInContainer()); ctx.executeCommandInNode(cmd, true); - return List.of(Artifact.newBuilder().classification(INTERNAL).fileInNode(jstatReport).build()); + return List.of(Artifact.newBuilder().classification(INTERNAL).file(jstatReport).build()); } } @@ -63,9 +63,9 @@ class JvmDumper { @Override public List produceArtifacts(Context ctx) { - Path jstackReport = ctx.outputDirectoryInNode().resolve("jvm-jstack.txt"); - ctx.executeCommandInNode(List.of("bash", "-c", "jstack " + ctx.servicePid() + " > " + jstackReport), true); - return List.of(Artifact.newBuilder().classification(INTERNAL).fileInNode(jstackReport).build()); + ContainerPath jstackReport = ctx.outputContainerPath().resolve("jvm-jstack.txt"); + ctx.executeCommandInNode(List.of("bash", "-c", "jstack " + ctx.servicePid() + " > " + jstackReport.pathInContainer()), true); + return List.of(Artifact.newBuilder().classification(INTERNAL).file(jstackReport).build()); } } @@ -80,9 +80,9 @@ class JvmDumper { @Override public List produceArtifacts(ArtifactProducer.Context ctx) { int seconds = (int) (ctx.options().duration().orElse(30.0)); - Path outputFile = ctx.outputDirectoryInNode().resolve("recording.jfr"); + ContainerPath outputFile = ctx.outputContainerPath().resolve("recording.jfr"); List startCommand = List.of("jcmd", Integer.toString(ctx.servicePid()), "JFR.start", "name=host-admin", - "path-to-gc-roots=true", "settings=profile", "filename=" + outputFile, "duration=" + seconds + "s"); + "path-to-gc-roots=true", "settings=profile", "filename=" + outputFile.pathInContainer(), "duration=" + seconds + "s"); ctx.executeCommandInNode(startCommand, true); sleeper.sleep(Duration.ofSeconds(seconds).plusSeconds(1)); int maxRetries = 10; @@ -92,7 +92,7 @@ class JvmDumper { .anyMatch(l -> l.contains("name=host-admin") && l.contains("running")); if (!stillRunning) { Artifact a = Artifact.newBuilder() - .classification(CONFIDENTIAL).fileInNode(outputFile).compressOnUpload().build(); + .classification(CONFIDENTIAL).file(outputFile).compressOnUpload().build(); return List.of(a); } sleeper.sleep(Duration.ofSeconds(1)); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PerfReporter.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PerfReporter.java index 07c8b709e04..3dae6544304 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PerfReporter.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PerfReporter.java @@ -1,7 +1,8 @@ // 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.servicedump; -import java.nio.file.Path; +import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; + import java.util.ArrayList; import java.util.List; @@ -25,15 +26,15 @@ class PerfReporter implements ArtifactProducer { if (ctx.options().callGraphRecording()) { perfRecordCommand.add("-g"); } - Path recordFile = ctx.outputDirectoryInNode().resolve("perf-record.bin"); + ContainerPath recordFile = ctx.outputContainerPath().resolve("perf-record.bin"); perfRecordCommand.addAll( - List.of("--output=" + recordFile, + List.of("--output=" + recordFile.pathInContainer(), "--pid=" + ctx.servicePid(), "sleep", Integer.toString(duration))); ctx.executeCommandInNode(perfRecordCommand, true); - Path reportFile = ctx.outputDirectoryInNode().resolve("perf-report.txt"); - ctx.executeCommandInNode(List.of("bash", "-c", "perf report --input=" + recordFile + " > " + reportFile), true); + ContainerPath reportFile = ctx.outputContainerPath().resolve("perf-report.txt"); + ctx.executeCommandInNode(List.of("bash", "-c", "perf report --input=" + recordFile.pathInContainer() + " > " + reportFile.pathInContainer()), true); return List.of( - Artifact.newBuilder().classification(CONFIDENTIAL).fileInNode(recordFile).compressOnUpload().build(), - Artifact.newBuilder().classification(INTERNAL).fileInNode(reportFile).build()); + Artifact.newBuilder().classification(CONFIDENTIAL).file(recordFile).compressOnUpload().build(), + Artifact.newBuilder().classification(INTERNAL).file(reportFile).build()); } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PmapReporter.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PmapReporter.java index 659628d03a0..8087ea7eec0 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PmapReporter.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/PmapReporter.java @@ -1,7 +1,8 @@ // 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.servicedump; -import java.nio.file.Path; +import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; + import java.util.List; import static com.yahoo.vespa.hosted.node.admin.maintenance.servicedump.Artifact.Classification.INTERNAL; @@ -15,9 +16,9 @@ class PmapReporter implements ArtifactProducer { @Override public List produceArtifacts(Context ctx) { - Path pmapReport = ctx.outputDirectoryInNode().resolve("pmap.txt"); - List cmd = List.of("bash", "-c", "pmap -x " + ctx.servicePid() + " > " + pmapReport); + ContainerPath pmapReport = ctx.outputContainerPath().resolve("pmap.txt"); + List cmd = List.of("bash", "-c", "pmap -x " + ctx.servicePid() + " > " + pmapReport.pathInContainer()); ctx.executeCommandInNode(cmd, true); - return List.of(Artifact.newBuilder().classification(INTERNAL).fileInNode(pmapReport).build()); + return List.of(Artifact.newBuilder().classification(INTERNAL).file(pmapReport).build()); } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaLogDumper.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaLogDumper.java index 24224789877..e6e8df7585e 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaLogDumper.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaLogDumper.java @@ -1,10 +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.servicedump; +import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; import com.yahoo.yolean.concurrent.Sleeper; import java.nio.file.Files; -import java.nio.file.Path; import java.time.Duration; import java.util.List; import java.util.logging.Logger; @@ -33,12 +33,12 @@ class VespaLogDumper implements ArtifactProducer { ctx.executeCommandInNode(List.of("kill", "-SIGPROF", Integer.toString(ctx.servicePid())), true); sleeper.sleep(Duration.ofSeconds(3)); } - Path vespaLogFile = ctx.pathOnHostFromPathInNode(ctx.pathInNodeUnderVespaHome("logs/vespa/vespa.log")); - Path destination = ctx.pathOnHostFromPathInNode(ctx.outputDirectoryInNode()).resolve("vespa.log"); + ContainerPath vespaLogFile = ctx.containerPathUnderVespaHome("logs/vespa/vespa.log"); + ContainerPath destination = ctx.outputContainerPath().resolve("vespa.log"); if (Files.exists(vespaLogFile)) { uncheck(() -> Files.copy(vespaLogFile, destination)); return List.of( - Artifact.newBuilder().classification(CONFIDENTIAL).fileOnHost(destination).compressOnUpload().build()); + Artifact.newBuilder().classification(CONFIDENTIAL).file(destination).compressOnUpload().build()); } else { log.info("Log file '" + vespaLogFile + "' does not exist"); return List.of(); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImpl.java index 27b3467163c..59f5e0f3f40 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImpl.java @@ -12,11 +12,11 @@ import com.yahoo.vespa.hosted.node.admin.maintenance.sync.SyncFileInfo; import com.yahoo.vespa.hosted.node.admin.maintenance.sync.SyncFileInfo.Compression; 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.fs.ContainerPath; import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult; import com.yahoo.yolean.concurrent.Sleeper; import java.net.URI; -import java.nio.file.Path; import java.time.Clock; import java.time.Instant; import java.time.temporal.ChronoUnit; @@ -88,21 +88,21 @@ public class VespaServiceDumperImpl implements VespaServiceDumper { handleFailure(context, request, startedAt, "No artifacts requested"); return; } - UnixPath directoryInNode = new UnixPath(context.pathInNodeUnderVespaHome("tmp/vespa-service-dump")); - UnixPath directoryOnHost = new UnixPath(context.pathOnHostFromPathInNode(directoryInNode.toPath())); + ContainerPath directory = context.containerPathUnderVespaHome("tmp/vespa-service-dump"); + UnixPath unixPathDirectory = new UnixPath(directory); try { context.log(log, Level.INFO, "Creating service dump for " + configId + " requested at " + Instant.ofEpochMilli(request.getCreatedMillisOrNull())); storeReport(context, ServiceDumpReport.createStartedReport(request, startedAt)); - if (directoryOnHost.exists()) { - context.log(log, Level.INFO, "Removing existing directory '" + directoryOnHost +"'."); - directoryOnHost.deleteRecursively(); + if (unixPathDirectory.exists()) { + context.log(log, Level.INFO, "Removing existing directory '" + unixPathDirectory +"'."); + unixPathDirectory.deleteRecursively(); } - context.log(log, Level.INFO, "Creating '" + directoryOnHost +"'."); - directoryOnHost.createDirectory("rwxrwxrwx"); + context.log(log, Level.INFO, "Creating '" + unixPathDirectory +"'."); + unixPathDirectory.createDirectory("rwxrwxrwx"); URI destination = serviceDumpDestination(nodeSpec, createDumpId(request)); - ProducerContext producerCtx = new ProducerContext(context, directoryInNode.toPath(), request); + ProducerContext producerCtx = new ProducerContext(context, directory, request); List producedArtifacts = new ArrayList<>(); for (ArtifactProducer producer : artifactProducers.resolve(requestedArtifacts)) { context.log(log, "Producing artifact of type '" + producer.artifactName() + "'"); @@ -113,9 +113,9 @@ public class VespaServiceDumperImpl implements VespaServiceDumper { } catch (Exception e) { handleFailure(context, request, startedAt, e); } finally { - if (directoryOnHost.exists()) { - context.log(log, Level.INFO, "Deleting directory '" + directoryOnHost +"'."); - directoryOnHost.deleteRecursively(); + if (unixPathDirectory.exists()) { + context.log(log, Level.INFO, "Deleting directory '" + unixPathDirectory +"'."); + unixPathDirectory.deleteRecursively(); } } } @@ -126,10 +126,8 @@ public class VespaServiceDumperImpl implements VespaServiceDumper { List filesToUpload = producedArtifacts.stream() .map(a -> { Compression compression = a.compressOnUpload() ? Compression.ZSTD : Compression.NONE; - Path fileInNode = a.fileInNode().orElse(null); - Path fileOnHost = fileInNode != null ? ctx.pathOnHostFromPathInNode(fileInNode) : a.fileOnHost().orElseThrow(); String classification = a.classification().map(Artifact.Classification::value).orElse(null); - return SyncFileInfo.forServiceDump(destination, fileOnHost, expiry, compression, owner, classification); + return SyncFileInfo.forServiceDump(destination, a.file(), expiry, compression, owner, classification); }) .collect(Collectors.toList()); ctx.log(log, Level.INFO, @@ -179,13 +177,13 @@ public class VespaServiceDumperImpl implements VespaServiceDumper { private class ProducerContext implements ArtifactProducer.Context, ArtifactProducer.Context.Options { final NodeAgentContext nodeAgentCtx; - final Path outputDirectoryInNode; + final ContainerPath path; final ServiceDumpReport request; volatile int pid = -1; - ProducerContext(NodeAgentContext nodeAgentCtx, Path outputDirectoryInNode, ServiceDumpReport request) { + ProducerContext(NodeAgentContext nodeAgentCtx, ContainerPath path, ServiceDumpReport request) { this.nodeAgentCtx = nodeAgentCtx; - this.outputDirectoryInNode = outputDirectoryInNode; + this.path = path; this.request = request; } @@ -194,8 +192,8 @@ public class VespaServiceDumperImpl implements VespaServiceDumper { @Override public int servicePid() { if (pid == -1) { - Path findPidBinary = nodeAgentCtx.pathInNodeUnderVespaHome("libexec/vespa/find-pid"); - CommandResult findPidResult = executeCommandInNode(List.of(findPidBinary.toString(), serviceId()), true); + ContainerPath findPidBinary = nodeAgentCtx.containerPathUnderVespaHome("libexec/vespa/find-pid"); + CommandResult findPidResult = executeCommandInNode(List.of(findPidBinary.pathInContainer(), serviceId()), true); this.pid = Integer.parseInt(findPidResult.getOutput()); } return pid; @@ -224,16 +222,11 @@ public class VespaServiceDumperImpl implements VespaServiceDumper { return result; } - @Override public Path outputDirectoryInNode() { return outputDirectoryInNode; } + @Override public ContainerPath outputContainerPath() { return path; } @Override - public Path pathInNodeUnderVespaHome(String relativePath) { - return nodeAgentCtx.pathInNodeUnderVespaHome(relativePath); - } - - @Override - public Path pathOnHostFromPathInNode(Path pathInNode) { - return nodeAgentCtx.pathOnHostFromPathInNode(pathInNode); + public ContainerPath containerPathUnderVespaHome(String relativePath) { + return nodeAgentCtx.containerPathUnderVespaHome(relativePath); } @Override public Options options() { return this; } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/ContainerData.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/ContainerData.java index e32641c0276..07e3268b9e4 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/ContainerData.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/ContainerData.java @@ -1,6 +1,8 @@ // 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.nodeagent; +import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; + import java.nio.file.Path; /** @@ -9,27 +11,18 @@ import java.nio.file.Path; * @author hakon */ public interface ContainerData { - /** - * Add or overwrite file in container at path. - * - * @param pathInContainer The path to the file inside the container, absolute or relative root /. - * @param data The content of the file. - */ - void addFile(Path pathInContainer, String data); - /** - * Add directory in container at path. - * - * @param pathInContainer The path to the directory inside the container, absolute or relative root /. - */ - void addDirectory(Path pathInContainer); + /** Add or overwrite file in container at path. */ + void addFile(ContainerPath path, String data); + + /** Add directory in container at path. */ + void addDirectory(ContainerPath path); /** * Symlink to a file in container at path. - * - * @param symlink The path to the symlink inside the container, absolute or relative root /. - * @param target The path to the target file for the symbolic link inside the container, absolute or relative root /. + * @param symlink The path to the symlink inside the container + * @param target The path to the target file for the symbolic link inside the container */ - void createSymlink(Path symlink, Path target); + void createSymlink(ContainerPath symlink, Path target); } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java index c585bd14e94..8cf8553bc34 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContext.java @@ -11,8 +11,8 @@ import com.yahoo.vespa.hosted.node.admin.component.TaskContext; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.container.ContainerNetworkMode; +import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; -import java.nio.file.FileSystem; import java.nio.file.Path; import java.util.Optional; @@ -58,42 +58,11 @@ public interface NodeAgentContext extends TaskContext { */ double vcpuOnThisHost(); - /** The file system used by the NodeAgentContext. All paths must have the same provider. */ - FileSystem fileSystem(); + ContainerPath containerPath(String pathInNode); - /** - * This method is the inverse of {@link #pathInNodeFromPathOnHost(Path)}} - * - * @param pathInNode absolute path in the container - * @return the absolute path on host pointing at the same inode - */ - Path pathOnHostFromPathInNode(Path pathInNode); - - default Path pathOnHostFromPathInNode(String pathInNode) { - return pathOnHostFromPathInNode(fileSystem().getPath(pathInNode)); - } + ContainerPath containerPathUnderVespaHome(String relativePath); - /** - * This method is the inverse of {@link #pathOnHostFromPathInNode(Path)} - * - * @param pathOnHost absolute path on host - * @return the absolute path in the container pointing at the same inode - */ - Path pathInNodeFromPathOnHost(Path pathOnHost); - - default Path pathInNodeFromPathOnHost(String pathOnHost) { - return pathInNodeFromPathOnHost(fileSystem().getPath(pathOnHost)); - } - - /** - * @param relativePath relative path under Vespa home in container - * @return the absolute path under Vespa home in the container - */ - Path pathInNodeUnderVespaHome(Path relativePath); - - default Path pathInNodeUnderVespaHome(String relativePath) { - return pathInNodeUnderVespaHome(fileSystem().getPath(relativePath)); - } + ContainerPath containerPathFromPathOnHost(Path pathOnHost); Optional hostExclusiveTo(); } 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 6925c1e9600..899df091e7a 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 @@ -16,11 +16,12 @@ import com.yahoo.vespa.hosted.node.admin.container.ContainerName; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.Acl; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.container.ContainerNetworkMode; +import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerFileSystem; +import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; import java.nio.file.FileSystem; -import java.nio.file.FileSystems; +import java.nio.file.Files; import java.nio.file.Path; -import java.nio.file.ProviderMismatchException; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -28,6 +29,8 @@ import java.util.function.Function; import java.util.logging.Level; import java.util.logging.Logger; +import static com.yahoo.yolean.Exceptions.uncheck; + /** * @author freva */ @@ -40,9 +43,8 @@ public class NodeAgentContextImpl implements NodeAgentContext { private final AthenzIdentity identity; private final ContainerNetworkMode containerNetworkMode; private final ZoneApi zone; - private final FileSystem fileSystem; - private final Path pathToNodeRootOnHost; - private final Path pathToVespaHome; + private final ContainerFileSystem containerFs; + private final ContainerPath pathToVespaHome; private final UserNamespace userNamespace; private final double cpuSpeedup; private final Set disabledNodeAgentTasks; @@ -50,8 +52,7 @@ public class NodeAgentContextImpl implements NodeAgentContext { public NodeAgentContextImpl(NodeSpec node, Acl acl, AthenzIdentity identity, ContainerNetworkMode containerNetworkMode, ZoneApi zone, - FileSystem fileSystem, FlagSource flagSource, - Path pathToContainerStorage, Path pathToVespaHome, + FlagSource flagSource, Path pathToContainerStorage, String pathToVespaHome, UserNamespace userNamespace, double cpuSpeedup, Optional hostExclusiveTo) { if (cpuSpeedup <= 0) @@ -63,9 +64,8 @@ public class NodeAgentContextImpl implements NodeAgentContext { this.identity = Objects.requireNonNull(identity); this.containerNetworkMode = Objects.requireNonNull(containerNetworkMode); this.zone = Objects.requireNonNull(zone); - this.fileSystem = Objects.requireNonNull(fileSystem); - this.pathToNodeRootOnHost = requireValidPath(pathToContainerStorage).resolve(containerName.asString()); - this.pathToVespaHome = requireValidPath(pathToVespaHome); + this.containerFs = ContainerFileSystem.create(pathToContainerStorage.resolve(containerName.asString()), userNamespace.rootUserIdOnHost(), userNamespace.rootGroupIdOnHost()); + this.pathToVespaHome = containerFs.getPath(pathToVespaHome); this.logPrefix = containerName.asString() + ": "; this.userNamespace = Objects.requireNonNull(userNamespace); this.cpuSpeedup = cpuSpeedup; @@ -120,41 +120,21 @@ public class NodeAgentContextImpl implements NodeAgentContext { } @Override - public FileSystem fileSystem() { - return fileSystem; + public ContainerPath containerPath(String pathInNode) { + return containerFs.getPath(pathInNode); } @Override - public Path pathOnHostFromPathInNode(Path pathInNode) { - requireValidPath(pathInNode); - - if (! pathInNode.isAbsolute()) - throw new IllegalArgumentException("Expected an absolute path in the container, got: " + pathInNode); + public ContainerPath containerPathUnderVespaHome(String relativePath) { + if (relativePath.startsWith("/")) + throw new IllegalArgumentException("Expected a relative path to the Vespa home, got: " + relativePath); - return pathToNodeRootOnHost.resolve(pathInNode.getRoot().relativize(pathInNode)); + return pathToVespaHome.resolve(relativePath); } @Override - public Path pathInNodeFromPathOnHost(Path pathOnHost) { - requireValidPath(pathOnHost); - - if (! pathOnHost.isAbsolute()) - throw new IllegalArgumentException("Expected an absolute path on the host, got: " + pathOnHost); - - if (!pathOnHost.startsWith(pathToNodeRootOnHost)) - throw new IllegalArgumentException("Path " + pathOnHost + " does not exist in the container"); - - return pathOnHost.getRoot().resolve(pathToNodeRootOnHost.relativize(pathOnHost)); - } - - @Override - public Path pathInNodeUnderVespaHome(Path relativePath) { - requireValidPath(relativePath); - - if (relativePath.isAbsolute()) - throw new IllegalArgumentException("Expected a relative path to the Vespa home, got: " + relativePath); - - return pathToVespaHome.resolve(relativePath); + public ContainerPath containerPathFromPathOnHost(Path pathOnHost) { + return ContainerPath.fromPathOnHost(containerFs, pathOnHost); } @Override @@ -186,24 +166,11 @@ public class NodeAgentContextImpl implements NodeAgentContext { ", identity=" + identity + ", dockerNetworking=" + containerNetworkMode + ", zone=" + zone + - ", pathToNodeRootOnHost=" + pathToNodeRootOnHost + ", pathToVespaHome=" + pathToVespaHome + ", hostExclusiveTo='" + hostExclusiveTo + '\'' + '}'; } - private Path requireValidPath(Path path) { - Objects.requireNonNull(path); - - Objects.requireNonNull(fileSystem); // to allow this method to be used in constructor. - if (!path.getFileSystem().provider().equals(fileSystem.provider())) { - throw new ProviderMismatchException("Expected file system provider " + fileSystem.provider() + - " but " + path + " had " + path.getFileSystem().provider()); - } - - return path; - } - public static NodeAgentContextImpl.Builder builder(NodeSpec node) { return new Builder(new NodeSpec.Builder(node)); } @@ -219,16 +186,17 @@ public class NodeAgentContextImpl implements NodeAgentContext { /** For testing only! */ public static class Builder { + private static final Path DEFAULT_CONTAINER_STORAGE = Path.of("/data/vespa/storage"); + private NodeSpec.Builder nodeSpecBuilder; private Acl acl; private AthenzIdentity identity; private ContainerNetworkMode containerNetworkMode; private ZoneApi zone; private UserNamespace userNamespace; - private FileSystem fileSystem = FileSystems.getDefault(); + private Path containerStorage; private FlagSource flagSource; private double cpuSpeedUp = 1; - private Path containerStorage; private Optional hostExclusiveTo = Optional.empty(); private Builder(NodeSpec.Builder nodeSpecBuilder) { @@ -267,8 +235,7 @@ public class NodeAgentContextImpl implements NodeAgentContext { /** Sets the file system to use for paths. */ public Builder fileSystem(FileSystem fileSystem) { - this.fileSystem = fileSystem; - return this; + return containerStorage(fileSystem.getPath(DEFAULT_CONTAINER_STORAGE.toString())); } public Builder flagSource(FlagSource flagSource) { @@ -292,7 +259,7 @@ public class NodeAgentContextImpl implements NodeAgentContext { } public NodeAgentContextImpl build() { - return new NodeAgentContextImpl( + NodeAgentContextImpl context = new NodeAgentContextImpl( nodeSpecBuilder.build(), Optional.ofNullable(acl).orElse(Acl.EMPTY), Optional.ofNullable(identity).orElseGet(() -> new AthenzService("domain", "service")), @@ -318,12 +285,16 @@ public class NodeAgentContextImpl implements NodeAgentContext { return getId().region().value(); } }), - fileSystem, Optional.ofNullable(flagSource).orElseGet(InMemoryFlagSource::new), - Optional.ofNullable(containerStorage).orElseGet(() -> fileSystem.getPath("/data/vespa/storage")), - fileSystem.getPath("/opt/vespa"), - Optional.ofNullable(userNamespace).orElseGet(() -> new UserNamespace(10000, 10000, "vespa", "users", 1000, 100)), + Optional.ofNullable(containerStorage).orElseGet(() -> Path.of("/data/vespa/storage")), + "/opt/vespa", + Optional.ofNullable(userNamespace).orElseGet(() -> new UserNamespace(100000, 100000, "vespa", "vespa", 1000, 100)), cpuSpeedUp, hostExclusiveTo); + + if (containerStorage != null) + uncheck(() -> Files.createDirectories(context.containerPath("/").pathOnHost())); + + return context; } } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java index 391aa46716a..3ab196a052e 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImpl.java @@ -27,6 +27,7 @@ import com.yahoo.vespa.hosted.node.admin.maintenance.acl.AclMaintainer; import com.yahoo.vespa.hosted.node.admin.maintenance.identity.CredentialsMaintainer; import com.yahoo.vespa.hosted.node.admin.maintenance.servicedump.VespaServiceDumper; import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException; +import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; import java.nio.file.Path; import java.time.Clock; @@ -597,17 +598,17 @@ public class NodeAgentImpl implements NodeAgent { protected ContainerData createContainerData(NodeAgentContext context) { return new ContainerData() { @Override - public void addFile(Path pathInContainer, String data) { + public void addFile(ContainerPath path, String data) { throw new UnsupportedOperationException("addFile not implemented"); } @Override - public void addDirectory(Path pathInContainer) { + public void addDirectory(ContainerPath path) { throw new UnsupportedOperationException("addDirectory not implemented"); } @Override - public void createSymlink(Path symlink, Path target) { + public void createSymlink(ContainerPath symlink, Path target) { throw new UnsupportedOperationException("createSymlink not implemented"); } }; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/UserNamespace.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/UserNamespace.java index 5b53879ceec..2baa27ce70e 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/UserNamespace.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/UserNamespace.java @@ -6,49 +6,23 @@ package com.yahoo.vespa.hosted.node.admin.nodeagent; */ public class UserNamespace { - /** Total number of UID/GID that are mapped for each container */ - private static final int ID_RANGE = 1 << 16; - private final int uidOffset; private final int gidOffset; private final String vespaUser; private final String vespaGroup; - private final int vespaUserId; - private final int vespaGroupId; public UserNamespace(int uidOffset, int gidOffset, String vespaUser, String vespaGroup, int vespaUserId, int vespaGroupId) { this.uidOffset = uidOffset; this.gidOffset = gidOffset; this.vespaUser = vespaUser; this.vespaGroup = vespaGroup; - this.vespaUserId = vespaUserId; - this.vespaGroupId = vespaGroupId; - } - - public int userIdOnHost(int userIdInContainer) { - assertValidId("User", userIdInContainer); - return uidOffset + userIdInContainer; } - public int groupIdOnHost(int groupIdInContainer) { - assertValidId("Group", groupIdInContainer); - return gidOffset + groupIdInContainer; - } - - int rootUserId() { return 0; } - int rootGroupId() { return 0; } - int rootUserIdOnHost() { return userIdOnHost(rootUserId()); } - int rootGroupIdOnHost() { return groupIdOnHost(rootGroupId()); } + public int rootUserIdOnHost() { return uidOffset; } + public int rootGroupIdOnHost() { return gidOffset; } + /** Returns name of the user that runs vespa inside the container */ public String vespaUser() { return vespaUser; } + /** Returns name of the group of the user that runs vespa inside the container */ public String vespaGroup() { return vespaGroup; } - public int vespaUserId() { return vespaUserId; } - public int vespaGroupId() { return vespaGroupId; } - public int vespaUserIdOnHost() { return userIdOnHost(vespaUserId()); } - public int vespaGroupIdOnHost() { return groupIdOnHost(vespaGroupId()); } - - private static void assertValidId(String type, int id) { - if (id < 0) throw new IllegalArgumentException(type + " ID cannot be negative, was " + id); - if (id >= ID_RANGE) throw new IllegalArgumentException(type + " ID must be less than " + ID_RANGE + ", was " + id); - } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/package-info.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/package-info.java new file mode 100644 index 00000000000..a2a86604fdd --- /dev/null +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/package-info.java @@ -0,0 +1,5 @@ +// Copyright Yahoo. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. +@ExportPackage +package com.yahoo.vespa.hosted.node.admin.task.util.fs; + +import com.yahoo.osgi.annotation.ExportPackage; 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 b7b247bbf87..cd020d81450 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 @@ -11,6 +11,7 @@ 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.FileFinder; import com.yahoo.vespa.hosted.node.admin.task.util.file.DiskSize; +import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; import com.yahoo.vespa.hosted.node.admin.task.util.process.TestTerminal; import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.After; @@ -50,9 +51,8 @@ public class StorageMaintainerTest { fileSystem.getPath("/data/vespa/storage/container-archive")); @Test - public void testDiskUsed() throws IOException { + public void testDiskUsed() { NodeAgentContext context = NodeAgentContextImpl.builder("host-1.domain.tld").fileSystem(fileSystem).build(); - Files.createDirectories(context.pathOnHostFromPathInNode("/")); terminal.expectCommand("du -xsk /data/vespa/storage/host-1 2>&1", 0, "321\t/data/vespa/storage/host-1/"); assertEquals(Optional.of(DiskSize.of(328_704)), storageMaintainer.diskUsageFor(context)); @@ -76,7 +76,7 @@ public class StorageMaintainerTest { Path pathToArchiveDir = fileSystem.getPath("/data/vespa/storage/container-archive"); Files.createDirectories(pathToArchiveDir); - Path containerStorageRoot = context1.pathOnHostFromPathInNode("/").getParent(); + Path containerStorageRoot = context1.containerPath("/").pathOnHost().getParent(); Set containerStorageRootContentsBeforeArchive = FileFinder.from(containerStorageRoot) .maxDepth(1) .stream() @@ -115,21 +115,21 @@ public class StorageMaintainerTest { NodeAgentContext context = NodeAgentContextImpl.builder(containerName + ".domain.tld") .fileSystem(fileSystem).build(); - Path containerVespaHomeOnHost = context.pathOnHostFromPathInNode(context.pathInNodeUnderVespaHome("")); - Files.createDirectories(context.pathOnHostFromPathInNode("/etc/something")); - Files.createFile(context.pathOnHostFromPathInNode("/etc/something/conf")); + ContainerPath containerVespaHome = context.containerPathUnderVespaHome(""); + Files.createDirectories(context.containerPath("/etc/something")); + Files.createFile(context.containerPath("/etc/something/conf")); - Files.createDirectories(containerVespaHomeOnHost.resolve("logs/vespa")); - Files.createFile(containerVespaHomeOnHost.resolve("logs/vespa/vespa.log")); - Files.createFile(containerVespaHomeOnHost.resolve("logs/vespa/zookeeper.log")); + Files.createDirectories(containerVespaHome.resolve("logs/vespa")); + Files.createFile(containerVespaHome.resolve("logs/vespa/vespa.log")); + Files.createFile(containerVespaHome.resolve("logs/vespa/zookeeper.log")); - Files.createDirectories(containerVespaHomeOnHost.resolve("var/db")); - Files.createFile(containerVespaHomeOnHost.resolve("var/db/some-file")); + Files.createDirectories(containerVespaHome.resolve("var/db")); + Files.createFile(containerVespaHome.resolve("var/db/some-file")); - Path containerRootOnHost = context.pathOnHostFromPathInNode("/"); - Set actualContents = FileFinder.files(containerRootOnHost) + ContainerPath containerRoot = context.containerPath("/"); + Set actualContents = FileFinder.files(containerRoot) .stream() - .map(fileAttributes -> containerRootOnHost.relativize(fileAttributes.path()).toString()) + .map(fileAttributes -> containerRoot.relativize(fileAttributes.path()).toString()) .collect(Collectors.toSet()); Set expectedContents = Set.of( "etc/something/conf", @@ -145,7 +145,6 @@ public class StorageMaintainerTest { NodeAgentContext context = NodeAgentContextImpl.builder( NodeSpec.Builder.testSpec("h123a.domain.tld").realResources(new NodeResources(1, 1, 1, 1)).build()) .fileSystem(fileSystem).build(); - Files.createDirectories(context.pathOnHostFromPathInNode("/")); mockDiskUsage(500L); storageMaintainer.cleanDiskIfFull(context); @@ -158,7 +157,6 @@ public class StorageMaintainerTest { NodeSpec.Builder.testSpec("h123a.domain.tld").realResources(new NodeResources(1, 1, 1, 1)).build()) .fileSystem(fileSystem).build(); - Files.createDirectories(context.pathOnHostFromPathInNode("/")); mockDiskUsage(950_000L); storageMaintainer.cleanDiskIfFull(context); 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 eacaa038194..faea4c0bac9 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 @@ -4,10 +4,10 @@ package com.yahoo.vespa.hosted.node.admin.maintenance.coredump; 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; +import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult; import org.junit.Test; -import java.nio.file.Path; import java.util.List; import java.util.Map; @@ -29,7 +29,7 @@ public class CoreCollectorTest { private final CoreCollector coreCollector = new CoreCollector(docker); private final NodeAgentContext context = NodeAgentContextImpl.builder("container-123.domain.tld").build(); - private final Path TEST_CORE_PATH = Path.of("/tmp/core.1234"); + private final ContainerPath TEST_CORE_PATH = context.containerPath("/tmp/core.1234"); private final String TEST_BIN_PATH = "/usr/bin/program"; private final List GDB_BACKTRACE = List.of("[New Thread 2703]", "Core was generated by `/usr/bin/program\'.", "Program terminated with signal 11, Segmentation fault.", @@ -38,7 +38,7 @@ public class CoreCollectorTest { @Test public void extractsBinaryPathTest() { - final String[] cmd = {"file", TEST_CORE_PATH.toString()}; + final String[] cmd = {"file", TEST_CORE_PATH.pathInContainer()}; mockExec(cmd, "/tmp/core.1234: ELF 64-bit LSB core file x86-64, version 1 (SYSV), SVR4-style, from " + @@ -126,7 +126,7 @@ public class CoreCollectorTest { @Test public void collectsDataTest() { - mockExec(new String[]{"file", TEST_CORE_PATH.toString()}, + mockExec(new String[]{"file", TEST_CORE_PATH.pathInContainer()}, "/tmp/core.1234: ELF 64-bit LSB core file x86-64, version 1 (SYSV), SVR4-style, from " + "'/usr/bin/program'"); mockExec(new String[]{"stat", GDB_PATH_RHEL7_DT9}, "", "stat: No such file or directory"); @@ -146,7 +146,7 @@ public class CoreCollectorTest { @Test public void collectsPartialIfBacktraceFailsTest() { - mockExec(new String[]{"file", TEST_CORE_PATH.toString()}, + mockExec(new String[]{"file", TEST_CORE_PATH.pathInContainer()}, "/tmp/core.1234: ELF 64-bit LSB core file x86-64, version 1 (SYSV), SVR4-style, from " + "'/usr/bin/program'"); mockExec(new String[]{"stat", GDB_PATH_RHEL7_DT9}, "The stat output"); @@ -159,7 +159,7 @@ public class CoreCollectorTest { @Test public void reportsJstackInsteadOfGdbForJdkCores() { - mockExec(new String[]{"file", TEST_CORE_PATH.toString()}, + 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)"); mockExec(new String[]{"stat", GDB_PATH_RHEL7_DT9}, "", "stat: No such file or directory"); @@ -180,7 +180,7 @@ public class CoreCollectorTest { @Test public void metadata_for_java_heap_dump() { - assertEquals(JAVA_HEAP_DUMP_METADATA, coreCollector.collect(context, Path.of("dump_java_pid123.hprof"))); + assertEquals(JAVA_HEAP_DUMP_METADATA, coreCollector.collect(context, context.containerPath("/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 31883311e33..9f507f451b9 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 @@ -7,6 +7,7 @@ import com.yahoo.vespa.hosted.node.admin.container.metrics.Metrics; 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; +import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; import com.yahoo.vespa.hosted.node.admin.task.util.process.TestChildProcess2; import com.yahoo.vespa.hosted.node.admin.task.util.process.TestTerminal; import com.yahoo.vespa.test.file.TestFileSystem; @@ -47,7 +48,7 @@ public class CoredumpHandlerTest { private final FileSystem fileSystem = TestFileSystem.create(); private final NodeAgentContext context = NodeAgentContextImpl.builder("container-123.domain.tld") .fileSystem(fileSystem).build(); - private final Path crashPathInContainer = fileSystem.getPath("/var/crash"); + private final ContainerPath containerCrashPath = context.containerPath("/var/crash"); private final Path doneCoredumpsPath = fileSystem.getPath("/home/docker/dumps"); private final TestTerminal terminal = new TestTerminal(); @@ -58,38 +59,38 @@ public class CoredumpHandlerTest { @SuppressWarnings("unchecked") private final Supplier coredumpIdSupplier = mock(Supplier.class); private final CoredumpHandler coredumpHandler = new CoredumpHandler(terminal, coreCollector, coredumpReporter, - crashPathInContainer.toString(), doneCoredumpsPath, 100, metrics, clock, coredumpIdSupplier); + containerCrashPath.pathInContainer(), doneCoredumpsPath, 100, metrics, clock, coredumpIdSupplier); @Test public void coredump_enqueue_test() throws IOException { - final Path crashPathOnHost = fileSystem.getPath("/home/docker/container-1/some/crash/path"); - final Path processingDir = fileSystem.getPath("/home/docker/container-1/some/other/processing"); + ContainerPath crashPath = context.containerPath("/some/crash/path"); + ContainerPath processingDir = context.containerPath("/some/other/processing"); - Files.createDirectories(crashPathOnHost); - createFileAged(crashPathOnHost.resolve("bash.core.431"), Duration.ZERO); + Files.createDirectories(crashPath); + createFileAged(crashPath.resolve("bash.core.431"), Duration.ZERO); - assertFolderContents(crashPathOnHost, "bash.core.431"); - Optional enqueuedPath = coredumpHandler.enqueueCoredump(crashPathOnHost, processingDir); + assertFolderContents(crashPath, "bash.core.431"); + Optional enqueuedPath = coredumpHandler.enqueueCoredump(crashPath, processingDir); assertEquals(Optional.empty(), enqueuedPath); // bash.core.431 finished writing... and 2 more have since been written clock.advance(Duration.ofMinutes(3)); - createFileAged(crashPathOnHost.resolve("vespa-proton.core.119"), Duration.ofMinutes(10)); - createFileAged(crashPathOnHost.resolve("vespa-slobrok.core.673"), Duration.ofMinutes(5)); + createFileAged(crashPath.resolve("vespa-proton.core.119"), Duration.ofMinutes(10)); + createFileAged(crashPath.resolve("vespa-slobrok.core.673"), Duration.ofMinutes(5)); when(coredumpIdSupplier.get()).thenReturn("id-123").thenReturn("id-321"); - enqueuedPath = coredumpHandler.enqueueCoredump(crashPathOnHost, processingDir); + enqueuedPath = coredumpHandler.enqueueCoredump(crashPath, processingDir); assertEquals(Optional.of(processingDir.resolve("id-123")), enqueuedPath); - assertFolderContents(crashPathOnHost, "bash.core.431", "vespa-slobrok.core.673"); + assertFolderContents(crashPath, "bash.core.431", "vespa-slobrok.core.673"); assertFolderContents(processingDir, "id-123"); assertFolderContents(processingDir.resolve("id-123"), "dump_vespa-proton.core.119"); verify(coredumpIdSupplier, times(1)).get(); // Enqueue another - enqueuedPath = coredumpHandler.enqueueCoredump(crashPathOnHost, processingDir); + enqueuedPath = coredumpHandler.enqueueCoredump(crashPath, processingDir); assertEquals(Optional.of(processingDir.resolve("id-321")), enqueuedPath); - assertFolderContents(crashPathOnHost, "bash.core.431"); + assertFolderContents(crashPath, "bash.core.431"); assertFolderContents(processingDir, "id-123", "id-321"); assertFolderContents(processingDir.resolve("id-321"), "dump_vespa-slobrok.core.673"); verify(coredumpIdSupplier, times(2)).get(); @@ -97,46 +98,45 @@ public class CoredumpHandlerTest { @Test public void enqueue_with_hs_err_files() throws IOException { - final Path crashPathOnHost = fileSystem.getPath("/home/docker/container-1/some/crash/path"); - final Path processingDir = fileSystem.getPath("/home/docker/container-1/some/other/processing"); - Files.createDirectories(crashPathOnHost); + ContainerPath crashPath = context.containerPath("/some/crash/path"); + ContainerPath processingDir = context.containerPath("/some/other/processing"); + Files.createDirectories(crashPath); - createFileAged(crashPathOnHost.resolve("java.core.69"), Duration.ofSeconds(515)); - createFileAged(crashPathOnHost.resolve("hs_err_pid69.log"), Duration.ofSeconds(520)); + createFileAged(crashPath.resolve("java.core.69"), Duration.ofSeconds(515)); + createFileAged(crashPath.resolve("hs_err_pid69.log"), Duration.ofSeconds(520)); - createFileAged(crashPathOnHost.resolve("java.core.2420"), Duration.ofSeconds(540)); - createFileAged(crashPathOnHost.resolve("hs_err_pid2420.log"), Duration.ofSeconds(549)); - createFileAged(crashPathOnHost.resolve("hs_err_pid2421.log"), Duration.ofSeconds(550)); + createFileAged(crashPath.resolve("java.core.2420"), Duration.ofSeconds(540)); + createFileAged(crashPath.resolve("hs_err_pid2420.log"), Duration.ofSeconds(549)); + createFileAged(crashPath.resolve("hs_err_pid2421.log"), Duration.ofSeconds(550)); when(coredumpIdSupplier.get()).thenReturn("id-123").thenReturn("id-321"); - Optional enqueuedPath = coredumpHandler.enqueueCoredump(crashPathOnHost, processingDir); + Optional enqueuedPath = coredumpHandler.enqueueCoredump(crashPath, processingDir); assertEquals(Optional.of(processingDir.resolve("id-123")), enqueuedPath); - assertFolderContents(crashPathOnHost, "hs_err_pid69.log", "java.core.69"); + assertFolderContents(crashPath, "hs_err_pid69.log", "java.core.69"); assertFolderContents(processingDir, "id-123"); assertFolderContents(processingDir.resolve("id-123"), "hs_err_pid2420.log", "hs_err_pid2421.log", "dump_java.core.2420"); } @Test public void coredump_to_process_test() throws IOException { - final Path crashPathOnHost = fileSystem.getPath("/home/docker/container-1/some/crash/path"); - final Path processingDir = fileSystem.getPath("/home/docker/container-1/some/other/processing"); + ContainerPath processingDir = context.containerPath("/some/other/processing"); // Initially there are no core dumps - Optional enqueuedPath = coredumpHandler.enqueueCoredump(crashPathOnHost, processingDir); + Optional enqueuedPath = coredumpHandler.enqueueCoredump(containerCrashPath, processingDir); assertEquals(Optional.empty(), enqueuedPath); // 3 core dumps occur - Files.createDirectories(crashPathOnHost); - createFileAged(crashPathOnHost.resolve("bash.core.431"), Duration.ZERO); - createFileAged(crashPathOnHost.resolve("vespa-proton.core.119"), Duration.ofMinutes(10)); - createFileAged(crashPathOnHost.resolve("vespa-slobrok.core.673"), Duration.ofMinutes(5)); + Files.createDirectories(containerCrashPath); + createFileAged(containerCrashPath.resolve("bash.core.431"), Duration.ZERO); + createFileAged(containerCrashPath.resolve("vespa-proton.core.119"), Duration.ofMinutes(10)); + createFileAged(containerCrashPath.resolve("vespa-slobrok.core.673"), Duration.ofMinutes(5)); when(coredumpIdSupplier.get()).thenReturn("id-123"); - enqueuedPath = coredumpHandler.getCoredumpToProcess(crashPathOnHost, processingDir); + enqueuedPath = coredumpHandler.getCoredumpToProcess(containerCrashPath, processingDir); assertEquals(Optional.of(processingDir.resolve("id-123")), enqueuedPath); // Running this again wont enqueue new core dumps as we are still processing the one enqueued previously - enqueuedPath = coredumpHandler.getCoredumpToProcess(crashPathOnHost, processingDir); + enqueuedPath = coredumpHandler.getCoredumpToProcess(containerCrashPath, processingDir); assertEquals(Optional.of(processingDir.resolve("id-123")), enqueuedPath); verify(coredumpIdSupplier, times(1)).get(); } @@ -164,11 +164,10 @@ public class CoredumpHandlerTest { "}}"; - Path coredumpDirectoryInContainer = fileSystem.getPath("/var/crash/id-123"); - Path coredumpDirectory = context.pathOnHostFromPathInNode(coredumpDirectoryInContainer); - Files.createDirectories(coredumpDirectory); + ContainerPath coredumpDirectory = context.containerPath("/var/crash/id-123"); + Files.createDirectories(coredumpDirectory.pathOnHost()); Files.createFile(coredumpDirectory.resolve("dump_core.456")); - when(coreCollector.collect(eq(context), eq(coredumpDirectoryInContainer.resolve("dump_core.456")))) + when(coreCollector.collect(eq(context), eq(coredumpDirectory.resolve("dump_core.456")))) .thenReturn(metadata); assertEquals(expectedMetadataStr, coredumpHandler.getMetadata(context, coredumpDirectory, () -> attributes)); @@ -181,12 +180,12 @@ public class CoredumpHandlerTest { @Test(expected = IllegalStateException.class) public void cant_get_metadata_if_no_core_file() throws IOException { - coredumpHandler.getMetadata(context, fileSystem.getPath("/fake/path"), Map::of); + coredumpHandler.getMetadata(context, context.containerPath("/fake/path"), Map::of); } @Test(expected = IllegalStateException.class) public void fails_to_get_core_file_if_only_compressed() throws IOException { - Path coredumpDirectory = fileSystem.getPath("/path/to/coredump/proccessing/id-123"); + ContainerPath coredumpDirectory = context.containerPath("/path/to/coredump/proccessing/id-123"); Files.createDirectories(coredumpDirectory); Files.createFile(coredumpDirectory.resolve("dump_bash.core.431.lz4")); coredumpHandler.findCoredumpFileInProcessingDirectory(coredumpDirectory); @@ -194,14 +193,14 @@ public class CoredumpHandlerTest { @Test public void process_single_coredump_test() throws IOException { - Path coredumpDirectory = fileSystem.getPath("/path/to/coredump/proccessing/id-123"); + ContainerPath coredumpDirectory = context.containerPath("/path/to/coredump/proccessing/id-123"); Files.createDirectories(coredumpDirectory); Files.write(coredumpDirectory.resolve("metadata.json"), "metadata".getBytes()); Files.createFile(coredumpDirectory.resolve("dump_bash.core.431")); assertFolderContents(coredumpDirectory, "metadata.json", "dump_bash.core.431"); - terminal.interceptCommand("/usr/bin/lz4 -f /path/to/coredump/proccessing/id-123/dump_bash.core.431 " + - "/path/to/coredump/proccessing/id-123/dump_bash.core.431.lz4 2>&1", + terminal.interceptCommand("/usr/bin/lz4 -f /data/vespa/storage/container-123/path/to/coredump/proccessing/id-123/dump_bash.core.431 " + + "/data/vespa/storage/container-123/path/to/coredump/proccessing/id-123/dump_bash.core.431.lz4 2>&1", commandLine -> { uncheck(() -> Files.createFile(fileSystem.getPath(commandLine.getArguments().get(3)))); return new TestChildProcess2(0, ""); @@ -216,10 +215,10 @@ public class CoredumpHandlerTest { @Test public void report_enqueued_and_processed_metrics() throws IOException { - Path processingPath = crashPathInContainer.resolve("processing"); - Files.createFile(crashPathInContainer.resolve("dump-1")); - Files.createFile(crashPathInContainer.resolve("dump-2")); - Files.createFile(crashPathInContainer.resolve("hs_err_pid2.log")); + Path processingPath = containerCrashPath.resolve("processing"); + Files.createFile(containerCrashPath.resolve("dump-1")); + 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("dump-3")); @@ -228,7 +227,7 @@ public class CoredumpHandlerTest { .createParents() .createNewFile(); - coredumpHandler.updateMetrics(context, crashPathInContainer); + coredumpHandler.updateMetrics(context, containerCrashPath); List updatedMetrics = metrics.getMetricsByType(Metrics.DimensionType.PRETAGGED); assertEquals(1, updatedMetrics.size()); Map values = updatedMetrics.get(0).getMetrics(); @@ -238,7 +237,7 @@ public class CoredumpHandlerTest { @Before public void setup() throws IOException { - Files.createDirectories(crashPathInContainer); + Files.createDirectories(containerCrashPath.pathOnHost()); } @After diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java index b91afd0bfef..782dbf52aea 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java @@ -7,6 +7,7 @@ import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.Test; import java.nio.file.FileSystem; +import java.nio.file.Path; import java.util.List; import static org.junit.Assert.assertEquals; @@ -25,54 +26,54 @@ public class NodeAgentContextImplTest { public void path_on_host_from_path_in_node_test() { assertEquals( "/data/vespa/storage/container-1", - context.pathOnHostFromPathInNode("/").toString()); + context.containerPath("/").pathOnHost().toString()); assertEquals( "/data/vespa/storage/container-1/dev/null", - context.pathOnHostFromPathInNode("/dev/null").toString()); + context.containerPath("/dev/null").pathOnHost().toString()); } @Test(expected=IllegalArgumentException.class) public void path_in_container_must_be_absolute() { - context.pathOnHostFromPathInNode("some/relative/path"); + context.containerPath("some/relative/path"); } @Test public void path_in_node_from_path_on_host_test() { assertEquals( "/dev/null", - context.pathInNodeFromPathOnHost(fileSystem.getPath("/data/vespa/storage/container-1/dev/null")).toString()); + context.containerPathFromPathOnHost(fileSystem.getPath("/data/vespa/storage/container-1/dev/null")).pathInContainer()); } @Test(expected=IllegalArgumentException.class) public void path_on_host_must_be_absolute() { - context.pathInNodeFromPathOnHost("some/relative/path"); + context.containerPathFromPathOnHost(Path.of("some/relative/path")); } @Test(expected=IllegalArgumentException.class) public void path_on_host_must_be_inside_container_storage_of_context() { - context.pathInNodeFromPathOnHost(fileSystem.getPath("/data/vespa/storage/container-2/dev/null")); + context.containerPathFromPathOnHost(fileSystem.getPath("/data/vespa/storage/container-2/dev/null")); } @Test(expected=IllegalArgumentException.class) public void path_on_host_must_be_inside_container_storage() { - context.pathInNodeFromPathOnHost(fileSystem.getPath("/home")); + context.containerPathFromPathOnHost(fileSystem.getPath("/home")); } @Test public void path_under_vespa_host_in_container_test() { assertEquals( "/opt/vespa", - context.pathInNodeUnderVespaHome("").toString()); + context.containerPathUnderVespaHome("").pathInContainer()); assertEquals( "/opt/vespa/logs/vespa/vespa.log", - context.pathInNodeUnderVespaHome("logs/vespa/vespa.log").toString()); + context.containerPathUnderVespaHome("logs/vespa/vespa.log").pathInContainer()); } @Test(expected=IllegalArgumentException.class) public void path_under_vespa_home_must_be_relative() { - context.pathInNodeUnderVespaHome("/home"); + context.containerPathUnderVespaHome("/home"); } @Test -- cgit v1.2.3 From 7f1f9e3d1d17d176c13d183d4131f58c808c2141 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Thu, 14 Oct 2021 16:55:49 +0200 Subject: Create container storage root directories when creating Container FS --- .../hosted/node/admin/nodeagent/NodeAgentContextImpl.java | 14 ++++---------- .../node/admin/task/util/fs/ContainerFileSystem.java | 4 ++++ .../hosted/node/admin/integration/ContainerFailTest.java | 3 ++- .../node/admin/maintenance/coredump/CoreCollectorTest.java | 4 +++- .../hosted/node/admin/nodeadmin/NodeAdminImplTest.java | 3 ++- .../node/admin/nodeagent/NodeAgentContextImplTest.java | 4 ++-- .../node/admin/nodeagent/NodeAgentContextManagerTest.java | 3 ++- .../hosted/node/admin/nodeagent/NodeAgentImplTest.java | 7 +++++-- 8 files changed, 24 insertions(+), 18 deletions(-) (limited to 'node-admin') 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 899df091e7a..3b1235d1ccf 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 @@ -20,7 +20,6 @@ import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerFileSystem; import com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerPath; import java.nio.file.FileSystem; -import java.nio.file.Files; import java.nio.file.Path; import java.util.Objects; import java.util.Optional; @@ -29,8 +28,6 @@ import java.util.function.Function; import java.util.logging.Level; import java.util.logging.Logger; -import static com.yahoo.yolean.Exceptions.uncheck; - /** * @author freva */ @@ -259,7 +256,9 @@ public class NodeAgentContextImpl implements NodeAgentContext { } public NodeAgentContextImpl build() { - NodeAgentContextImpl context = new NodeAgentContextImpl( + Objects.requireNonNull(containerStorage, "Must set one of containerStorage or fileSystem"); + + return new NodeAgentContextImpl( nodeSpecBuilder.build(), Optional.ofNullable(acl).orElse(Acl.EMPTY), Optional.ofNullable(identity).orElseGet(() -> new AthenzService("domain", "service")), @@ -286,15 +285,10 @@ public class NodeAgentContextImpl implements NodeAgentContext { } }), Optional.ofNullable(flagSource).orElseGet(InMemoryFlagSource::new), - Optional.ofNullable(containerStorage).orElseGet(() -> Path.of("/data/vespa/storage")), + containerStorage, "/opt/vespa", Optional.ofNullable(userNamespace).orElseGet(() -> new UserNamespace(100000, 100000, "vespa", "vespa", 1000, 100)), cpuSpeedUp, hostExclusiveTo); - - if (containerStorage != null) - uncheck(() -> Files.createDirectories(context.containerPath("/").pathOnHost())); - - return context; } } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystem.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystem.java index 495b72e4554..36edfa1c1ee 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystem.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystem.java @@ -4,12 +4,15 @@ package com.yahoo.vespa.hosted.node.admin.task.util.fs; import java.io.IOException; import java.nio.file.FileStore; import java.nio.file.FileSystem; +import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.PathMatcher; import java.nio.file.WatchService; import java.nio.file.attribute.UserPrincipalLookupService; import java.util.Set; +import static com.yahoo.yolean.Exceptions.uncheck; + /** * @author valerijf */ @@ -82,6 +85,7 @@ public class ContainerFileSystem extends FileSystem { } public static ContainerFileSystem create(Path containerStorageRoot, int uidOffset, int gidOffset) { + uncheck(() -> Files.createDirectories(containerStorageRoot)); return new ContainerFileSystemProvider(containerStorageRoot, uidOffset, gidOffset).getFileSystem(null); } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/ContainerFailTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/ContainerFailTest.java index 8683c2cb417..a2dbfe0db4b 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/ContainerFailTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/integration/ContainerFailTest.java @@ -6,6 +6,7 @@ import com.yahoo.vespa.hosted.node.admin.container.ContainerName; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl; +import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.Test; import java.util.List; @@ -33,7 +34,7 @@ public class ContainerFailTest { .build(); tester.addChildNodeRepositoryNode(nodeSpec); - NodeAgentContext context = NodeAgentContextImpl.builder(nodeSpec).build(); + NodeAgentContext context = NodeAgentContextImpl.builder(nodeSpec).fileSystem(TestFileSystem.create()).build(); tester.inOrder(tester.containerOperations).createContainer(containerMatcher(containerName), any(), any()); tester.inOrder(tester.containerOperations).resumeNode(containerMatcher(containerName)); 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 faea4c0bac9..ded99cf3778 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 @@ -6,6 +6,7 @@ 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.fs.ContainerPath; import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult; +import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.Test; import java.util.List; @@ -27,7 +28,8 @@ 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").build(); + private final NodeAgentContext context = NodeAgentContextImpl.builder("container-123.domain.tld") + .fileSystem(TestFileSystem.create()).build(); private final ContainerPath TEST_CORE_PATH = context.containerPath("/tmp/core.1234"); private final String TEST_BIN_PATH = "/usr/bin/program"; diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java index 232b95f5ede..f8dc05b5eda 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeadmin/NodeAdminImplTest.java @@ -5,6 +5,7 @@ import com.yahoo.test.ManualClock; import com.yahoo.vespa.hosted.node.admin.container.metrics.Metrics; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl; +import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.Test; import org.mockito.InOrder; @@ -154,7 +155,7 @@ public class NodeAdminImplTest { } private NodeAgentContext createNodeAgentContext(String hostname) { - return NodeAgentContextImpl.builder(hostname).build(); + return NodeAgentContextImpl.builder(hostname).fileSystem(TestFileSystem.create()).build(); } private NodeAgentWithScheduler mockNodeAgentWithSchedulerFactory(NodeAgentContext context) { diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java index 782dbf52aea..1c439234a34 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextImplTest.java @@ -87,9 +87,9 @@ public class NodeAgentContextImplTest { assertTrue(context2.isDisabled(NodeAgentTask.CoreDumps)); } - private static NodeAgentContext createContextWithDisabledTasks(String... tasks) { + private NodeAgentContext createContextWithDisabledTasks(String... tasks) { InMemoryFlagSource flagSource = new InMemoryFlagSource(); flagSource.withListFlag(PermanentFlags.DISABLED_HOST_ADMIN_TASKS.id(), List.of(tasks), String.class); - return NodeAgentContextImpl.builder("node123").flagSource(flagSource).build(); + return NodeAgentContextImpl.builder("node123").fileSystem(fileSystem).flagSource(flagSource).build(); } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManagerTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManagerTest.java index 51fedd54381..197b1c81b57 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManagerTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentContextManagerTest.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.nodeagent; +import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.Test; import java.time.Clock; @@ -142,7 +143,7 @@ public class NodeAgentContextManagerTest { } private static NodeAgentContext generateContext() { - return NodeAgentContextImpl.builder("container-123.domain.tld").build(); + return NodeAgentContextImpl.builder("container-123.domain.tld").fileSystem(TestFileSystem.create()).build(); } private static class AsyncExecutor { diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java index 8764db502bb..1c6831e6379 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/NodeAgentImplTest.java @@ -27,10 +27,12 @@ import com.yahoo.vespa.hosted.node.admin.maintenance.acl.AclMaintainer; import com.yahoo.vespa.hosted.node.admin.maintenance.identity.CredentialsMaintainer; import com.yahoo.vespa.hosted.node.admin.maintenance.servicedump.VespaServiceDumper; import com.yahoo.vespa.hosted.node.admin.nodeadmin.ConvergenceException; +import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.Before; import org.junit.Test; import org.mockito.InOrder; +import java.nio.file.FileSystem; import java.time.Duration; import java.time.Instant; import java.util.List; @@ -72,6 +74,7 @@ public class NodeAgentImplTest { private final CredentialsMaintainer credentialsMaintainer = mock(CredentialsMaintainer.class); private final InMemoryFlagSource flagSource = new InMemoryFlagSource(); private final ManualClock clock = new ManualClock(Instant.now()); + private final FileSystem fileSystem = TestFileSystem.create(); @Before public void setUp() { @@ -233,7 +236,7 @@ public class NodeAgentImplTest { nodeAgent.doConverge(secondContext); inOrder.verify(orchestrator, never()).resume(any(String.class)); - NodeAgentContext thirdContext = NodeAgentContextImpl.builder(specBuilder.vcpu(5).build()).cpuSpeedUp(1.25).build(); + NodeAgentContext thirdContext = NodeAgentContextImpl.builder(specBuilder.vcpu(5).build()).fileSystem(fileSystem).cpuSpeedUp(1.25).build(); nodeAgent.doConverge(thirdContext); ContainerResources resourcesAfterThird = ContainerResources.from(0, 4, 16); mockGetContainer(dockerImage, resourcesAfterThird, true); @@ -777,7 +780,7 @@ public class NodeAgentImplTest { } private NodeAgentContext createContext(NodeSpec nodeSpec) { - return NodeAgentContextImpl.builder(nodeSpec).build(); + return NodeAgentContextImpl.builder(nodeSpec).fileSystem(fileSystem).build(); } private NodeSpec.Builder nodeBuilder(NodeState state) { -- cgit v1.2.3 From fa2aa2c40a3028752d861592f4a9b37982b80797 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Fri, 15 Oct 2021 11:48:53 +0200 Subject: Add support for symlinks --- .../task/util/fs/ContainerFileSystemProvider.java | 16 +++++++- .../task/util/fs/ContainerFileSystemTest.java | 46 ++++++++++++++++++++++ 2 files changed, 61 insertions(+), 1 deletion(-) (limited to 'node-admin') diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java index 73cd3f8cfc5..a85e4ee8699 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java @@ -120,6 +120,20 @@ class ContainerFileSystemProvider extends FileSystemProvider { fixOwnerToContainerRoot(toContainerPath(target)); } + @Override + public void createSymbolicLink(Path link, Path target, FileAttribute... attrs) throws IOException { + Path pathOnHost = pathOnHost(link); + if (target instanceof ContainerPath) + target = pathOnHost.getFileSystem().getPath(toContainerPath(target).pathInContainer()); + provider(pathOnHost).createSymbolicLink(pathOnHost, target, attrs); + } + + @Override + public Path readSymbolicLink(Path link) throws IOException { + Path pathOnHost = pathOnHost(link); + return provider(pathOnHost).readSymbolicLink(pathOnHost); + } + @Override public boolean isSameFile(Path path, Path path2) throws IOException { // 'path' FS provider should be 'this' @@ -250,7 +264,7 @@ class ContainerFileSystemProvider extends FileSystemProvider { private static T cast(Object value, Class type) { if (type.isInstance(value)) return type.cast(value); - throw new ProviderMismatchException("Expected " + type.getName() + ", was " + value.getClass().getName()); + throw new ProviderMismatchException("Expected " + type.getSimpleName() + ", was " + value.getClass().getName()); } private static Path pathOnHost(Path path) { diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java index eb15450b756..fcd8e0ec406 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java @@ -9,6 +9,8 @@ import java.io.IOException; import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; +import java.nio.file.StandardCopyOption; +import java.nio.file.StandardOpenOption; import java.util.Map; import static com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerUserPrincipalLookupService.OVERFLOW_ID; @@ -52,6 +54,21 @@ class ContainerFileSystemTest { Files.copy(hostFile.toPath(), destination); assertOwnership(destination, 0, 0, 10000, 11000); + + // Set owner + group on both source host file and destination container file + hostFile.setOwnerId(5).setGroupId(10); + new UnixPath(destination).setOwnerId(500).setGroupId(200); + assertOwnership(destination, 500, 200, 10500, 11200); + // Copy the host file to destination again with COPY_ATTRIBUTES and REPLACE_EXISTING + Files.copy(hostFile.toPath(), destination, StandardCopyOption.COPY_ATTRIBUTES, StandardCopyOption.REPLACE_EXISTING); + // The destination is recreated, so the owner should be root + assertOwnership(destination, 0, 0, 10000, 11000); + + // Set owner + group and copy within ContainerFS + new UnixPath(destination).setOwnerId(500).setGroupId(200); + ContainerPath destination2 = ContainerPath.fromPathInContainer(containerFs, Path.of("/dest2")); + Files.copy(destination, destination2, StandardCopyOption.COPY_ATTRIBUTES, StandardCopyOption.REPLACE_EXISTING); + assertOwnership(destination2, 0, 0, 10000, 11000); } @Test @@ -67,6 +84,35 @@ class ContainerFileSystemTest { hostFile.createNewFile(); Files.move(hostFile.toPath(), destination); assertOwnership(destination, 0, 0, 10000, 11000); + + // Set owner + group on both source host file and destination container file + hostFile.createNewFile(); + hostFile.setOwnerId(5).setGroupId(10); + new UnixPath(destination).setOwnerId(500).setGroupId(200); + assertOwnership(destination, 500, 200, 10500, 11200); + // Move the host file to destination again with COPY_ATTRIBUTES and REPLACE_EXISTING + Files.move(hostFile.toPath(), destination, StandardCopyOption.COPY_ATTRIBUTES, StandardCopyOption.REPLACE_EXISTING); + // The destination is recreated, so the owner should be root + assertOwnership(destination, 0, 0, 10000, 11000); + + // Set owner + group and move within ContainerFS + new UnixPath(destination).setOwnerId(500).setGroupId(200); + ContainerPath destination2 = ContainerPath.fromPathInContainer(containerFs, Path.of("/dest2")); + Files.move(destination, destination2, StandardCopyOption.COPY_ATTRIBUTES, StandardCopyOption.REPLACE_EXISTING); + assertOwnership(destination2, 0, 0, 10000, 11000); + } + + @Test + public void symlink() throws IOException { + ContainerPath source = ContainerPath.fromPathInContainer(containerFs, Path.of("/src")); + // Symlink from ContainerPath to some relative path (different FS provider) + Files.createSymbolicLink(source, fileSystem.getPath("../relative/target")); + assertEquals(fileSystem.getPath("../relative/target"), Files.readSymbolicLink(source)); + Files.delete(source); + + // Symlinks from ContainerPath to a ContainerPath: Target is resolved within container with base FS provider + Files.createSymbolicLink(source, ContainerPath.fromPathInContainer(containerFs, Path.of("/path/in/container"))); + assertEquals(fileSystem.getPath("/path/in/container"), Files.readSymbolicLink(source)); } private static void assertOwnership(ContainerPath path, int contUid, int contGid, int hostUid, int hostGid) throws IOException { -- cgit v1.2.3 From 03a710a40926a3ea80072cc8676f4edd18662f84 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Fri, 15 Oct 2021 11:49:11 +0200 Subject: Do not chown to root if file already existed --- .../admin/task/util/fs/ContainerFileSystemProvider.java | 7 +++++-- .../node/admin/task/util/fs/ContainerFileSystemTest.java | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) (limited to 'node-admin') diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java index a85e4ee8699..cf1985eff58 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java @@ -10,6 +10,7 @@ import java.nio.file.DirectoryStream; import java.nio.file.FileStore; import java.nio.file.FileSystem; import java.nio.file.FileSystemAlreadyExistsException; +import java.nio.file.Files; import java.nio.file.LinkOption; import java.nio.file.OpenOption; import java.nio.file.Path; @@ -80,8 +81,9 @@ class ContainerFileSystemProvider extends FileSystemProvider { @Override public SeekableByteChannel newByteChannel(Path path, Set options, FileAttribute... attrs) throws IOException { Path pathOnHost = pathOnHost(path); + boolean existedBefore = Files.exists(pathOnHost); SeekableByteChannel seekableByteChannel = provider(pathOnHost).newByteChannel(pathOnHost, options, attrs); - fixOwnerToContainerRoot(toContainerPath(path)); + if (!existedBefore) fixOwnerToContainerRoot(toContainerPath(path)); return seekableByteChannel; } @@ -94,8 +96,9 @@ class ContainerFileSystemProvider extends FileSystemProvider { @Override public void createDirectory(Path dir, FileAttribute... attrs) throws IOException { Path pathOnHost = pathOnHost(dir); + boolean existedBefore = Files.exists(pathOnHost); provider(pathOnHost).createDirectory(pathOnHost); - fixOwnerToContainerRoot(toContainerPath(dir)); + if (!existedBefore) fixOwnerToContainerRoot(toContainerPath(dir)); } @Override diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java index fcd8e0ec406..970a264d0df 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java @@ -6,6 +6,7 @@ import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.jupiter.api.Test; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.nio.file.FileSystem; import java.nio.file.Files; import java.nio.file.Path; @@ -42,6 +43,21 @@ class ContainerFileSystemTest { assertOwnership(destination, 0, 0, 10000, 11000); } + @Test + public void file_write_and_read() throws IOException { + ContainerPath containerPath = ContainerPath.fromPathInContainer(containerFs, Path.of("/file")); + UnixPath unixPath = new UnixPath(containerPath); + unixPath.writeUtf8File("hello"); + assertOwnership(containerPath, 0, 0, 10000, 11000); + + unixPath.setOwnerId(500).setGroupId(200); + assertOwnership(containerPath, 500, 200, 10500, 11200); + Files.write(containerPath, " world".getBytes(StandardCharsets.UTF_8), StandardOpenOption.APPEND); + assertOwnership(containerPath, 500, 200, 10500, 11200); // Owner should not have been updated as the file already existed + + assertEquals("hello world", unixPath.readUtf8File()); + } + @Test public void copy() throws IOException { UnixPath hostFile = new UnixPath(fileSystem.getPath("/file")).createNewFile(); -- cgit v1.2.3 From 95ee45325f25117fcda801ce0066cf66d6167a5a Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Fri, 15 Oct 2021 13:50:11 +0200 Subject: Use UserNamespace in ContainerUserPrincipalLookupService --- .../node/admin/nodeagent/NodeAgentContextImpl.java | 2 +- .../hosted/node/admin/nodeagent/UserNamespace.java | 42 ++++++++-- .../admin/task/util/fs/ContainerFileSystem.java | 6 +- .../task/util/fs/ContainerFileSystemProvider.java | 18 +++-- .../fs/ContainerUserPrincipalLookupService.java | 89 ++++++++++------------ .../node/admin/nodeagent/UserNamespaceTest.java | 29 +++++++ .../task/util/fs/ContainerFileSystemTest.java | 9 ++- .../node/admin/task/util/fs/ContainerPathTest.java | 4 +- .../ContainerUserPrincipalLookupServiceTest.java | 30 +++----- 9 files changed, 137 insertions(+), 92 deletions(-) create mode 100644 node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/UserNamespaceTest.java (limited to 'node-admin') 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 3b1235d1ccf..9bcf5d58d6e 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 @@ -61,7 +61,7 @@ public class NodeAgentContextImpl implements NodeAgentContext { this.identity = Objects.requireNonNull(identity); this.containerNetworkMode = Objects.requireNonNull(containerNetworkMode); this.zone = Objects.requireNonNull(zone); - this.containerFs = ContainerFileSystem.create(pathToContainerStorage.resolve(containerName.asString()), userNamespace.rootUserIdOnHost(), userNamespace.rootGroupIdOnHost()); + this.containerFs = ContainerFileSystem.create(pathToContainerStorage.resolve(containerName.asString()), userNamespace); this.pathToVespaHome = containerFs.getPath(pathToVespaHome); this.logPrefix = containerName.asString() + ": "; this.userNamespace = Objects.requireNonNull(userNamespace); diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/UserNamespace.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/UserNamespace.java index 2baa27ce70e..1a25b5c3c5e 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/UserNamespace.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/nodeagent/UserNamespace.java @@ -1,28 +1,58 @@ // 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.nodeagent; +import java.util.Objects; + /** * @author valerijf */ public class UserNamespace { + /** Total number of UID/GID that are mapped for each container */ + private static final int ID_RANGE = 65_536; // 2^16 + + /** + * IDs outside the ID range are translated to the overflow ID before being written to disk: + * https://github.com/torvalds/linux/blob/5bfc75d92efd494db37f5c4c173d3639d4772966/Documentation/admin-guide/sysctl/fs.rst#overflowgid--overflowuid */ + private static final int OVERFLOW_ID = 65_534; + private final int uidOffset; private final int gidOffset; private final String vespaUser; private final String vespaGroup; + private final int vespaUserId; + private final int vespaGroupId; public UserNamespace(int uidOffset, int gidOffset, String vespaUser, String vespaGroup, int vespaUserId, int vespaGroupId) { this.uidOffset = uidOffset; this.gidOffset = gidOffset; - this.vespaUser = vespaUser; - this.vespaGroup = vespaGroup; + this.vespaUser = Objects.requireNonNull(vespaUser); + this.vespaGroup = Objects.requireNonNull(vespaGroup); + this.vespaUserId = vespaUserId; + this.vespaGroupId = vespaGroupId; } - public int rootUserIdOnHost() { return uidOffset; } - public int rootGroupIdOnHost() { return gidOffset; } + public int userIdOnHost(int containerUid) { return toHostId(containerUid, uidOffset); } + public int groupIdOnHost(int containerGid) { return toHostId(containerGid, gidOffset); } + public int userIdInContainer(int hostUid) { return toContainerId(hostUid, uidOffset); } + public int groupIdInContainer(int hostGid) { return toContainerId(hostGid, gidOffset); } - /** Returns name of the user that runs vespa inside the container */ public String vespaUser() { return vespaUser; } - /** Returns name of the group of the user that runs vespa inside the container */ public String vespaGroup() { return vespaGroup; } + public int vespaUserId() { return vespaUserId; } + public int vespaGroupId() { return vespaGroupId; } + + public int idRange() { return ID_RANGE; } + public int overflowId() { return OVERFLOW_ID; } + + private static int toHostId(int containerId, int idOffset) { + if (containerId < 0 || containerId > ID_RANGE) + throw new IllegalArgumentException("Invalid container id: " + containerId); + return idOffset + containerId; + } + + private static int toContainerId(int hostId, int idOffset) { + hostId = hostId - idOffset; + return hostId < 0 || hostId >= ID_RANGE ? OVERFLOW_ID : hostId; + } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystem.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystem.java index 36edfa1c1ee..078a60ba7a5 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystem.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystem.java @@ -1,6 +1,8 @@ // 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.task.util.fs; +import com.yahoo.vespa.hosted.node.admin.nodeagent.UserNamespace; + import java.io.IOException; import java.nio.file.FileStore; import java.nio.file.FileSystem; @@ -84,8 +86,8 @@ public class ContainerFileSystem extends FileSystem { throw new UnsupportedOperationException(); } - public static ContainerFileSystem create(Path containerStorageRoot, int uidOffset, int gidOffset) { + public static ContainerFileSystem create(Path containerStorageRoot, UserNamespace userNamespace) { uncheck(() -> Files.createDirectories(containerStorageRoot)); - return new ContainerFileSystemProvider(containerStorageRoot, uidOffset, gidOffset).getFileSystem(null); + return new ContainerFileSystemProvider(containerStorageRoot, userNamespace).getFileSystem(null); } } diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java index cf1985eff58..a44f90b164b 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemProvider.java @@ -1,6 +1,8 @@ // 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.task.util.fs; +import com.yahoo.vespa.hosted.node.admin.nodeagent.UserNamespace; + import java.io.IOException; import java.net.URI; import java.nio.channels.SeekableByteChannel; @@ -43,10 +45,10 @@ class ContainerFileSystemProvider extends FileSystemProvider { private final ContainerUserPrincipalLookupService userPrincipalLookupService; private final Path containerRootOnHost; - ContainerFileSystemProvider(Path containerRootOnHost, int uidOffset, int gidOffset) { + ContainerFileSystemProvider(Path containerRootOnHost, UserNamespace userNamespace) { this.containerFs = new ContainerFileSystem(this); this.userPrincipalLookupService = new ContainerUserPrincipalLookupService( - containerRootOnHost.getFileSystem().getUserPrincipalLookupService(), uidOffset, gidOffset); + containerRootOnHost.getFileSystem().getUserPrincipalLookupService(), userNamespace); this.containerRootOnHost = containerRootOnHost; } @@ -197,12 +199,12 @@ class ContainerFileSystemProvider extends FileSystemProvider { return provider(pathOnHost).readAttributes(pathOnHost, attributes, options); Map attrs = new HashMap<>(provider(pathOnHost).readAttributes(pathOnHost, "unix:*", options)); - int uid = userPrincipalLookupService.hostUidToContainerUid((int) attrs.get("uid")); - int gid = userPrincipalLookupService.hostGidToContainerGid((int) attrs.get("gid")); + int uid = userPrincipalLookupService.userIdInContainer((int) attrs.get("uid")); + int gid = userPrincipalLookupService.groupIdInContainer((int) attrs.get("gid")); attrs.put("uid", uid); attrs.put("gid", gid); - attrs.put("owner", new ContainerUserPrincipal(uid, (UserPrincipal) attrs.get("owner"))); - attrs.put("group", new ContainerGroupPrincipal(gid, (GroupPrincipal) attrs.get("group"))); + attrs.put("owner", userPrincipalLookupService.userPrincipal(uid, (UserPrincipal) attrs.get("owner"))); + attrs.put("group", userPrincipalLookupService.groupPrincipal(gid, (GroupPrincipal) attrs.get("group"))); return attrs; } @@ -218,8 +220,8 @@ class ContainerFileSystemProvider extends FileSystemProvider { switch (attribute.substring(index + 1)) { case "owner": return cast(value, ContainerUserPrincipal.class).baseFsPrincipal(); case "group": return cast(value, ContainerGroupPrincipal.class).baseFsPrincipal(); - case "uid": return userPrincipalLookupService.containerUidToHostUid(cast(value, Integer.class)); - case "gid": return userPrincipalLookupService.containerGidToHostGid(cast(value, Integer.class)); + case "uid": return userPrincipalLookupService.userIdOnHost(cast(value, Integer.class)); + case "gid": return userPrincipalLookupService.groupIdOnHost(cast(value, Integer.class)); } } // else basic file attribute return value; diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerUserPrincipalLookupService.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerUserPrincipalLookupService.java index 893e86ca239..ae65f6a7f7f 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerUserPrincipalLookupService.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerUserPrincipalLookupService.java @@ -1,7 +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.task.util.fs; -import com.google.common.collect.ImmutableBiMap; +import com.yahoo.vespa.hosted.node.admin.nodeagent.UserNamespace; import java.io.IOException; import java.nio.file.attribute.GroupPrincipal; @@ -9,58 +9,60 @@ import java.nio.file.attribute.UserPrincipal; import java.nio.file.attribute.UserPrincipalLookupService; import java.nio.file.attribute.UserPrincipalNotFoundException; import java.util.Objects; -import java.util.Optional; /** * @author valerijf */ class ContainerUserPrincipalLookupService extends UserPrincipalLookupService { - /** Total number of UID/GID that are mapped for each container */ - private static final int ID_RANGE = 1 << 16; - - /** - * IDs outside the ID range are translated to the overflow ID before being written to disk: - * https://github.com/torvalds/linux/blob/5bfc75d92efd494db37f5c4c173d3639d4772966/Documentation/admin-guide/sysctl/fs.rst#overflowgid--overflowuid */ - static final int OVERFLOW_ID = 65_534; - - private static final ImmutableBiMap CONTAINER_IDS_BY_NAME = ImmutableBiMap.builder() - .put("root", 0) - .put("vespa", 1000) - .build(); - private final UserPrincipalLookupService baseFsUserPrincipalLookupService; - private final int uidOffset; - private final int gidOffset; + private final UserNamespace userNamespace; - ContainerUserPrincipalLookupService(UserPrincipalLookupService baseFsUserPrincipalLookupService, int uidOffset, int gidOffset) { - this.baseFsUserPrincipalLookupService = baseFsUserPrincipalLookupService; - this.uidOffset = uidOffset; - this.gidOffset = gidOffset; + ContainerUserPrincipalLookupService(UserPrincipalLookupService baseFsUserPrincipalLookupService, UserNamespace userNamespace) { + this.baseFsUserPrincipalLookupService = Objects.requireNonNull(baseFsUserPrincipalLookupService); + this.userNamespace = Objects.requireNonNull(userNamespace); } - public int containerUidToHostUid(int containerUid) { return containerIdToHostId(containerUid, uidOffset); } - public int containerGidToHostGid(int containerGid) { return containerIdToHostId(containerGid, gidOffset); } - public int hostUidToContainerUid(int hostUid) { return hostIdToContainerId(hostUid, uidOffset); } - public int hostGidToContainerGid(int hostGid) { return hostIdToContainerId(hostGid, gidOffset); } + public int userIdOnHost(int containerUid) { return userNamespace.userIdOnHost(containerUid); } + public int groupIdOnHost(int containerGid) { return userNamespace.groupIdOnHost(containerGid); } + public int userIdInContainer(int hostUid) { return userNamespace.userIdInContainer(hostUid); } + public int groupIdInContainer(int hostGid) { return userNamespace.groupIdInContainer(hostGid); } @Override public ContainerUserPrincipal lookupPrincipalByName(String name) throws IOException { - int containerUid = resolve(name); - String hostUid = String.valueOf(containerUidToHostUid(containerUid)); - return new ContainerUserPrincipal(containerUid, baseFsUserPrincipalLookupService.lookupPrincipalByName(hostUid)); + int containerUid = resolveName(name, userNamespace.vespaUser(), userNamespace.vespaUserId()); + String user = resolveId(containerUid, userNamespace.vespaUser(), userNamespace.vespaUserId()); + String hostUid = String.valueOf(userIdOnHost(containerUid)); + return new ContainerUserPrincipal(containerUid, user, baseFsUserPrincipalLookupService.lookupPrincipalByName(hostUid)); } @Override public ContainerGroupPrincipal lookupPrincipalByGroupName(String group) throws IOException { - int containerGid = resolve(group); - String hostGid = String.valueOf(containerGidToHostGid(containerGid)); - return new ContainerGroupPrincipal(containerGid, baseFsUserPrincipalLookupService.lookupPrincipalByGroupName(hostGid)); + int containerGid = resolveName(group, userNamespace.vespaGroup(), userNamespace.vespaGroupId()); + String name = resolveId(containerGid, userNamespace.vespaGroup(), userNamespace.vespaGroupId()); + String hostGid = String.valueOf(groupIdOnHost(containerGid)); + return new ContainerGroupPrincipal(containerGid, name, baseFsUserPrincipalLookupService.lookupPrincipalByGroupName(hostGid)); + } + + public ContainerUserPrincipal userPrincipal(int uid, UserPrincipal baseFsPrincipal) { + String name = resolveId(uid, userNamespace.vespaUser(), userNamespace.vespaUserId()); + return new ContainerUserPrincipal(uid, name, baseFsPrincipal); + } + + public ContainerGroupPrincipal groupPrincipal(int gid, GroupPrincipal baseFsPrincipal) { + String name = resolveId(gid, userNamespace.vespaGroup(), userNamespace.vespaGroupId()); + return new ContainerGroupPrincipal(gid, name, baseFsPrincipal); } - private static int resolve(String name) throws UserPrincipalNotFoundException { - Integer id = CONTAINER_IDS_BY_NAME.get(name); - if (id != null) return id; + private String resolveId(int id, String vespaName, int vespaId) { + if (id == 0) return "root"; + if (id == vespaId) return vespaName; + return String.valueOf(id); + } + + private int resolveName(String name, String vespaName, int vespaId) throws UserPrincipalNotFoundException { + if (name.equals("root")) return 0; + if (name.equals(vespaName)) return vespaId; try { return Integer.parseInt(name); @@ -74,9 +76,9 @@ class ContainerUserPrincipalLookupService extends UserPrincipalLookupService { private final String name; private final UserPrincipal baseFsPrincipal; - private NamedPrincipal(int id, UserPrincipal baseFsPrincipal) { + private NamedPrincipal(int id, String name, UserPrincipal baseFsPrincipal) { this.id = id; - this.name = Optional.ofNullable(CONTAINER_IDS_BY_NAME.inverse().get(id)).orElseGet(() -> Integer.toString(id)); + this.name = Objects.requireNonNull(name); this.baseFsPrincipal = Objects.requireNonNull(baseFsPrincipal); } @@ -113,23 +115,12 @@ class ContainerUserPrincipalLookupService extends UserPrincipalLookupService { } static final class ContainerUserPrincipal extends NamedPrincipal { - ContainerUserPrincipal(int id, UserPrincipal baseFsPrincipal) { super(id, baseFsPrincipal); } + private ContainerUserPrincipal(int id, String name, UserPrincipal baseFsPrincipal) { super(id, name, baseFsPrincipal); } } static final class ContainerGroupPrincipal extends NamedPrincipal implements GroupPrincipal { - ContainerGroupPrincipal(int id, GroupPrincipal baseFsPrincipal) { super(id, baseFsPrincipal); } + private ContainerGroupPrincipal(int id, String name, GroupPrincipal baseFsPrincipal) { super(id, name, baseFsPrincipal); } @Override public GroupPrincipal baseFsPrincipal() { return (GroupPrincipal) super.baseFsPrincipal(); } } - - private static int containerIdToHostId(int id, int idOffset) { - if (id < 0 || id > ID_RANGE) - throw new IllegalArgumentException("Invalid container id: " + id); - return idOffset + id; - } - - private static int hostIdToContainerId(int id, int idOffset) { - id = id - idOffset; - return id < 0 || id >= ID_RANGE ? OVERFLOW_ID : id; - } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/UserNamespaceTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/UserNamespaceTest.java new file mode 100644 index 00000000000..73b59a17c37 --- /dev/null +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/nodeagent/UserNamespaceTest.java @@ -0,0 +1,29 @@ +// 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.nodeagent; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * @author valerijf + */ +class UserNamespaceTest { + + private final UserNamespace userNamespace = new UserNamespace(1000, 2000, "vespa", "users", 1000, 100); + + @Test + public void translates_between_ids() { + assertEquals(1001, userNamespace.userIdOnHost(1)); + assertEquals(2001, userNamespace.groupIdOnHost(1)); + assertEquals(1, userNamespace.userIdInContainer(1001)); + assertEquals(1, userNamespace.groupIdInContainer(2001)); + + assertEquals(userNamespace.overflowId(), userNamespace.userIdInContainer(1)); + assertEquals(userNamespace.overflowId(), userNamespace.userIdInContainer(999999)); + + assertThrows(IllegalArgumentException.class, () -> userNamespace.userIdOnHost(-1)); + assertThrows(IllegalArgumentException.class, () -> userNamespace.userIdOnHost(70_000)); + } +} \ No newline at end of file diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java index 970a264d0df..a5fc6a1373f 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerFileSystemTest.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.task.util.fs; +import com.yahoo.vespa.hosted.node.admin.nodeagent.UserNamespace; import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixPath; import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.jupiter.api.Test; @@ -14,7 +15,6 @@ import java.nio.file.StandardCopyOption; import java.nio.file.StandardOpenOption; import java.util.Map; -import static com.yahoo.vespa.hosted.node.admin.task.util.fs.ContainerUserPrincipalLookupService.OVERFLOW_ID; import static org.junit.jupiter.api.Assertions.assertEquals; /** @@ -24,7 +24,8 @@ class ContainerFileSystemTest { private final FileSystem fileSystem = TestFileSystem.create(); private final UnixPath containerRootOnHost = new UnixPath(fileSystem.getPath("/data/storage/ctr1")); - private final ContainerFileSystem containerFs = ContainerFileSystem.create(containerRootOnHost.createDirectories().toPath(), 10_000, 11_000); + private final UserNamespace userNamespace = new UserNamespace(10_000, 11_000, "vespa", "users", 1000, 100); + private final ContainerFileSystem containerFs = ContainerFileSystem.create(containerRootOnHost.createDirectories().toPath(), userNamespace); @Test public void creates_files_and_directories_with_container_root_as_owner() throws IOException { @@ -65,7 +66,7 @@ class ContainerFileSystemTest { // If file is copied to JimFS path, the UID/GIDs are not fixed Files.copy(hostFile.toPath(), destination.pathOnHost()); - assertEquals(String.valueOf(OVERFLOW_ID), Files.getOwner(destination).getName()); + assertEquals(String.valueOf(userNamespace.overflowId()), Files.getOwner(destination).getName()); Files.delete(destination); Files.copy(hostFile.toPath(), destination); @@ -94,7 +95,7 @@ class ContainerFileSystemTest { // If file is moved to JimFS path, the UID/GIDs are not fixed Files.move(hostFile.toPath(), destination.pathOnHost()); - assertEquals(String.valueOf(OVERFLOW_ID), Files.getOwner(destination).getName()); + assertEquals(String.valueOf(userNamespace.overflowId()), Files.getOwner(destination).getName()); Files.delete(destination); hostFile.createNewFile(); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerPathTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerPathTest.java index ebbbaf3b525..6bca8c2f0b1 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerPathTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerPathTest.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.task.util.fs; +import com.yahoo.vespa.hosted.node.admin.nodeagent.UserNamespace; import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -12,6 +13,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; import java.io.IOException; import java.nio.file.FileSystem; @@ -25,7 +27,7 @@ import java.nio.file.Path; class ContainerPathTest { private final FileSystem baseFs = TestFileSystem.create(); - private final ContainerFileSystem containerFs = ContainerFileSystem.create(baseFs.getPath("/data/storage/ctr1"), 0, 0); + private final ContainerFileSystem containerFs = ContainerFileSystem.create(baseFs.getPath("/data/storage/ctr1"), mock(UserNamespace.class)); @Test public void create_new_container_path() { diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerUserPrincipalLookupServiceTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerUserPrincipalLookupServiceTest.java index a459c24049e..bc26cfa73f3 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerUserPrincipalLookupServiceTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/task/util/fs/ContainerUserPrincipalLookupServiceTest.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.task.util.fs; +import com.yahoo.vespa.hosted.node.admin.nodeagent.UserNamespace; import com.yahoo.vespa.test.file.TestFileSystem; import org.junit.jupiter.api.Test; @@ -17,35 +18,22 @@ import static org.junit.jupiter.api.Assertions.assertThrows; */ class ContainerUserPrincipalLookupServiceTest { + private final UserNamespace userNamespace = new UserNamespace(10_000, 11_000, "vespa", "users", 1000, 100); private final ContainerUserPrincipalLookupService userPrincipalLookupService = - new ContainerUserPrincipalLookupService(TestFileSystem.create().getUserPrincipalLookupService(), 1000, 2000); + new ContainerUserPrincipalLookupService(TestFileSystem.create().getUserPrincipalLookupService(), userNamespace); @Test public void correctly_resolves_ids() throws IOException { ContainerUserPrincipal user = userPrincipalLookupService.lookupPrincipalByName("1000"); assertEquals("vespa", user.getName()); - assertEquals("2000", user.baseFsPrincipal().getName()); + assertEquals("11000", user.baseFsPrincipal().getName()); assertEquals(user, userPrincipalLookupService.lookupPrincipalByName("vespa")); - ContainerGroupPrincipal group = userPrincipalLookupService.lookupPrincipalByGroupName("1000"); - assertEquals("vespa", group.getName()); - assertEquals("3000", group.baseFsPrincipal().getName()); - assertEquals(group, userPrincipalLookupService.lookupPrincipalByGroupName("vespa")); + ContainerGroupPrincipal group = userPrincipalLookupService.lookupPrincipalByGroupName("100"); + assertEquals("users", group.getName()); + assertEquals("11100", group.baseFsPrincipal().getName()); + assertEquals(group, userPrincipalLookupService.lookupPrincipalByGroupName("users")); assertThrows(UserPrincipalNotFoundException.class, () -> userPrincipalLookupService.lookupPrincipalByName("test")); } - - @Test - public void translates_between_ids() { - assertEquals(1001, userPrincipalLookupService.containerUidToHostUid(1)); - assertEquals(2001, userPrincipalLookupService.containerGidToHostGid(1)); - assertEquals(1, userPrincipalLookupService.hostUidToContainerUid(1001)); - assertEquals(1, userPrincipalLookupService.hostGidToContainerGid(2001)); - - assertEquals(65_534, userPrincipalLookupService.hostUidToContainerUid(1)); - assertEquals(65_534, userPrincipalLookupService.hostUidToContainerUid(999999)); - - assertThrows(IllegalArgumentException.class, () -> userPrincipalLookupService.containerUidToHostUid(-1)); - assertThrows(IllegalArgumentException.class, () -> userPrincipalLookupService.containerUidToHostUid(70_000)); - } -} \ No newline at end of file +} -- cgit v1.2.3 From dda42abfdf6dc0538fff025c8e2712b44f528ec0 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Fri, 15 Oct 2021 14:44:29 +0200 Subject: Set correct owner --- .../identity/AthenzCredentialsMaintainer.java | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) (limited to 'node-admin') diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java index c5cb6020e1c..280e58c91f1 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/identity/AthenzCredentialsMaintainer.java @@ -206,8 +206,8 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { EntityBindingsMapper.toAttestationData(signedIdentityDocument), csr); EntityBindingsMapper.writeSignedIdentityDocumentToFile(identityDocumentFile, signedIdentityDocument); - writePrivateKeyAndCertificate(privateKeyFile, keyPair.getPrivate(), - certificateFile, instanceIdentity.certificate()); + writePrivateKeyAndCertificate(context.userNamespace().vespaUserId(), + privateKeyFile, keyPair.getPrivate(), certificateFile, instanceIdentity.certificate()); context.log(logger, "Instance successfully registered and credentials written to file"); } } @@ -234,8 +234,8 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { context.identity(), identityDocument.providerUniqueId().asDottedString(), csr); - writePrivateKeyAndCertificate(privateKeyFile, keyPair.getPrivate(), - certificateFile, instanceIdentity.certificate()); + writePrivateKeyAndCertificate(context.userNamespace().vespaUserId(), + privateKeyFile, keyPair.getPrivate(), certificateFile, instanceIdentity.certificate()); context.log(logger, "Instance successfully refreshed and credentials written to file"); } catch (ZtsClientException e) { if (e.getErrorCode() == 403 && e.getDescription().startsWith("Certificate revoked")) { @@ -251,18 +251,19 @@ public class AthenzCredentialsMaintainer implements CredentialsMaintainer { } - private static void writePrivateKeyAndCertificate(ContainerPath privateKeyFile, + private static void writePrivateKeyAndCertificate(int vespaUid, + ContainerPath privateKeyFile, PrivateKey privateKey, ContainerPath certificateFile, X509Certificate certificate) { - writeFile(privateKeyFile, KeyUtils.toPem(privateKey)); - writeFile(certificateFile, X509CertificateUtils.toPem(certificate)); + writeFile(privateKeyFile, vespaUid, KeyUtils.toPem(privateKey)); + writeFile(certificateFile, vespaUid, X509CertificateUtils.toPem(certificate)); } - private static void writeFile(ContainerPath path, String utf8Content) { + private static void writeFile(ContainerPath path, int vespaUid, String utf8Content) { new UnixPath(path.resolveSibling(path.getFileName() + ".tmp")) .writeUtf8File(utf8Content, "r--------") - .setOwner("vespa") + .setOwnerId(vespaUid) .atomicMove(path); } -- cgit v1.2.3