Reduce the amount of duplicate logging when using kubernetes agents

Only the logging of the build publish command will get duplicated on kubernetes agents

Author: janssk1

src/main/java/io/jenkins/plugins/jfrog/JfStep.java

@@ -87,21 +87,34 @@ public void perform(@NonNull Run<?, ?> run, @NonNull FilePath workspace, @NonNul
             builder = builder.toWindowsCommand();
         }
 
-        try (ByteArrayOutputStream taskOutputStream = new ByteArrayOutputStream()) {
-            JfTaskListener jfTaskListener = new JfTaskListener(listener, taskOutputStream);
-            Launcher.ProcStarter jfLauncher = setupJFrogEnvironment(run, env, launcher, jfTaskListener, workspace, jfrogBinaryPath, isWindows);
-            // Running the 'jf' command
-            int exitValue = jfLauncher.cmds(builder).join();
-            if (exitValue != 0) {
-                throw new RuntimeException("Running 'jf' command failed with exit code " + exitValue);
+        try {
+            Launcher.ProcStarter procStarter = setupJFrogEnvironment(run, env, launcher, listener, workspace, jfrogBinaryPath, isWindows);
+            if (isBuildPublish()) {
+                String stdOut = executeJfCommandReturningStdOut(procStarter, builder, listener);
+                addBuildInfoActionIfNeeded(new JenkinsBuildInfoLog(listener), run, stdOut);
+            } else {
+                executeJfCommand(procStarter, builder);
             }
-            addBuildInfoActionIfNeeded(new JenkinsBuildInfoLog(listener), run, taskOutputStream);
         } catch (Exception e) {
             String errorMessage = "Couldn't execute 'jf' command. " + ExceptionUtils.getRootCauseMessage(e);
             throw new RuntimeException(errorMessage, e);
         }
     }
 
+    private static String executeJfCommandReturningStdOut(Launcher.ProcStarter procStarter, ArgumentListBuilder builder, @NonNull TaskListener listener) throws IOException, InterruptedException {
+        ByteArrayOutputStream taskOutputStream = new ByteArrayOutputStream();
+        JfTaskListener jfTaskListener = new JfTaskListener(listener, taskOutputStream);
+        executeJfCommand(procStarter.stdout(jfTaskListener), builder);
+        return taskOutputStream.toString(StandardCharsets.UTF_8);
+    }
+
+    private static void executeJfCommand(Launcher.ProcStarter procStarter, ArgumentListBuilder builder) throws IOException, InterruptedException {
+        int exitValue = procStarter.cmds(builder).join();
+        if (exitValue != 0) {
+            throw new RuntimeException("Running 'jf' command failed with exit code " + exitValue);
+        }
+    }
+
     /**
      * Get JFrog CLI path in agent, according to the JFROG_BINARY_PATH environment variable.
      * The JFROG_BINARY_PATH also can be set implicitly in Declarative Pipeline by choosing the JFrog CLI tool or
@@ -162,7 +175,7 @@ public Launcher.ProcStarter setupJFrogEnvironment(Run<?, ?> run, EnvVars env, La
         }
         FilePath jfrogHomeTempDir = Utils.createAndGetJfrogCliHomeTempDir(workspace, String.valueOf(run.getNumber()));
         CliEnvConfigurator.configureCliEnv(env, jfrogHomeTempDir.getRemote(), jfrogCliConfigEncryption);
-        Launcher.ProcStarter jfLauncher = launcher.launch().envs(env).pwd(workspace).stdout(listener);
+        Launcher.ProcStarter jfLauncher = launcher.launch().envs(env).stdout(listener).pwd(workspace);
         // Configure all servers, skip if all server ids have already been configured.
         if (shouldConfig(jfrogHomeTempDir)) {
             logIfNoToolProvided(env, listener);
@@ -190,7 +203,7 @@ private boolean shouldConfig(FilePath jfrogHomeTempDir) throws IOException, Inte
     /**
      * Locally configure all servers that was configured in the Jenkins UI.
      */
-    private void configAllServers(Launcher.ProcStarter launcher, String jfrogBinaryPath, boolean isWindows, Job<?, ?> job) throws IOException, InterruptedException {
+    private void configAllServers(Launcher.ProcStarter procStarter, String jfrogBinaryPath, boolean isWindows, Job<?, ?> job) throws IOException, InterruptedException {
         // Config all servers using the 'jf c add' command.
         List<JFrogPlatformInstance> jfrogInstances = JFrogPlatformBuilder.getJFrogPlatformInstances();
         if (jfrogInstances != null && jfrogInstances.size() > 0) {
@@ -202,10 +215,7 @@ private void configAllServers(Launcher.ProcStarter launcher, String jfrogBinaryP
                     builder = builder.toWindowsCommand();
                 }
                 // Running 'jf' command
-                int exitValue = launcher.cmds(builder).join();
-                if (exitValue != 0) {
-                    throw new RuntimeException("Running 'jf' command failed with exit code " + exitValue);
-                }
+                executeJfCommand(procStarter, builder);
             }
         }
     }
@@ -238,20 +248,13 @@ private void addConfigArguments(ArgumentListBuilder builder, JFrogPlatformInstan
      *
      * @param log              - Task logger
      * @param run              - The Jenkins project
-     * @param taskOutputStream - Task's output stream
+     * @param originalTaskOutput - Task's output stream
      */
-    void addBuildInfoActionIfNeeded(Log log, Run<?, ?> run, ByteArrayOutputStream taskOutputStream) {
-        if (args.length < 2 ||
-                !args[0].equals("rt") ||
-                !equalsAny(args[1], "bp", "build-publish")) {
-            return;
-        }
-
+    void addBuildInfoActionIfNeeded(Log log, Run<?, ?> run, String originalTaskOutput) {
         // Search for '{' and '}' in the output of 'jf rt build-publish'
-        String taskOutput = taskOutputStream.toString(StandardCharsets.UTF_8);
-        taskOutput = substringBetween(taskOutput, "{", "}");
+        String taskOutput = substringBetween(originalTaskOutput, "{", "}");
         if (taskOutput == null) {
-            logIllegalBuildPublishOutput(log, taskOutputStream);
+            logIllegalBuildPublishOutput(log, originalTaskOutput);
             return;
         }
 
@@ -260,11 +263,11 @@ void addBuildInfoActionIfNeeded(Log log, Run<?, ?> run, ByteArrayOutputStream ta
         try {
             buildInfoOutputModel = mapper.readValue("{" + taskOutput + "}", BuildInfoOutputModel.class);
             if (buildInfoOutputModel == null) {
-                logIllegalBuildPublishOutput(log, taskOutputStream);
+                logIllegalBuildPublishOutput(log, originalTaskOutput);
                 return;
             }
         } catch (JsonProcessingException e) {
-            logIllegalBuildPublishOutput(log, taskOutputStream);
+            logIllegalBuildPublishOutput(log, originalTaskOutput);
             log.warn(ExceptionUtils.getRootCauseMessage(e));
             return;
         }
@@ -276,8 +279,14 @@ void addBuildInfoActionIfNeeded(Log log, Run<?, ?> run, ByteArrayOutputStream ta
         }
     }
 
-    private void logIllegalBuildPublishOutput(Log log, ByteArrayOutputStream taskOutputStream) {
-        log.warn("Illegal build-publish output: " + taskOutputStream.toString(StandardCharsets.UTF_8));
+    boolean isBuildPublish() {
+        return args.length >= 2 &&
+                args[0].equals("rt") &&
+                equalsAny(args[1], "bp", "build-publish");
+    }
+
+    private void logIllegalBuildPublishOutput(Log log, String taskOutput) {
+        log.warn("Illegal build-publish output: " + taskOutput);
     }
 
     @Symbol("jf")

src/test/java/io/jenkins/plugins/jfrog/AddBuildInfoActionTest.java

@@ -16,9 +16,6 @@
 import org.mockito.junit.MockitoRule;
 import org.mockito.junit.jupiter.MockitoExtension;
 
-import java.io.ByteArrayOutputStream;
-import java.io.IOException;
-import java.nio.charset.StandardCharsets;
 import java.util.stream.Stream;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -55,7 +52,7 @@ private static Stream<Arguments> positiveDataProvider() {
 
     @ParameterizedTest
     @MethodSource("positiveDataProvider")
-    public void addBuildInfoActionPositiveTest(String command, String output) throws IOException {
+    public void addBuildInfoActionPositiveTest(String command, String output) {
         doNothing().when(run).addAction(valueCapture.capture());
         runCliCommand(command, output);
 
@@ -75,15 +72,15 @@ private static Stream<Arguments> negativeDataProvider() {
 
     @ParameterizedTest
     @MethodSource("negativeDataProvider")
-    public void addBuildInfoActionNegativeTest(String command, String output) throws IOException {
+    public void addBuildInfoActionNegativeTest(String command, String output) {
         runCliCommand(command, output);
         Mockito.verify(run, never()).addAction(isA(Action.class));
     }
 
-    private void runCliCommand(String command, String output) throws IOException {
-        try (ByteArrayOutputStream taskOutputStream = new ByteArrayOutputStream()) {
-            taskOutputStream.writeBytes(output.getBytes(StandardCharsets.UTF_8));
-            new JfStep(command).addBuildInfoActionIfNeeded(new NullLog(), run, taskOutputStream);
+    private void runCliCommand(String command, String output) {
+        JfStep jfStep = new JfStep(command);
+        if (jfStep.isBuildPublish()) {
+            jfStep.addBuildInfoActionIfNeeded(new NullLog(), run, output);
         }
     }
 }