From 9c8d2f466adbc4f948fb659ed8f92f0aad5e6d6f Mon Sep 17 00:00:00 2001 From: Devin Nusbaum Date: Mon, 6 Dec 2021 10:29:31 -0500 Subject: [PATCH] Recover gracefully when a `PlaceholderTask` is in the queue but the associated build is complete (#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 * 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 Co-authored-by: Jesse Glick --- pom.xml | 1 + .../support/steps/ExecutorStepExecution.java | 14 +++++++ .../support/steps/ExecutorStepTest.java | 42 +++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/pom.xml b/pom.xml index ceec031c..a8a8ba92 100644 --- a/pom.xml +++ b/pom.xml @@ -71,6 +71,7 @@ true jenkinsci/${project.artifactId}-plugin 2.40 + 1666.vd1360abbfe9e diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecution.java b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecution.java index c81bf0c1..77fb8e6f 100644 --- a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecution.java +++ b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecution.java @@ -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; } diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepTest.java index 3eae0921..e7269c84 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepTest.java @@ -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; @@ -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; @@ -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;