Merge pull request #1092 from jbrejner/master

[FIXED JENKINS-21394] Avoid irrelevant job queing while node is offline

Conflicts:
	test/src/test/java/hudson/model/ProjectTest.java

core/src/main/java/hudson/model/AbstractProject.java

@@ -67,6 +67,7 @@ import hudson.scm.SCMS;
 import hudson.search.SearchIndexBuilder;
 import hudson.security.ACL;
 import hudson.security.Permission;
+import hudson.slaves.Cloud;
 import hudson.slaves.WorkspaceList;
 import hudson.tasks.BuildStep;
 import hudson.tasks.BuildStepDescriptor;
@@ -1375,7 +1376,6 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
                 // At this point we start thinking about triggering a build just to get a workspace,
                 // because otherwise there's no way we can detect changes.
                 // However, first there are some conditions in which we do not want to do so.
-
                 // give time for slaves to come online if we are right after reconnection (JENKINS-8408)
                 long running = Jenkins.getInstance().getInjector().getInstance(Uptime.class).getUptime();
                 long remaining = TimeUnit2.MINUTES.toMillis(10)-running;
@@ -1385,6 +1385,14 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
                     return NO_CHANGES;
                 }
 
+                // Do not trigger build, if no suitable slave is online
+                if (workspaceOfflineReason.equals(WorkspaceOfflineReason.all_suitable_nodes_are_offline)) {
+                    // No suitable executor is online
+                    listener.getLogger().print(Messages.AbstractProject_AwaitingWorkspaceToComeOnline(running/1000));
+                    listener.getLogger().println( " (" + workspaceOfflineReason.name() + ")");
+                    return NO_CHANGES;
+                }
+
                 Label label = getAssignedLabel();
                 if (label != null && label.isSelfLabel()) {
                     // if the build is fixed on a node, then attempting a build will do us
@@ -1409,12 +1417,11 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
             } else {
                 WorkspaceList l = b.getBuiltOn().toComputer().getWorkspaceList();
                 return pollWithWorkspace(listener, scm, b, ws, l);
-
             }
+			
         } else {
             // polling without workspace
             LOGGER.fine("Polling SCM changes of " + getName());
-
             if (pollingBaseline==null) // see NOTE-NO-BASELINE above
                 calcPollingBaseline(getLastBuild(),null,listener);
             PollingResult r = scm.poll(this, null, null, listener, pollingBaseline);
@@ -1449,11 +1456,50 @@ public abstract class AbstractProject<P extends AbstractProject<P,R>,R extends A
     enum WorkspaceOfflineReason {
         nonexisting_workspace,
         builton_node_gone,
-        builton_node_no_executors
+        builton_node_no_executors,
+        all_suitable_nodes_are_offline,
+        use_ondemand_slave
+    }
+
+    /**
+     * Returns true if all suitable nodes for the job are offline.
+     *
+     */
+
+    private boolean isAllSuitableNodesOffline(R build) {
+        Label label = getAssignedLabel();
+        List<Node> allNodes = Jenkins.getInstance().getNodes();
+
+        if (allNodes.isEmpty() && !(label == Jenkins.getInstance().getSelfLabel())) {
+            // no master/slave. pointless to talk about nodes
+            label = null;
+        }
+
+        if (label != null) {
+            return label.isOffline();
+        } else {
+            if (canRoam) {
+                for (Node n : Jenkins.getInstance().getNodes()) {
+                    Computer c = n.toComputer();
+                    if (c != null && c.isOnline() && c.isAcceptingTasks()) {
+                        // Some executor is ready and this job can run anywhere
+                        return false;
+                    }
+                }
+            }
+        }
+        return true;
     }
 
     private WorkspaceOfflineReason workspaceOffline(R build) throws IOException, InterruptedException {
         FilePath ws = build.getWorkspace();
+        Label label = getAssignedLabel();
+
+        if (isAllSuitableNodesOffline(build)) {            
+            Collection<Cloud> applicableClouds = label == null ? Jenkins.getInstance().clouds : label.getClouds();
+            return applicableClouds.isEmpty() ? WorkspaceOfflineReason.all_suitable_nodes_are_offline : WorkspaceOfflineReason.use_ondemand_slave;            
+        }
+
         if (ws==null || !ws.exists()) {
             return WorkspaceOfflineReason.nonexisting_workspace;
         }

test/src/main/java/org/jvnet/hudson/test/JenkinsRule.java

@@ -876,7 +876,7 @@ public class JenkinsRule implements TestRule, MethodRule, RootAction {
     public DumbSlave createSlave(String nodeName, String labels, EnvVars env) throws Exception {
         synchronized (jenkins) {
             DumbSlave slave = new DumbSlave(nodeName, "dummy",
-    				createTmpDir().getPath(), "1", Node.Mode.NORMAL, labels==null?"":labels, createComputerLauncher(env), RetentionStrategy.NOOP, Collections.EMPTY_LIST);
+    				createTmpDir().getPath(), "1", Node.Mode.NORMAL, labels==null?"":labels, createComputerLauncher(env), RetentionStrategy.NOOP, Collections.EMPTY_LIST);                        
     		jenkins.addNode(slave);
     		return slave;
     	}

test/src/test/java/hudson/model/ProjectTest.java

@@ -60,6 +60,12 @@ import java.io.File;
 import hudson.FilePath;
 import hudson.slaves.EnvironmentVariablesNodeProperty;
 import hudson.EnvVars;
+import hudson.model.labels.LabelAtom;
+import hudson.slaves.Cloud;
+import hudson.slaves.DumbSlave;
+import hudson.slaves.DummyCloudImpl;
+import hudson.slaves.NodeProvisioner;
+import hudson.slaves.OfflineCause;
 import hudson.tasks.Shell;
 import org.jvnet.hudson.test.TestExtension;
 import java.util.List;
@@ -74,12 +80,17 @@ import static org.junit.Assert.*;
 import hudson.tasks.Fingerprinter;
 import hudson.tasks.ArtifactArchiver;
 import hudson.tasks.BuildTrigger;
+import hudson.util.OneShotEvent;
 import java.util.Map;
+import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Logger;
 
 import org.junit.Ignore;
+import org.jvnet.localizer.Localizable;
+import sun.awt.image.OffScreenImage;
 
 /**
  *
@@ -637,6 +648,110 @@ public class ProjectTest {
        assertFalse("Project should be enabled.", project.isDisabled());
     }
     
+    /**
+     * Job is un-restricted (no nabel), this is submitted to queue, which spawns an on demand slave
+     * @throws Exception 
+     */
+    @Test
+    public void testJobSubmittedShouldSpawnCloud() throws Exception {
+        /**
+         * Setup a project with an SCM. Jenkins should have no executors in itself. 
+         */
+        FreeStyleProject proj = j.createFreeStyleProject("JENKINS-21394-spawn");        
+        RequiresWorkspaceSCM requiresWorkspaceScm = new RequiresWorkspaceSCM(true);
+        proj.setScm(requiresWorkspaceScm);        
+        j.jenkins.setNumExecutors(0);        
+        /*
+         * We have a cloud
+         */
+        DummyCloudImpl2 c2 = new DummyCloudImpl2(j, 0);
+        c2.label = new LabelAtom("test-cloud-label");        
+        j.jenkins.clouds.add(c2);
+        
+        SCMTrigger t = new SCMTrigger("@daily", true);
+        t.start(proj, true);
+        proj.addTrigger(t);
+        t.new Runner().run();
+        
+        Thread.sleep(1000);
+        //Assert that the job IS submitted to Queue.
+        assertEquals(1, j.jenkins.getQueue().getItems().length);        
+    }
+    
+    /**
+     * Job is restricted, but label can not be provided by any cloud, only normal slaves. Then job will not submit, because no slave is available.
+     * @throws Exception
+     */
+    @Test
+    public void testUnrestrictedJobNoLabelByCloudNoQueue() throws Exception {
+        assertTrue(j.jenkins.clouds.isEmpty());
+        //Create slave. (Online)
+        Slave s1 = j.createOnlineSlave();
+        
+        //Create a project, and bind the job to the created slave
+        FreeStyleProject proj = j.createFreeStyleProject("JENKINS-21394-noqueue");
+        proj.setAssignedLabel(s1.getSelfLabel());
+                        
+        //Add an SCM to the project. We require a workspace for the poll
+        RequiresWorkspaceSCM requiresWorkspaceScm = new RequiresWorkspaceSCM(true);
+        proj.setScm(requiresWorkspaceScm);
+ 
+        j.buildAndAssertSuccess(proj);        
+
+        //Now create another slave. And restrict the job to that slave. The slave is offline, leaving the job with no assignable nodes.
+        //We tell our mock SCM to return that it has got changes. But since there are no slaves, we get the desired result. 
+        Slave s2 = j.createSlave();
+        proj.setAssignedLabel(s2.getSelfLabel());
+        requiresWorkspaceScm.hasChange = true;
+        
+        //Poll (We now should have NO online slaves, this should now return NO_CHANGES.
+        PollingResult pr = proj.poll(j.createTaskListener());
+        assertFalse(pr.hasChanges());
+        
+        SCMTrigger t = new SCMTrigger("@daily", true);
+        t.start(proj, true);
+        proj.addTrigger(t);
+        
+        t.new Runner().run();
+        
+        /**
+         * Assert that the log contains the correct message.
+         */
+        HtmlPage log = j.createWebClient().getPage(proj, "scmPollLog");
+        String logastext = log.asText();
+        assertTrue(logastext.contains("(" + AbstractProject.WorkspaceOfflineReason.all_suitable_nodes_are_offline.name() + ")"));
+        
+    }
+    
+    /**
+     * Job is restricted. Label is on slave that can be started in cloud. Job is submitted to queue, which spawns an on demand slave.
+     * @throws Exception 
+     */
+    @Test
+    public void testRestrictedLabelOnSlaveYesQueue() throws Exception {        
+        FreeStyleProject proj = j.createFreeStyleProject("JENKINS-21394-yesqueue");
+        RequiresWorkspaceSCM requiresWorkspaceScm = new RequiresWorkspaceSCM(true);
+        proj.setScm(requiresWorkspaceScm);        
+        j.jenkins.setNumExecutors(0);
+        
+        /*
+         * We have a cloud
+         */
+        DummyCloudImpl2 c2 = new DummyCloudImpl2(j, 0);
+        c2.label = new LabelAtom("test-cloud-label");        
+        j.jenkins.clouds.add(c2);
+        proj.setAssignedLabel(c2.label);
+        
+        SCMTrigger t = new SCMTrigger("@daily", true);
+        t.start(proj, true);
+        proj.addTrigger(t);
+        t.new Runner().run();
+        
+        Thread.sleep(1000);
+        //The job should be in queue
+        assertEquals(1, j.jenkins.getQueue().getItems().length);    
+    }
+    
     public static class TransientAction extends InvisibleAction{
         
     }
@@ -654,6 +769,36 @@ public class ProjectTest {
         
     }
     
+    @TestExtension 
+    public static class RequiresWorkspaceSCM extends NullSCM {
+        
+        public boolean hasChange = false;
+        
+        public RequiresWorkspaceSCM() { } 
+        
+        public RequiresWorkspaceSCM(boolean hasChange) {
+            this.hasChange = hasChange;
+        }
+        
+        @Override
+        public boolean pollChanges(AbstractProject<?, ?> project, Launcher launcher, FilePath workspace, TaskListener listener) throws IOException, InterruptedException {
+            return true;
+        }
+                       
+        @Override
+        public boolean requiresWorkspaceForPolling(){
+            return true;
+        }
+        
+        @Override
+        protected PollingResult compareRemoteRevisionWith(AbstractProject project, Launcher launcher, FilePath workspace, TaskListener listener, SCMRevisionState baseline) throws IOException, InterruptedException {            
+            if(!hasChange) {
+                return PollingResult.NO_CHANGES;
+            }
+            return PollingResult.SIGNIFICANT;
+        }
+    }
+    
     @TestExtension
     public static class AlwaysChangedSCM extends NullSCM{
 
@@ -661,6 +806,7 @@ public class ProjectTest {
         public boolean pollChanges(AbstractProject<?, ?> project, Launcher launcher, FilePath workspace, TaskListener listener) throws IOException, InterruptedException {
             return true;
         }
+        
         @Override
         public boolean requiresWorkspaceForPolling(){
             return false;
@@ -748,4 +894,94 @@ public class ProjectTest {
     public class ActionImpl extends InvisibleAction{
         
     }
+    
+    @TestExtension
+    public static class DummyCloudImpl2 extends Cloud {
+        private final transient JenkinsRule caller;
+
+        /**
+         * Configurable delay between the {@link Cloud#provision(Label,int)} and the actual launch of a slave,
+         * to emulate a real cloud that takes some time for provisioning a new system.
+         *
+         * <p>
+         * Number of milliseconds.
+         */
+        private final int delay;
+
+        // stats counter to perform assertions later
+        public int numProvisioned;
+
+        /**
+         * Only reacts to provisioning for this label.
+         */
+        public Label label;
+        
+        public DummyCloudImpl2() { 
+            super("test");
+            this.delay = 0;
+            this.caller = null;
+        }
+
+        public DummyCloudImpl2(JenkinsRule caller, int delay) {
+            super("test");
+            this.caller = caller;
+            this.delay = delay;
+        }
+
+        @Override
+        public Collection<NodeProvisioner.PlannedNode> provision(Label label, int excessWorkload) {
+            List<NodeProvisioner.PlannedNode> r = new ArrayList<NodeProvisioner.PlannedNode>();
+
+            //Always provision...even if there is no workload.
+            while(excessWorkload >= 0) {
+                System.out.println("Provisioning");
+                numProvisioned++;
+                Future<Node> f = Computer.threadPoolForRemoting.submit(new ProjectTest.DummyCloudImpl2.Launcher(delay));
+                r.add(new NodeProvisioner.PlannedNode(name+" #"+numProvisioned,f,1));
+                excessWorkload-=1;
+            }
+            return r;
+        }
+
+        @Override
+        public boolean canProvision(Label label) {
+            //This cloud can ALWAYS provision 
+           return true;
+            /* return label==this.label; */
+        }
+
+        private final class Launcher implements Callable<Node> {
+            private final long time;
+            /**
+             * This is so that we can find out the status of Callable from the debugger.
+             */
+            private volatile Computer computer;
+
+            private Launcher(long time) {
+                this.time = time;
+            }
+
+            @Override
+            public Node call() throws Exception {
+                // simulate the delay in provisioning a new slave,
+                // since it's normally some async operation.
+                Thread.sleep(time);
+
+                System.out.println("launching slave");
+                DumbSlave slave = caller.createSlave(label);
+                computer = slave.toComputer();
+                computer.connect(false).get();
+                synchronized (ProjectTest.DummyCloudImpl2.this) {
+                    System.out.println(computer.getName()+" launch"+(computer.isOnline()?"ed successfully":" failed"));
+                    System.out.println(computer.getLog());
+                }
+                return slave;
+            }
+        }
+
+        @Override
+        public Descriptor<Cloud> getDescriptor() {
+            throw new UnsupportedOperationException();
+        }
+    }
 }