Merge pull request kubernetes-client#697 from brendandburns/pf

k8s-ci-robot · web-flow · commit 10320efd8007 · 2019-09-16T20:32:23.000-07:00
Fix error handling.
diff --git a/examples/src/main/java/io/kubernetes/client/examples/WebSocketsExample.java b/examples/src/main/java/io/kubernetes/client/examples/WebSocketsExample.java
@@ -49,6 +49,10 @@ public void close() {
 
           public void bytesMessage(InputStream is) {}
 
+          public void failure(Exception ex) {
+            ex.printStackTrace();
+          }
+
           public void textMessage(Reader in) {
             try {
               BufferedReader reader = new BufferedReader(in);
diff --git a/util/src/main/java/io/kubernetes/client/Copy.java b/util/src/main/java/io/kubernetes/client/Copy.java
@@ -143,6 +143,14 @@ public void copyDirectoryFromPod(
         }
       }
     }
+    try {
+      int status = proc.waitFor();
+      if (status != 0) {
+        throw new IOException("Copy command failed with status " + status);
+      }
+    } catch (InterruptedException ex) {
+      throw new IOException(ex);
+    }
   }
 
   public static void copyFileFromPod(String namespace, String pod, String srcPath, Path dest)
diff --git a/util/src/main/java/io/kubernetes/client/Exec.java b/util/src/main/java/io/kubernetes/client/Exec.java
@@ -278,6 +278,22 @@ protected void handleMessage(int stream, InputStream inStream) throws IOExceptio
               } else super.handleMessage(stream, inStream);
             }
 
+            @Override
+            public void failure(Exception ex) {
+              super.failure(ex);
+              // TODO, it's possible we should suppress this error message, but currently there's
+              // no good place to surface the message, and without it, this will be really hard to
+              // debug.
+              ex.printStackTrace();
+              synchronized (ExecProcess.this) {
+                // Try for a pretty unique error code, so if someone searches they'll find this
+                // code.
+                statusCode = -1975219;
+                isAlive = false;
+                ExecProcess.this.notifyAll();
+              }
+            }
+
             @Override
             public void close() {
               // notify of process completion
diff --git a/util/src/main/java/io/kubernetes/client/util/WebSocketStreamHandler.java b/util/src/main/java/io/kubernetes/client/util/WebSocketStreamHandler.java
@@ -43,6 +43,7 @@ public class WebSocketStreamHandler implements WebSockets.SocketListener, Closea
   private final Map<Integer, PipedOutputStream> pipedOutput = new HashMap<>();
   private final Map<Integer, OutputStream> output = new HashMap<>();
   private WebSocket socket;
+  private Exception error;
 
   @SuppressWarnings("unused")
   private String protocol;
@@ -88,6 +89,15 @@ protected void handleMessage(int stream, InputStream inStream) throws IOExceptio
     ByteStreams.copy(inStream, out);
   }
 
+  @Override
+  public void failure(Exception ex) {
+    this.error = ex;
+  }
+
+  public Exception getError() {
+    return this.error;
+  }
+
   @Override
   public synchronized void close() {
     if (state != State.CLOSED) {
diff --git a/util/src/main/java/io/kubernetes/client/util/WebSockets.java b/util/src/main/java/io/kubernetes/client/util/WebSockets.java
@@ -62,6 +62,13 @@ public interface SocketListener {
      */
     public void textMessage(Reader in);
 
+    /**
+     * Called when there has been a failure
+     *
+     * @param the exception associated with the failure.
+     */
+    public void failure(Exception ex);
+
     /** Called when the stream is closed. */
     public void close();
   }
@@ -147,7 +154,7 @@ public void onClose(int code, String reason) {
 
     @Override
     public void onFailure(IOException e, Response res) {
-      e.printStackTrace();
+      listener.failure(e);
       listener.close();
     }
   }
diff --git a/util/src/test/java/io/kubernetes/client/ExecTest.java b/util/src/test/java/io/kubernetes/client/ExecTest.java
@@ -152,7 +152,7 @@ public void testUrl() throws IOException, ApiException, InterruptedException {
             .withQueryParam("tty", equalTo("false"))
             .withQueryParam("command", equalTo("cmd")));
 
-    assertEquals(-1, p.exitValue());
+    assertEquals(-1975219, p.exitValue());
   }
 
   @Test

Original file line number	Diff line number	Diff line change
`@@ -143,6 +143,14 @@ public void copyDirectoryFromPod(`
`143`	`143`	`}`
`144`	`144`	`}`
`145`	`145`	`}`
	`146`	`+ try {`
	`147`	`+ int status = proc.waitFor();`
	`148`	`+ if (status != 0) {`
	`149`	`+ throw new IOException("Copy command failed with status " + status);`
	`150`	`+ }`
	`151`	`+ } catch (InterruptedException ex) {`
	`152`	`+ throw new IOException(ex);`
	`153`	`+ }`
`146`	`154`	`}`
`147`	`155`
`148`	`156`	`public static void copyFileFromPod(String namespace, String pod, String srcPath, Path dest)`
Original file line number	Diff line number	Diff line change
`@@ -152,7 +152,7 @@ public void testUrl() throws IOException, ApiException, InterruptedException {`
`152`	`152`	`.withQueryParam("tty", equalTo("false"))`
`153`	`153`	`.withQueryParam("command", equalTo("cmd")));`
`154`	`154`
`155`		`- assertEquals(-1, p.exitValue());`
	`155`	`+ assertEquals(-1975219, p.exitValue());`
`156`	`156`	`}`
`157`	`157`
`158`	`158`	`@Test`