[JENKINS-27035] Gathering command read/write events (#3071)

* Slave.JnlpJar.getURL did not work in some modes when core had a snapshot dependency on Remoting. * Starting to implement Channel.Listener.read/write. * Now obtaining response timing statistics. * For now, avoiding timestamped snapshots, as it caused problems for #3120 which I have asked for help from @stephenc diagnosing. * Simplified logging a bit. * onResponse * hudson.FilePath$Mkdirs is a lot more readable than hudson.FilePath$13. * Specific snapshot. * onJar * LoggingChannelListener * remoting.version=3.17 * Making a few test assertions more lenient to adapt to jenkinsci/remoting#247.
jenkinsci · Feb 6, 2018 · f4b7892 · f4b7892
1 parent 01a6558
commit f4b7892
Show file tree

Hide file tree

Showing 6 changed files with 25 additions and 16 deletions.
diff --git a/core/src/main/java/hudson/FilePath.java b/core/src/main/java/hudson/FilePath.java
@@ -1167,19 +1167,22 @@ public VirtualFile toVirtualFile() {
      * Creates this directory.
      */
     public void mkdirs() throws IOException, InterruptedException {
-        if(!act(new SecureFileCallable<Boolean>() {
-            private static final long serialVersionUID = 1L;
-            public Boolean invoke(File f, VirtualChannel channel) throws IOException, InterruptedException {
-                if(mkdirs(f) || f.exists())
-                    return true;    // OK
+        if (!act(new Mkdirs())) {
+            throw new IOException("Failed to mkdirs: " + remote);
+        }
+    }
+    private class Mkdirs extends SecureFileCallable<Boolean> {
+        private static final long serialVersionUID = 1L;
+        @Override
+        public Boolean invoke(File f, VirtualChannel channel) throws IOException, InterruptedException {
+            if(mkdirs(f) || f.exists())
+                return true;    // OK
 
-                // following Ant <mkdir> task to avoid possible race condition.
-                Thread.sleep(10);
+            // following Ant <mkdir> task to avoid possible race condition.
+            Thread.sleep(10);
 
-                return mkdirs(f) || f.exists();
-            }
-        }))
-            throw new IOException("Failed to mkdirs: "+remote);
+            return mkdirs(f) || f.exists();
+        }
     }
 
     /**
@@ -2828,6 +2831,11 @@ public ClassLoader getClassLoader() {
             return classLoader;
         }
 
+        @Override
+        public String toString() {
+            return callable.toString();
+        }
+
         private static final long serialVersionUID = 1L;
     }
 

diff --git a/core/src/main/java/hudson/slaves/SlaveComputer.java b/core/src/main/java/hudson/slaves/SlaveComputer.java
@@ -88,6 +88,7 @@
 import java.util.logging.Logger;
 
 import static hudson.slaves.SlaveComputer.LogHolder.SLAVE_LOG_HANDLER;
+import org.jenkinsci.remoting.util.LoggingChannelListener;
 
 
 /**
@@ -520,7 +521,7 @@ public void setChannel(Channel channel, OutputStream launchLog, Channel.Listener
 
         channel.setProperty(SlaveComputer.class, this);
 
-        channel.addListener(new Channel.Listener() {
+        channel.addListener(new LoggingChannelListener(logger, Level.FINEST) {
             @Override
             public void onClosed(Channel c, IOException cause) {
                 // Orderly shutdown will have null exception

diff --git a/pom.xml b/pom.xml
@@ -105,7 +105,7 @@ THE SOFTWARE.
     <maven-war-plugin.version>3.0.0</maven-war-plugin.version> <!-- JENKINS-47127 bump when 3.2.0 is out. Cf. MWAR-407 -->
 
     <!-- Minimum Remoting version, which is tested for API compatibility -->
-    <remoting.version>3.16</remoting.version>
+    <remoting.version>3.17</remoting.version>
     <remoting.minimum.supported.version>2.60</remoting.minimum.supported.version>
 
   </properties>

diff --git a/test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java b/test/src/test/java/hudson/bugs/JnlpAccessWithSecuredHudsonTest.java
@@ -139,7 +139,7 @@ public void serviceUsingDirectSecret() throws Exception {
             assertTrue(f.exists());
             try {
                 fail("SECURITY-206: " + channel.call(new Attack(f.getAbsolutePath())));
-            } catch (SecurityException x) {
+            } catch (Exception x) {
                 assertThat(Functions.printThrowable(x), containsString("https://jenkins.io/redirect/security-144"));
             }
         } finally {

diff --git a/test/src/test/java/jenkins/security/Security218Test.java b/test/src/test/java/jenkins/security/Security218Test.java
@@ -75,7 +75,7 @@ private void check(DumbSlave s) throws Exception {
         try {
             Object o = s.getComputer().getChannel().call(new EvilReturnValue());
             fail("Expected the connection to die: " + o);
-        } catch (SecurityException e) {
+        } catch (Exception e) {
             assertThat(e.getMessage(), containsString(MethodClosure.class.getName()));
         }
     }

diff --git a/test/src/test/java/jenkins/security/s2m/DefaultFilePathFilterTest.java b/test/src/test/java/jenkins/security/s2m/DefaultFilePathFilterTest.java
@@ -63,7 +63,7 @@ public void setUp() {
         try {
             s.getChannel().call(new ReverseCallable(reverse));
             fail("should have failed");
-        } catch (SecurityException x) {
+        } catch (Exception x) {
             // good
 
             // make sure that the stack trace contains the call site info to help assist diagnosis