Merge pull request #4967 from sahilkhatri09/fix-exec-exit-code-overwrite

Keep received exec exit status when the connection fails on close

Author: kubernetes-prow[bot]

util/src/main/java/io/kubernetes/client/Exec.java

@@ -556,10 +556,15 @@ public void failure(Throwable ex) {
               ExecProcess.this.onUnhandledError.accept(ex);
 
               synchronized (ExecProcess.this) {
-                // Try for a pretty unique error code, so if someone searches
-                // they'll find this
-                // code.
-                statusCode = -1975219;
+                // The server may close the connection right after sending the exit status, so
+                // closing our side can fail after the command finished. Keep a received exit
+                // status in that case.
+                if (statusCode < 0) {
+                  // Try for a pretty unique error code, so if someone searches
+                  // they'll find this
+                  // code.
+                  statusCode = -1975219;
+                }
                 isAlive = false;
                 ExecProcess.this.latch.countDown();
               }

util/src/test/java/io/kubernetes/client/ExecTest.java

@@ -204,6 +204,29 @@ void customUnhandledError() throws IOException, InterruptedException {
     assertThat(process.exitValue()).isEqualTo(EXPECTED_ERROR_EXIT_CODE);
   }
 
+  @ParameterizedTest
+  @MethodSource("exitStatusBeforeFailureData")
+  void unhandledErrorAfterExitStatusKeepsExitCode(String status, int expectedExitCode)
+      throws IOException, InterruptedException {
+    final Consumer<Throwable> consumer = mock(Consumer.class);
+    final Throwable throwable = mock(Throwable.class);
+    final ExecProcess process = new ExecProcess(client, consumer);
+    process.getHandler().open("wss", null);
+
+    process.getHandler().bytesMessage(makeStream(3, status.getBytes(StandardCharsets.UTF_8)));
+    // e.g. sending the close frame fails because the server already closed the connection
+    process.getHandler().failure(throwable);
+    process.waitFor();
+
+    verify(consumer, times(1)).accept(throwable);
+    assertThat(process.isAlive()).isFalse();
+    assertThat(process.exitValue()).isEqualTo(expectedExitCode);
+  }
+
+  private static Stream<Arguments> exitStatusBeforeFailureData() {
+    return Stream.of(arguments(OUTPUT_EXIT0, 0), arguments(OUTPUT_EXIT126, 126));
+  }
+
   @Test
   void url() throws IOException, ApiException, InterruptedException {
     final Consumer<Throwable> consumer = mock(Consumer.class);