Merge pull request #300 from jenkinsci/JENKINS-50392-exit-code

[JENKINS-50392] Get the exit code the correct way

Author: carlossg

pom.xml

@@ -46,7 +46,7 @@
     <!-- dependency versions -->
     <jenkins.version>2.32.1</jenkins.version>
 
-    <kubernetes-client.version>3.1.10</kubernetes-client.version>
+    <kubernetes-client.version>3.2.0</kubernetes-client.version>
 
     <!-- jenkins plugins versions -->
     <jenkins-basic-steps.version>2.3</jenkins-basic-steps.version>

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecDecorator.java

@@ -18,20 +18,19 @@
 
 import static org.csanchez.jenkins.plugins.kubernetes.pipeline.Constants.*;
 
+import java.io.ByteArrayOutputStream;
 import java.io.Closeable;
 import java.io.IOException;
 import java.io.InterruptedIOException;
 import java.io.OutputStream;
 import java.io.PrintStream;
 import java.io.Serializable;
-import java.nio.ByteBuffer;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Objects;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
@@ -268,22 +267,18 @@ private Proc doLaunch(boolean quiet, String[] cmdEnvs, OutputStream outputForCal
                     printStream = new PrintStream(stream, false, StandardCharsets.UTF_8.toString());
                 }
 
-                // we need to keep the last bytes in the stream to parse the exit code as it is printed there
-                // so we use a buffer
-                ExitCodeOutputStream exitCodeOutputStream = new ExitCodeOutputStream();
-                // send container output to all 3 streams (pid, out, job).
-                stream = new TeeOutputStream(exitCodeOutputStream, stream);
                 // Send to proc caller as well if they sent one
                 if (outputForCaller != null) {
                     stream = new TeeOutputStream(outputForCaller, stream);
                 }
+                ByteArrayOutputStream error = new ByteArrayOutputStream();
 
                 String msg = "Executing shell script inside container [" + containerName + "] of pod [" + podName + "]";
                 LOGGER.log(Level.FINEST, msg);
                 printStream.println(msg);
 
                 Execable<String, ExecWatch> execable = client.pods().inNamespace(namespace).withName(podName).inContainer(containerName)
-                        .redirectingInput().writingOutput(stream).writingError(stream)
+                        .redirectingInput().writingOutput(stream).writingError(stream).writingErrorChannel(error)
                         .usingListener(new ExecListener() {
                             @Override
                             public void onOpen(Response response) {
@@ -374,7 +369,7 @@ public void onClose(int i, String s) {
 
                     int pid = readPidFromPidFile(commands);
                     LOGGER.log(Level.INFO, "Created process inside pod: ["+podName+"], container: ["+containerName+"] with pid:["+pid+"]");
-                    ContainerExecProc proc = new ContainerExecProc(watch, alive, finished, exitCodeOutputStream::getExitCode);
+                    ContainerExecProc proc = new ContainerExecProc(watch, alive, finished, error);
                     processes.put(pid, proc);
                     closables.add(proc);
                     return proc;
@@ -481,10 +476,10 @@ private static void doExec(ExecWatch watch, PrintStream out, boolean[] masks, St
 
             // get the command exit code and print it padded so it is easier to parse in ContainerExecProc
             // We need to exit so that we know when the command has finished.
-            sb.append(ExitCodeOutputStream.EXIT_COMMAND);
-            out.print(ExitCodeOutputStream.EXIT_COMMAND);
+            sb.append(EXIT + NEWLINE);
+            out.print(EXIT + NEWLINE);
             LOGGER.log(Level.FINEST, "Executing command: {0}", sb);
-            watch.getInput().write(ExitCodeOutputStream.EXIT_COMMAND.getBytes(StandardCharsets.UTF_8));
+            watch.getInput().write((EXIT + NEWLINE).getBytes(StandardCharsets.UTF_8));
 
             out.flush();
             watch.getInput().flush();
@@ -569,49 +564,4 @@ private static void closeWatch(ExecWatch watch) {
     public void setKubernetesClient(KubernetesClient client) {
         this.client = client;
     }
-
-    /**
-     * Keeps the last bytes of the output stream to parse the exit code
-     */
-    static class ExitCodeOutputStream extends OutputStream {
-
-        public static final String EXIT_COMMAND_TXT = "EXITCODE";
-        public static final String EXIT_COMMAND = "printf \"" + EXIT_COMMAND_TXT + " %3d\" $?; " + EXIT + NEWLINE;
-
-        private EvictingQueue<Integer> queue = EvictingQueue.create(20);
-
-        public ExitCodeOutputStream() {
-        }
-
-        @Override
-        public void write(int b) throws IOException {
-            queue.add(b);
-            byte[] bb = new byte[]{(byte) b};
-            System.out.print(new String(bb, StandardCharsets.UTF_8));
-        }
-
-        public int getExitCode() {
-            ByteBuffer b = ByteBuffer.allocate(queue.size());
-            queue.stream().filter(Objects::nonNull).forEach((i) -> b.put((byte) i.intValue()));
-            // output ends in a 3 digit padded exit code + newline (13 10)
-            // as defined in ContainerExecDecorator#doExec
-            // ie. 32 32 49 13 10 for exit code 1
-            int i = 1;
-            String s = new String(b.array(), StandardCharsets.UTF_8);
-            if (s.indexOf(EXIT_COMMAND_TXT) < 0) {
-                LOGGER.log(Level.WARNING, "Unable to find \"{0}\" in {1}", new Object[]{EXIT_COMMAND_TXT, s});
-                return i;
-            }
-            // parse the exitcode int printed after EXITCODE
-            int start = s.indexOf(EXIT_COMMAND_TXT) + EXIT_COMMAND_TXT.length();
-            s = s.substring(start, start + 4).trim();
-            try {
-                i = Integer.parseInt(s);
-            } catch (NumberFormatException e) {
-                LOGGER.log(Level.WARNING, "Unable to parse exit code as integer: \"{0}\" {1} / {2}",
-                        new Object[]{s, queue.toString(), Arrays.toString(b.array())});
-            }
-            return i;
-        }
-    }
 }

src/main/java/org/csanchez/jenkins/plugins/kubernetes/pipeline/ContainerExecProc.java

@@ -2,6 +2,7 @@
 
 import static org.csanchez.jenkins.plugins.kubernetes.pipeline.Constants.*;
 
+import java.io.ByteArrayOutputStream;
 import java.io.Closeable;
 import java.io.IOException;
 import java.io.InputStream;
@@ -13,6 +14,9 @@
 import java.util.logging.Level;
 import java.util.logging.Logger;
 
+import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.ObjectMapper;
+
 import hudson.Proc;
 import io.fabric8.kubernetes.client.dsl.ExecWatch;
 
@@ -27,22 +31,20 @@ public class ContainerExecProc extends Proc implements Closeable {
     private final AtomicBoolean alive;
     private final CountDownLatch finished;
     private final ExecWatch watch;
-    private final Callable<Integer> exitCode;
-
-    /**
-     * 
-     * @param watch
-     * @param alive
-     * @param finished
-     * @param exitCode
-     *            a way to get the exit code
-     */
+    private final ByteArrayOutputStream error;
+
+    @Deprecated
     public ContainerExecProc(ExecWatch watch, AtomicBoolean alive, CountDownLatch finished,
             Callable<Integer> exitCode) {
+        this(watch, alive, finished, new ByteArrayOutputStream());
+    }
+
+    public ContainerExecProc(ExecWatch watch, AtomicBoolean alive, CountDownLatch finished,
+            ByteArrayOutputStream error) {
         this.watch = watch;
         this.alive = alive;
         this.finished = finished;
-        this.exitCode = exitCode;
+        this.error = error;
     }
 
     @Override
@@ -71,7 +73,38 @@ public int join() throws IOException, InterruptedException {
             LOGGER.log(Level.FINEST, "Waiting for websocket to close on command finish ({0})", finished);
             finished.await();
             LOGGER.log(Level.FINEST, "Command is finished ({0})", finished);
-            return exitCode.call();
+            if (error.size() == 0) {
+                return 0;
+            } else {
+                // {"metadata":{},"status":"Success"}
+                // or
+                // {"metadata":{},"status":"Failure",
+                //   "message":"command terminated with non-zero exit code: Error executing in Docker Container: 127",
+                //   "reason":"NonZeroExitCode",
+                //   "details":{"causes":[{"reason":"ExitCode","message":"127"}]}}
+                try {
+                    ObjectMapper mapper = new ObjectMapper();
+                    JsonNode errorJson = mapper.readTree(error.toByteArray());
+                    if ("Success".equalsIgnoreCase(errorJson.get("status").asText())) {
+                        return 0;
+                    }
+                    JsonNode causes = errorJson.get("details").get("causes");
+                    if (causes.isArray()) {
+                        for (JsonNode cause : causes) {
+                            if ("ExitCode".equalsIgnoreCase(cause.get("reason").asText(""))) {
+                                return cause.get("message").asInt();
+                            }
+                        }
+                    }
+                    LOGGER.log(Level.WARNING, "Unable to parse exit code from error message: {0}",
+                            error.toString(StandardCharsets.UTF_8.name()));
+                    return -1;
+                } catch (IOException e) {
+                    LOGGER.log(Level.WARNING, "Unable to parse exit code from error message: "
+                            + error.toString(StandardCharsets.UTF_8.name()), e);
+                    return -1;
+                }
+            }
         } catch (Exception e) {
             LOGGER.log(Level.WARNING, "Error getting exit code", e);
             return -1;
@@ -98,7 +131,7 @@ public OutputStream getStdin() {
     @Override
     public void close() throws IOException {
         try {
-            //We are calling explicitly close, in order to cleanup websockets and threads (are not closed implicitly).
+            // We are calling explicitly close, in order to cleanup websockets and threads (are not closed implicitly).
             watch.close();
         } catch (Exception e) {
             LOGGER.log(Level.INFO, "failed to close watch", e);