[FIXED JENKINS-20272] Don't monitor response on offline agents (#2911)

* [FIXED JENKINS-20272] Don't monitor response on offline agents * Updating to only not check if channel is null. * Fix broken test.
jenkinsci · Jul 30, 2017 · 5f125d1 · 5f125d1
1 parent 564ca0b
commit 5f125d1
Show file tree

Hide file tree

Showing 2 changed files with 51 additions and 1 deletion.
diff --git a/core/src/main/java/hudson/node_monitors/ResponseTimeMonitor.java b/core/src/main/java/hudson/node_monitors/ResponseTimeMonitor.java
@@ -23,7 +23,6 @@
  */
 package hudson.node_monitors;
 
-import hudson.Util;
 import hudson.Extension;
 import hudson.model.Computer;
 import hudson.remoting.Callable;
@@ -49,6 +48,9 @@ public class ResponseTimeMonitor extends NodeMonitor {
     public static final AbstractNodeMonitorDescriptor<Data> DESCRIPTOR = new AbstractAsyncNodeMonitorDescriptor<Data>() {
         @Override
         protected Callable<Data,IOException> createCallable(Computer c) {
+            if (c.getChannel() == null) {
+                return null;
+            }
             return new Step1(get(c));
         }
 

diff --git a/test/src/test/java/hudson/node_monitors/ResponseTimeMonitorTest.java b/test/src/test/java/hudson/node_monitors/ResponseTimeMonitorTest.java
@@ -0,0 +1,48 @@
+package hudson.node_monitors;
+
+import hudson.model.User;
+import hudson.slaves.DumbSlave;
+import hudson.slaves.OfflineCause;
+import hudson.slaves.SlaveComputer;
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
+/**
+ * @author Andrew Bayer
+ */
+public class ResponseTimeMonitorTest {
+
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+
+    /**
+     * Makes sure that it doesn't try to monitor an already-offline agent.
+     */
+    @Test
+    @Issue("JENKINS-20272")
+    public void skipOfflineAgent() throws Exception {
+        DumbSlave s = j.createSlave();
+        SlaveComputer c = s.getComputer();
+        c.connect(false).get(); // wait until it's connected
+
+        // Try as temporarily offline first.
+        c.setTemporarilyOffline(true, new OfflineCause.UserCause(User.getUnknown(), "Temporarily offline"));
+        assertNotNull(ResponseTimeMonitor.DESCRIPTOR.monitor(c));
+
+        // Now try as actually disconnected.
+        c.setTemporarilyOffline(false, null);
+        j.disconnectSlave(s);
+        assertNull(ResponseTimeMonitor.DESCRIPTOR.monitor(c));
+
+        // Now reconnect and make sure we get a non-null response.
+        c.connect(false).get(); // wait until it's connected
+
+        assertNotNull(ResponseTimeMonitor.DESCRIPTOR.monitor(c));
+    }
+
+}