From f322121bbe051b8c0e9b2efd6218468234e30928 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Mon, 15 Nov 2021 16:39:14 +0100 Subject: Remove unused class --- .../maintenance/servicedump/AbstractProducer.java | 68 ---------------------- 1 file changed, 68 deletions(-) delete mode 100644 node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/AbstractProducer.java 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 deleted file mode 100644 index a1416d3274c..00000000000 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/AbstractProducer.java +++ /dev/null @@ -1,68 +0,0 @@ -// 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.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.util.List; -import java.util.logging.Level; -import java.util.logging.Logger; -import java.util.stream.Collectors; - -/** - * @author bjorncs - */ -abstract class AbstractProducer implements ArtifactProducer { - - private final Logger log = Logger.getLogger(getClass().getName()); - - private final ContainerOperations container; - - protected AbstractProducer(ContainerOperations container) { this.container = container; } - - protected ContainerOperations container() { return container; } - - protected CommandResult executeCommand(NodeAgentContext ctx, List command, boolean logOutput) throws IOException { - CommandResult result = container.executeCommandInContainerAsRoot(ctx, command.toArray(new String[0])); - String cmdString = command.stream().map(s -> "'" + s + "'").collect(Collectors.joining(" ", "\"", "\"")); - int exitCode = result.getExitCode(); - String output = result.getOutput().trim(); - String prefixedOutput = output.contains("\n") - ? "\n" + output - : (output.isEmpty() ? "" : output); - if (exitCode > 0) { - String errorMsg = logOutput - ? String.format("Failed to execute %s (exited with code %d): %s", cmdString, exitCode, prefixedOutput) - : String.format("Failed to execute %s (exited with code %d)", cmdString, exitCode); - throw new IOException(errorMsg); - } else { - String logMsg = logOutput - ? String.format("Executed command %s. Exited with code %d and output: %s", cmdString, exitCode, prefixedOutput) - : String.format("Executed command %s. Exited with code %d.", cmdString, exitCode); - ctx.log(log, logMsg); - } - return result; - } - - protected int findVespaServicePid(NodeAgentContext ctx, String configId) throws IOException { - ContainerPath findPidBinary = ctx.paths().underVespaHome("libexec/vespa/find-pid"); - CommandResult findPidResult = executeCommand(ctx, List.of(findPidBinary.pathInContainer(), configId), true); - return Integer.parseInt(findPidResult.getOutput()); - } - - protected double duration(NodeAgentContext ctx, ServiceDumpReport.DumpOptions options, double defaultValue) { - double duration = options != null && options.duration() != null && options.duration() > 0 - ? options.duration() : defaultValue; - double maxDuration = 300; - if (duration > maxDuration) { - ctx.log(log, Level.WARNING, - String.format("Specified duration %.3fs longer than max allowed (%.3fs)", duration, maxDuration)); - return maxDuration; - } - return duration; - } - -} -- cgit v1.2.3 From 15fb3e939af9ef532ca91ae14d96e493cb0aad34 Mon Sep 17 00:00:00 2001 From: Valerij Fredriksen Date: Mon, 15 Nov 2021 16:39:47 +0100 Subject: Require UnixUser to ContainerEngine::execute --- .../node/admin/container/ContainerEngine.java | 5 ++-- .../node/admin/container/ContainerOperations.java | 11 ++++---- .../admin/maintenance/coredump/CoreCollector.java | 14 +++++----- .../servicedump/VespaServiceDumperImpl.java | 2 +- .../node/admin/container/ContainerEngineMock.java | 3 ++- .../maintenance/coredump/CoreCollectorTest.java | 2 +- .../servicedump/VespaServiceDumperImplTest.java | 30 +++++++++++----------- 7 files changed, 35 insertions(+), 32 deletions(-) diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngine.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngine.java index a3bce4c687e..cfa0452ebf9 100644 --- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngine.java +++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngine.java @@ -6,6 +6,7 @@ import com.yahoo.vespa.hosted.node.admin.component.TaskContext; import com.yahoo.vespa.hosted.node.admin.container.image.Image; import com.yahoo.vespa.hosted.node.admin.nodeagent.ContainerData; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; +import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixUser; import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult; import java.time.Duration; @@ -40,8 +41,8 @@ public interface ContainerEngine { /** Returns the network interface used by container in given context */ String networkInterface(NodeAgentContext context); - /** Execute command inside container as root. Ignores non-zero exit code */ - CommandResult executeAsRoot(NodeAgentContext context, Duration timeout, String... command); + /** Execute command inside container as given user. Ignores non-zero exit code */ + CommandResult execute(NodeAgentContext context, UnixUser user, Duration timeout, String... command); /** Execute command inside the container's network namespace. Throws on non-zero exit code */ CommandResult executeInNetworkNamespace(NodeAgentContext context, String... command); 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 3017773700a..8a66373c28b 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 @@ -7,6 +7,7 @@ import com.yahoo.vespa.hosted.node.admin.container.image.ContainerImageDownloade import com.yahoo.vespa.hosted.node.admin.container.image.ContainerImagePruner; import com.yahoo.vespa.hosted.node.admin.nodeagent.ContainerData; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; +import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixUser; import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandLine; import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult; @@ -66,13 +67,13 @@ public class ContainerOperations { } /** Executes a command inside container identified by given context. Does NOT throw on non-zero exit code */ - public CommandResult executeCommandInContainerAsRoot(NodeAgentContext context, String... command) { - return executeCommandInContainerAsRoot(context, CommandLine.DEFAULT_TIMEOUT.toSeconds(), command); + public CommandResult executeCommandInContainer(NodeAgentContext context, UnixUser user, String... command) { + return executeCommandInContainer(context, user, CommandLine.DEFAULT_TIMEOUT, command); } /** Execute command inside container identified by given context. Does NOT throw on non-zero exit code */ - public CommandResult executeCommandInContainerAsRoot(NodeAgentContext context, Long timeoutSeconds, String... command) { - return containerEngine.executeAsRoot(context, Duration.ofSeconds(timeoutSeconds), command); + public CommandResult executeCommandInContainer(NodeAgentContext context, UnixUser user, Duration timeout, String... command) { + return containerEngine.execute(context, user, timeout, command); } /** Execute command in inside containers network namespace, identified by given context. Throws on non-zero exit code */ @@ -142,7 +143,7 @@ public class ContainerOperations { private String executeNodeCtlInContainer(NodeAgentContext context, String program) { String[] command = new String[] {context.paths().underVespaHome("bin/vespa-nodectl").pathInContainer(), program}; - return executeCommandInContainerAsRoot(context, command).getOutput(); + return executeCommandInContainer(context, context.users().vespa(), command).getOutput(); } } 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 60330984f57..60435082745 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 @@ -34,10 +34,10 @@ public class CoreCollector { static final Map JAVA_HEAP_DUMP_METADATA = Map.of("bin_path", "java", "backtrace", List.of("Heap dump, no backtrace available")); - private final ContainerOperations docker; + private final ContainerOperations container; - public CoreCollector(ContainerOperations docker) { - this.docker = docker; + public CoreCollector(ContainerOperations container) { + this.container = container; } String getGdbPath(NodeAgentContext context) { @@ -47,7 +47,7 @@ public class CoreCollector { 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); + CommandResult result = container.executeCommandInContainer(context, context.users().root(), wrappedCommand); Matcher matcher = CORE_GENERATOR_PATH_PATTERN.matcher(result.getOutput()); if (! matcher.find()) { @@ -60,7 +60,7 @@ public class CoreCollector { String readBinPath(NodeAgentContext context, ContainerPath coredumpPath) { String[] command = {"file", coredumpPath.pathInContainer()}; try { - CommandResult result = docker.executeCommandInContainerAsRoot(context, command); + CommandResult result = container.executeCommandInContainer(context, context.users().root(), command); if (result.getExitCode() != 0) { throw new ConvergenceException("file command failed with " + asString(result)); } @@ -86,7 +86,7 @@ public class CoreCollector { String threads = allThreads ? "thread apply all bt" : "bt"; String[] command = {getGdbPath(context), "-n", "-ex", threads, "-batch", binPath, coredumpPath.pathInContainer()}; - CommandResult result = docker.executeCommandInContainerAsRoot(context, command); + CommandResult result = container.executeCommandInContainer(context, context.users().root(), command); if (result.getExitCode() != 0) throw new ConvergenceException("Failed to read backtrace " + asString(result) + ", Command: " + Arrays.toString(command)); @@ -96,7 +96,7 @@ public class CoreCollector { List readJstack(NodeAgentContext context, ContainerPath coredumpPath, String binPath) { String[] command = {"jhsdb", "jstack", "--exe", binPath, "--core", coredumpPath.pathInContainer()}; - CommandResult result = docker.executeCommandInContainerAsRoot(context, command); + CommandResult result = container.executeCommandInContainer(context, context.users().root(), command); if (result.getExitCode() != 0) throw new ConvergenceException("Failed to read jstack " + asString(result) + ", Command: " + Arrays.toString(command)); 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 86dc1ed983d..b30b8e22fc5 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 @@ -201,7 +201,7 @@ public class VespaServiceDumperImpl implements VespaServiceDumper { @Override public CommandResult executeCommandInNode(List command, boolean logOutput) { - CommandResult result = container.executeCommandInContainerAsRoot(nodeAgentCtx, command.toArray(new String[0])); + CommandResult result = container.executeCommandInContainer(nodeAgentCtx, nodeAgentCtx.users().vespa(), command.toArray(new String[0])); String cmdString = command.stream().map(s -> "'" + s + "'").collect(Collectors.joining(" ", "\"", "\"")); int exitCode = result.getExitCode(); String output = result.getOutput().trim(); diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngineMock.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngineMock.java index 3eab24a7a66..25cdff4b726 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngineMock.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/container/ContainerEngineMock.java @@ -6,6 +6,7 @@ import com.yahoo.vespa.hosted.node.admin.component.TaskContext; import com.yahoo.vespa.hosted.node.admin.container.image.Image; import com.yahoo.vespa.hosted.node.admin.nodeagent.ContainerData; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContext; +import com.yahoo.vespa.hosted.node.admin.task.util.file.UnixUser; import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult; import java.time.Duration; @@ -109,7 +110,7 @@ public class ContainerEngineMock implements ContainerEngine { } @Override - public CommandResult executeAsRoot(NodeAgentContext context, Duration timeout, String... command) { + public CommandResult execute(NodeAgentContext context, UnixUser user, Duration timeout, String... command) { return new CommandResult(null, 0, ""); } 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 413fabb7880..8ab6bce2b8c 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 @@ -177,7 +177,7 @@ public class CoreCollectorTest { } private void mockExec(NodeAgentContext context, String[] cmd, String output, String error) { - when(docker.executeCommandInContainerAsRoot(context, cmd)) + when(docker.executeCommandInContainer(context, context.users().root(), cmd)) .thenReturn(new CommandResult(null, error.isEmpty() ? 0 : 1, error.isEmpty() ? output : error)); } } diff --git a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImplTest.java b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImplTest.java index 68127231554..452efecefe1 100644 --- a/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImplTest.java +++ b/node-admin/src/test/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/VespaServiceDumperImplTest.java @@ -1,7 +1,6 @@ // 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.yolean.concurrent.Sleeper; import com.yahoo.test.ManualClock; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeSpec; import com.yahoo.vespa.hosted.node.admin.configserver.noderepository.NodeState; @@ -12,6 +11,7 @@ import com.yahoo.vespa.hosted.node.admin.maintenance.sync.SyncFileInfo; import com.yahoo.vespa.hosted.node.admin.nodeagent.NodeAgentContextImpl; import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult; import com.yahoo.vespa.test.file.TestFileSystem; +import com.yahoo.yolean.concurrent.Sleeper; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.ArgumentCaptor; @@ -62,7 +62,7 @@ class VespaServiceDumperImplTest { void invokes_perf_commands_when_generating_perf_report() { // Setup mocks ContainerOperations operations = mock(ContainerOperations.class); - when(operations.executeCommandInContainerAsRoot(any(), any())) + when(operations.executeCommandInContainer(any(), any(), any())) .thenReturn(new CommandResult(null, 0, "12345")) .thenReturn(new CommandResult(null, 0, "")) .thenReturn(new CommandResult(null, 0, "")); @@ -78,13 +78,13 @@ class VespaServiceDumperImplTest { .build(); reporter.processServiceDumpRequest(context); - verify(operations).executeCommandInContainerAsRoot( - context, "/opt/vespa/libexec/vespa/find-pid", "default/container.1"); - verify(operations).executeCommandInContainerAsRoot( - context, "perf", "record", "-g", "--output=/opt/vespa/tmp/vespa-service-dump/perf-record.bin", + verify(operations).executeCommandInContainer( + context, context.users().vespa(), "/opt/vespa/libexec/vespa/find-pid", "default/container.1"); + verify(operations).executeCommandInContainer( + context, context.users().vespa(), "perf", "record", "-g", "--output=/opt/vespa/tmp/vespa-service-dump/perf-record.bin", "--pid=12345", "sleep", "45"); - verify(operations).executeCommandInContainerAsRoot( - context, "bash", "-c", "perf report --input=/opt/vespa/tmp/vespa-service-dump/perf-record.bin" + + verify(operations).executeCommandInContainer( + context, context.users().vespa(), "bash", "-c", "perf report --input=/opt/vespa/tmp/vespa-service-dump/perf-record.bin" + " > /opt/vespa/tmp/vespa-service-dump/perf-report.txt"); String expectedJson = "{\"createdMillis\":1600000000000,\"startedAt\":1600001000000,\"completedAt\":1600001000000," + @@ -103,7 +103,7 @@ class VespaServiceDumperImplTest { void invokes_jcmd_commands_when_creating_jfr_recording() { // Setup mocks ContainerOperations operations = mock(ContainerOperations.class); - when(operations.executeCommandInContainerAsRoot(any(), any())) + when(operations.executeCommandInContainer(any(), any(), any())) .thenReturn(new CommandResult(null, 0, "12345")) .thenReturn(new CommandResult(null, 0, "ok")) .thenReturn(new CommandResult(null, 0, "name=host-admin success")); @@ -120,12 +120,12 @@ class VespaServiceDumperImplTest { .build(); reporter.processServiceDumpRequest(context); - verify(operations).executeCommandInContainerAsRoot( - context, "/opt/vespa/libexec/vespa/find-pid", "default/container.1"); - verify(operations).executeCommandInContainerAsRoot( - context, "jcmd", "12345", "JFR.start", "name=host-admin", "path-to-gc-roots=true", "settings=profile", + verify(operations).executeCommandInContainer( + context, context.users().vespa(), "/opt/vespa/libexec/vespa/find-pid", "default/container.1"); + verify(operations).executeCommandInContainer( + context, context.users().vespa(), "jcmd", "12345", "JFR.start", "name=host-admin", "path-to-gc-roots=true", "settings=profile", "filename=/opt/vespa/tmp/vespa-service-dump/recording.jfr", "duration=30s"); - verify(operations).executeCommandInContainerAsRoot(context, "jcmd", "12345", "JFR.check", "name=host-admin"); + verify(operations).executeCommandInContainer(context, context.users().vespa(), "jcmd", "12345", "JFR.check", "name=host-admin"); String expectedJson = "{\"createdMillis\":1600000000000,\"startedAt\":1600001000000," + "\"completedAt\":1600001000000," + @@ -142,7 +142,7 @@ class VespaServiceDumperImplTest { void handles_multiple_artifact_types() { // Setup mocks ContainerOperations operations = mock(ContainerOperations.class); - when(operations.executeCommandInContainerAsRoot(any(), any())) + when(operations.executeCommandInContainer(any(), any(), any())) // For perf report: .thenReturn(new CommandResult(null, 0, "12345")) .thenReturn(new CommandResult(null, 0, "")) -- cgit v1.2.3