Merge pull request #226 from jglick/stale-placeholdertask

Recover gracefully when a `PlaceholderTask` is in the queue but the associated build is complete (II)

pom.xml

@@ -95,6 +95,7 @@
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-api</artifactId>
+            <version>1162.va_1e49062a_00e</version>
         </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>

src/main/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecution.java

@@ -65,6 +65,7 @@
 import org.jenkinsci.plugins.workflow.actions.QueueItemAction;
 import org.jenkinsci.plugins.workflow.actions.ThreadNameAction;
 import org.jenkinsci.plugins.workflow.flow.FlowExecution;
+import org.jenkinsci.plugins.workflow.flow.FlowExecutionList;
 import org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner;
 import org.jenkinsci.plugins.workflow.graph.FlowNode;
 import org.jenkinsci.plugins.workflow.steps.AbstractStepExecutionImpl;
@@ -423,6 +424,23 @@ public String getCookie() {
         }
 
         @Override public CauseOfBlockage getCauseOfBlockage() {
+            if (FlowExecutionList.get().isResumptionComplete()) {
+                // We only do this if resumption is complete so that we do not load the run and resume its execution in this context in normal scenarios.
+                Run<?, ?> run = runForDisplay();
+                if (!stopping && run != null && !run.isLogUpdated()) {
+                    stopping = true;
+                    LOGGER.warning(() -> "Refusing to build " + this + " and cancelling it because associated build is complete");
+                    Timer.get().execute(() -> Queue.getInstance().cancel(this));
+                }
+            }
+            if (stopping) {
+                return new CauseOfBlockage() {
+                    @Override
+                    public String getShortDescription() {
+                        return "Stopping " + getDisplayName();
+                    }
+                };
+            }
             return null;
         }
 

src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepTest.java

@@ -34,6 +34,7 @@
 import hudson.model.Executor;
 import hudson.model.Item;
 import hudson.model.Job;
+import hudson.model.Label;
 import hudson.model.Node;
 import hudson.model.Queue;
 import hudson.model.Result;
@@ -56,6 +57,7 @@
 import java.io.StringWriter;
 import java.nio.file.Files;
 import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
@@ -1156,6 +1158,46 @@ public void getOwnerTaskPermissions() throws Throwable {
         });
     }
 
+    @Test public void placeholderTaskInQueueButAssociatedBuildComplete() throws Throwable {
+        logging.record(ExecutorStepExecution.class, Level.FINE).capture(50);
+        Path tempQueueFile = tmp.newFile().toPath();
+        sessions.then(r -> {
+            WorkflowJob p = r.createProject(WorkflowJob.class, "p");
+            p.setDefinition(new CpsFlowDefinition("node('custom-label') { }", true));
+            WorkflowRun b = p.scheduleBuild2(0).waitForStart();
+            // Get into a state where a PlaceholderTask is in the queue.
+            while (true) {
+                Queue.Item[] items = Queue.getInstance().getItems();
+                if (items.length == 1 && items[0].task instanceof ExecutorStepExecution.PlaceholderTask) {
+                    break;
+                }
+                Thread.sleep(500L);
+            }
+            // Copy queue.xml to a temp file while the PlaceholderTask is in the queue.
+            r.jenkins.getQueue().save();
+            Files.copy(sessions.getHome().toPath().resolve("queue.xml"), tempQueueFile, StandardCopyOption.REPLACE_EXISTING);
+            // Create a node with the correct label and let the build complete.
+            DumbSlave node = r.createOnlineSlave(Label.get("custom-label"));
+            r.assertBuildStatusSuccess(r.waitForCompletion(b));
+            // Remove node so that tasks requiring custom-label are stuck in the queue.
+            Jenkins.get().removeNode(node);
+        });
+        // Copy the temp queue.xml over the real one. The associated build has already completed, so the queue now
+        // has a bogus PlaceholderTask.
+        Files.copy(tempQueueFile, sessions.getHome().toPath().resolve("queue.xml"), StandardCopyOption.REPLACE_EXISTING);
+        sessions.then(r -> {
+            WorkflowJob p = r.jenkins.getItemByFullName("p", WorkflowJob.class);
+            WorkflowRun b = p.getBuildByNumber(1);
+            assertFalse(b.isLogUpdated());
+            r.assertBuildStatusSuccess(b);
+            Queue.getInstance().maintain(); // Otherwise we may have to wait up to 5 seconds.
+            while (Queue.getInstance().getItems().length > 0) {
+                Thread.sleep(100L);
+            }
+            assertThat(logging.getMessages(), hasItem(startsWith("Refusing to build ExecutorStepExecution.PlaceholderTask{runId=p#")));
+        });
+    }
+
     private static class MainAuthenticator extends QueueItemAuthenticator {
         @Override public Authentication authenticate(Queue.Task task) {
             return task instanceof WorkflowJob ? User.getById("dev", true).impersonate() : null;