Add abstract producer class to reduce logic for executing shell command

2 files changed, 52 insertions, 18 deletions

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
new file mode 100644
index 00000000000..b0010037027
--- /dev/null
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/AbstractProducer.java
@@ -0,0 +1,48 @@
+// 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.process.CommandResult;
+
+import java.io.IOException;
+import java.util.List;
+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<String> 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() ? "<no output>" : output);
+        String logMsg = logOutput
+                ? String.format("Executed command %s.\nExited with code %d and output: %s", cmdString, exitCode, prefixedOutput)
+                : String.format("Executed command %s.\nExited with code %d.", cmdString, exitCode);
+        ctx.log(log, logMsg);
+        if (exitCode > 0) {
+            String errorMsg = logOutput
+                    ? String.format("Failed to execute %s: %s", cmdString, prefixedOutput)
+                    : String.format("Failed to execute %s (exited with code %d)", cmdString, exitCode);
+            throw new IOException(errorMsg);
+        }
+        return result;
+    }
+
+
+}
diff --git a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JvmDumpProducer.java b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JvmDumpProducer.java
index 014ad10beb6..5cbbf304bb8 100644
--- a/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JvmDumpProducer.java
+++ b/node-admin/src/main/java/com/yahoo/vespa/hosted/node/admin/maintenance/servicedump/JvmDumpProducer.java
@@ -4,24 +4,18 @@ 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.file.UnixPath;
-import com.yahoo.vespa.hosted.node.admin.task.util.process.CommandResult;
 
 import java.io.IOException;
-import java.util.logging.Level;
-import java.util.logging.Logger;
+import java.util.List;
 
 /**
  * Creates a dump of JVM based Vespa services using vespa-jvm-dumper
  *
  * @author bjorncs
  */
-class JvmDumpProducer implements ArtifactProducer {
+class JvmDumpProducer extends AbstractProducer {
 
-    private static final Logger log = Logger.getLogger(JvmDumpProducer.class.getName());
-
-    private final ContainerOperations container;
-
-    JvmDumpProducer(ContainerOperations container) { this.container = container; }
+    JvmDumpProducer(ContainerOperations container) { super(container); }
 
     public static String NAME = "jvm-dump";
 
@@ -31,14 +25,6 @@ class JvmDumpProducer implements ArtifactProducer {
     public void produceArtifact(NodeAgentContext context, String configId, ServiceDumpReport.DumpOptions options,
                                 UnixPath resultDirectoryInNode) throws IOException {
         UnixPath vespaJvmDumper = new UnixPath(context.pathInNodeUnderVespaHome("bin/vespa-jvm-dumper"));
-        context.log(log, Level.INFO,
-                "Executing '" + vespaJvmDumper + "' with arguments '" + configId + "' and '" + resultDirectoryInNode + "'");
-        CommandResult result = container.executeCommandInContainerAsRoot(
-                context, vespaJvmDumper.toString(), configId, resultDirectoryInNode.toString());
-        context.log(log, Level.INFO,
-                "vespa-jvm-dumper exited with code '" + result.getExitCode() + "' and output:\n" + result.getOutput());
-        if (result.getExitCode() > 0) {
-            throw new IOException("Failed to jvm dump: " + result.getOutput());
-        }
+        executeCommand(context, List.of(vespaJvmDumper.toString(), configId, resultDirectoryInNode.toString()), true);
     }
 }