Skip to content

Commit

Permalink
Recover gracefully when a PlaceholderTask is in the queue but the a…
Browse files Browse the repository at this point in the history
…ssociated build is complete (jenkinsci#185)

* Add ignored test where a PlaceholderTask is in the queue but the associated build is complete

* Update annotation to explain why test is ignored

Co-authored-by: Jesse Glick <jglick@cloudbees.com>

* Simplify test using JenkinsSessionRule.getHome and TemporaryFolder

* De-flake test and update to released version of jenkins-test-harness

* Check if build is complete in PlaceholderTask.getCauseOfBlockage and if so, block execution and cancel task

* Simplify lambda in PlaceholderTask.getCauseOfBlockage

Co-authored-by: Jesse Glick <jglick@cloudbees.com>

Co-authored-by: Jesse Glick <jglick@cloudbees.com>
  • Loading branch information
dwnusbaum and jglick authored Dec 6, 2021
1 parent f832bc1 commit 9c8d2f4
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 0 deletions.
1 change: 1 addition & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@
<useBeta>true</useBeta>
<gitHubRepo>jenkinsci/${project.artifactId}-plugin</gitHubRepo>
<hpi.compatibleSinceVersion>2.40</hpi.compatibleSinceVersion>
<jenkins-test-harness.version>1666.vd1360abbfe9e</jenkins-test-harness.version>
</properties>
<dependencyManagement>
<dependencies>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,20 @@ public String getCookie() {
}

@Override public CauseOfBlockage getCauseOfBlockage() {
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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import hudson.model.FreeStyleProject;
import hudson.model.Item;
import hudson.model.Job;
import hudson.model.Label;
import hudson.model.Node;
import hudson.model.Queue;
import hudson.model.Result;
Expand Down Expand Up @@ -79,6 +80,7 @@

import hudson.util.VersionNumber;
import java.nio.charset.StandardCharsets;
import java.nio.file.StandardCopyOption;
import java.util.Set;
import jenkins.model.Jenkins;
import jenkins.security.MasterToSlaveCallable;
Expand Down Expand Up @@ -1264,6 +1266,46 @@ public void accessPermittedOnlyFromCurrentBuild() throws Throwable {
r.buildAndAssertSuccess(main);
});
}

@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);
while (Queue.getInstance().getItems().length > 0) {
Thread.sleep(100L);
}
assertThat(logging.getMessages(), hasItem(startsWith("Refusing to build ExecutorStepExecution.PlaceholderTask{runId=p#")));
});
}

public static final class WriteBackStep extends Step {
static File controllerFile;
static boolean legal = true;
Expand Down

0 comments on commit 9c8d2f4

Please sign in to comment.