From 8010d92d5b24eaf1ab3d4e05be5f8b25a5af181a Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Wed, 20 Nov 2024 18:14:54 +0100 Subject: [PATCH 01/16] Add a failing test --- pom.xml | 7 +- .../ExecutorStepDynamicContextRJRTest.java | 127 ++++++++++++++++++ 2 files changed, 133 insertions(+), 1 deletion(-) create mode 100644 src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepDynamicContextRJRTest.java diff --git a/pom.xml b/pom.xml index d8dd2f07..394a8541 100644 --- a/pom.xml +++ b/pom.xml @@ -67,7 +67,7 @@ 999999-SNAPSHOT - 2182.v0138ccb_c0b_cb_ + 2357.vf2a_982b_b_910f 2.440.1 true jenkinsci/${project.artifactId}-plugin @@ -116,6 +116,11 @@ workflow-basic-steps test + + org.jenkins-ci.plugins + pipeline-input-step + test + org.jenkins-ci.plugins pipeline-stage-step diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepDynamicContextRJRTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepDynamicContextRJRTest.java new file mode 100644 index 00000000..8c5ebcf6 --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepDynamicContextRJRTest.java @@ -0,0 +1,127 @@ +/* + * The MIT License + * + * Copyright (c) 2024, CloudBees, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package org.jenkinsci.plugins.workflow.support.steps; + +import static org.awaitility.Awaitility.await; +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.arrayWithSize; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.iterableWithSize; +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.time.Duration; +import java.util.Arrays; +import java.util.List; +import java.util.concurrent.TimeoutException; +import java.util.logging.Level; +import java.util.logging.Logger; +import org.hamcrest.Description; +import org.hamcrest.TypeSafeMatcher; +import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; +import org.jenkinsci.plugins.workflow.flow.FlowExecutionList; +import org.jenkinsci.plugins.workflow.job.WorkflowJob; +import org.jenkinsci.plugins.workflow.support.steps.input.InputAction; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.InboundAgentRule; +import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.PrefixedOutputStream; +import org.jvnet.hudson.test.RealJenkinsRule; +import org.jvnet.hudson.test.TailLog; + +public class ExecutorStepDynamicContextRJRTest { + private static final Logger LOGGER = Logger.getLogger(ExecutorStepDynamicContextRJRTest.class.getName()); + + @Rule public RealJenkinsRule rjr = new RealJenkinsRule().withColor(PrefixedOutputStream.Color.GREEN).withPackageLogger(ExecutorStepExecution.class, Level.FINE).withPackageLogger(FlowExecutionList.class, Level.FINE); + + @Rule + public InboundAgentRule iar = new InboundAgentRule(); + + @Test public void restartWhileWaitingForANode() throws Throwable { + rjr.startJenkins(); + try (var tailLog = new TailLog(rjr, "p", 1)) { + iar.createAgent(rjr, InboundAgentRule.Options.newBuilder().name("J").label("mib").color(PrefixedOutputStream.Color.YELLOW).webSocket().build()); + rjr.runRemotely(ExecutorStepDynamicContextRJRTest::setupJobAndStart); + rjr.stopJenkinsForcibly(); + rjr.startJenkins(); + rjr.runRemotely(ExecutorStepDynamicContextRJRTest::assertQueueItems); + } + } + + private static void assertQueueItems(JenkinsRule r) throws Throwable { + var p = r.jenkins.getItemByFullName("p", WorkflowJob.class); + var b = p.getBuildByNumber(1); + var items = await("Waiting for agent J to reconnect").atMost(Duration.ofSeconds(30)).until(() -> r.jenkins.getQueue().getItems(), arrayWithSize(1)); + LOGGER.info("Items in queue: " + Arrays.asList(items)); + var actions = await().until(() -> b.getActions(InputAction.class), hasSize(1)); + proceed(actions, "Complete branch 1 ?", p.getName() + "#" + b.number); + actions = await().until(() -> b.getActions(InputAction.class), allOf(iterableWithSize(1), hasItem(new InputActionIsWaitingForInput()))); + r.waitForMessage("Complete branch 2 ?", b); + proceed(actions, "Complete branch 2 ?", p.getName() + "#" + b.number); + r.waitForCompletion(b); + } + + private static class InputActionIsWaitingForInput extends TypeSafeMatcher { + + @Override + protected boolean matchesSafely(InputAction inputAction) { + try { + return inputAction.isWaitingForInput(); + } catch (Exception e) { + return false; + } + } + + @Override + public void describeTo(Description description) { + description.appendText("is waiting for input"); + } + } + + private static void proceed(List actions, String inputMessage, String name) throws InterruptedException, TimeoutException, IOException { + for (var action : actions) { + if (action.getExecutions().isEmpty()) { + continue; + } + var inputStepExecution = action.getExecutions().get(0); + if (inputMessage.equals(inputStepExecution.getDisplayName())) { + LOGGER.info(() -> "proceeding " + name); + inputStepExecution.proceed(null); + break; + } + } + } + + private static void setupJobAndStart(JenkinsRule r) throws Exception { + var p = r.createProject(WorkflowJob.class, "p"); + p.setDefinition(new CpsFlowDefinition("parallel 'Branch 1': { node('mib') { input 'Complete branch 1 ?' } }, 'Branch 2': { sleep 1; node('mib') { input 'Complete branch 2 ?' } }", true)); + var b = p.scheduleBuild2(0).waitForStart(); + r.waitForMessage("Complete branch 1 ?", b); + assertTrue(b.isBuilding()); + await().until(() -> r.jenkins.getQueue().getItems(), arrayWithSize(1)); + } +} From 165eeb051415bf91246ce6a7c651663b96372089 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Thu, 21 Nov 2024 10:03:41 +0100 Subject: [PATCH 02/16] Improve assertions --- .../ExecutorStepDynamicContextRJRTest.java | 34 +++++++++++-------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepDynamicContextRJRTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepDynamicContextRJRTest.java index 8c5ebcf6..db30371b 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepDynamicContextRJRTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepDynamicContextRJRTest.java @@ -29,12 +29,12 @@ import static org.hamcrest.Matchers.arrayWithSize; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.hasSize; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.iterableWithSize; import static org.junit.Assert.assertTrue; import java.io.IOException; import java.time.Duration; -import java.util.Arrays; import java.util.List; import java.util.concurrent.TimeoutException; import java.util.logging.Level; @@ -75,40 +75,46 @@ public class ExecutorStepDynamicContextRJRTest { private static void assertQueueItems(JenkinsRule r) throws Throwable { var p = r.jenkins.getItemByFullName("p", WorkflowJob.class); var b = p.getBuildByNumber(1); - var items = await("Waiting for agent J to reconnect").atMost(Duration.ofSeconds(30)).until(() -> r.jenkins.getQueue().getItems(), arrayWithSize(1)); - LOGGER.info("Items in queue: " + Arrays.asList(items)); - var actions = await().until(() -> b.getActions(InputAction.class), hasSize(1)); - proceed(actions, "Complete branch 1 ?", p.getName() + "#" + b.number); - actions = await().until(() -> b.getActions(InputAction.class), allOf(iterableWithSize(1), hasItem(new InputActionIsWaitingForInput()))); + await("Waiting for agent J to reconnect").atMost(Duration.ofSeconds(30)).until(() -> r.jenkins.getComputer("J").isOnline(), is(true)); + var actions = await().until(() -> b.getActions(InputAction.class), allOf(iterableWithSize(1), hasItem(new InputActionWithId("Branch1")))); + proceed(actions, "Branch1", p.getName() + "#" + b.number); + // This is quicker than waitForMessage that can wait for up to 10 minutes + actions = await().until(() -> b.getActions(InputAction.class), allOf(iterableWithSize(1), hasItem(new InputActionWithId("Branch2")))); r.waitForMessage("Complete branch 2 ?", b); - proceed(actions, "Complete branch 2 ?", p.getName() + "#" + b.number); + proceed(actions, "Branch2", p.getName() + "#" + b.number); r.waitForCompletion(b); } - private static class InputActionIsWaitingForInput extends TypeSafeMatcher { + private static class InputActionWithId extends TypeSafeMatcher { + private final String inputId; + + private InputActionWithId(String inputId) { + this.inputId = inputId; + } + @Override protected boolean matchesSafely(InputAction inputAction) { try { - return inputAction.isWaitingForInput(); - } catch (Exception e) { + return inputAction.getExecutions().stream().anyMatch(execution -> inputId.equals(execution.getId())); + } catch (InterruptedException | TimeoutException e) { return false; } } @Override public void describeTo(Description description) { - description.appendText("is waiting for input"); + description.appendText("has input with id ").appendValue(inputId); } } - private static void proceed(List actions, String inputMessage, String name) throws InterruptedException, TimeoutException, IOException { + private static void proceed(List actions, String inputId, String name) throws InterruptedException, TimeoutException, IOException { for (var action : actions) { if (action.getExecutions().isEmpty()) { continue; } var inputStepExecution = action.getExecutions().get(0); - if (inputMessage.equals(inputStepExecution.getDisplayName())) { + if (inputId.equals(inputStepExecution.getId())) { LOGGER.info(() -> "proceeding " + name); inputStepExecution.proceed(null); break; @@ -118,7 +124,7 @@ private static void proceed(List actions, String inputMessage, Stri private static void setupJobAndStart(JenkinsRule r) throws Exception { var p = r.createProject(WorkflowJob.class, "p"); - p.setDefinition(new CpsFlowDefinition("parallel 'Branch 1': { node('mib') { input 'Complete branch 1 ?' } }, 'Branch 2': { sleep 1; node('mib') { input 'Complete branch 2 ?' } }", true)); + p.setDefinition(new CpsFlowDefinition("parallel 'Branch 1': { node('mib') { input id: 'Branch1', message: 'Complete branch 1 ?' } }, 'Branch 2': { sleep 1; node('mib') { input id:'Branch2', message: 'Complete branch 2 ?' } }", true)); var b = p.scheduleBuild2(0).waitForStart(); r.waitForMessage("Complete branch 1 ?", b); assertTrue(b.isBuilding()); From 2e08fa8251fa661b9c4496621709af51003666d1 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Thu, 21 Nov 2024 12:24:11 +0100 Subject: [PATCH 03/16] Better assertion to avoid flakes --- .../support/steps/ExecutorStepDynamicContextRJRTest.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepDynamicContextRJRTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepDynamicContextRJRTest.java index db30371b..ebcfdc43 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepDynamicContextRJRTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepDynamicContextRJRTest.java @@ -43,6 +43,8 @@ import org.hamcrest.TypeSafeMatcher; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; import org.jenkinsci.plugins.workflow.flow.FlowExecutionList; +import org.jenkinsci.plugins.workflow.graphanalysis.DepthFirstScanner; +import org.jenkinsci.plugins.workflow.graphanalysis.NodeStepTypePredicate; import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.support.steps.input.InputAction; import org.junit.Rule; @@ -129,5 +131,8 @@ private static void setupJobAndStart(JenkinsRule r) throws Exception { r.waitForMessage("Complete branch 1 ?", b); assertTrue(b.isBuilding()); await().until(() -> r.jenkins.getQueue().getItems(), arrayWithSize(1)); + LOGGER.info("Node steps: " + new DepthFirstScanner().filteredNodes(b.getExecution(), new NodeStepTypePredicate("node"))); + // "Branch 1", "Branch 2" and the second node step ? Not fully clear + await().until(() -> new DepthFirstScanner().filteredNodes(b.getExecution(), new NodeStepTypePredicate("node")), hasSize(3)); } } From eef5c13ac8212a5736cd40c42c28050812265de4 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Thu, 21 Nov 2024 13:06:06 +0100 Subject: [PATCH 04/16] Reschedule queue item if we can't find the one referenced in the action --- .../support/steps/ExecutorStepExecution.java | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) 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 5787ab57..3d245f8a 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 @@ -215,8 +215,25 @@ public void stop(@NonNull Throwable cause) throws Exception { @Override public void onResume() { try { if (state == null) { - Run run = getContext().get(Run.class); - LOGGER.fine(() -> "No ExecutorStepDynamicContext found for node block in " + run + "; perhaps loading from a historical build record, hoping for the best"); + var flowNode = getContext().get(FlowNode.class); + LOGGER.fine(() -> "No ExecutorStepDynamicContext found for node block " + getContext() + "; will attempt to recover"); + if (flowNode == null) { + LOGGER.fine(() -> "No FlowNode found for node block " + getContext() + "; can't recover" ); + } else { + var action = flowNode.getAction(QueueItemActionImpl.class); + if (action == null) { + LOGGER.fine(() -> "No QueueItemAction found for node block " + getContext() + "; can't recover"); + } else { + LOGGER.fine(() -> "QueueItemAction with id=" + action.id + " found for node block " + getContext()); + var queueItem = Queue.getInstance().getItem(action.id); + if (queueItem == null) { + LOGGER.fine(() -> "Could not find queue item " + action.id + ", rescheduling it"); + start(); + } else { + LOGGER.fine(() -> "Found Queue.Item " + queueItem + " for node block " + getContext() + "; should be fine"); + } + } + } return; } state.resume(getContext()); From f6a8366c4234244d64ab59a32cc90a290ddff9d6 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Thu, 21 Nov 2024 13:54:29 +0100 Subject: [PATCH 05/16] New jenkins-test-harness needs new jenkins core --- pom.xml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pom.xml b/pom.xml index 394a8541..54fcc27b 100644 --- a/pom.xml +++ b/pom.xml @@ -68,7 +68,7 @@ 999999-SNAPSHOT 2357.vf2a_982b_b_910f - 2.440.1 + 2.479.1 true jenkinsci/${project.artifactId}-plugin 2.40 @@ -77,8 +77,8 @@ io.jenkins.tools.bom - bom-2.440.x - 2907.vcb_35d6f2f7de + bom-2.479.x + 3696.vb_b_4e2d1a_0542 import pom From bb43813a039bc210af3c1e8739db4102ce9d18e5 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Thu, 21 Nov 2024 14:11:21 +0100 Subject: [PATCH 06/16] Also need to bump parent pom --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 54fcc27b..09d4de9f 100644 --- a/pom.xml +++ b/pom.xml @@ -28,7 +28,7 @@ org.jenkins-ci.plugins plugin - 4.88 + 5.3 org.jenkins-ci.plugins.workflow From debd41fcec1011d029540ced4efa727eedf6b72f Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Thu, 21 Nov 2024 14:46:24 +0100 Subject: [PATCH 07/16] Use existing method --- .../plugins/workflow/support/steps/ExecutorStepExecution.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 3d245f8a..d75b8252 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 @@ -225,7 +225,7 @@ public void stop(@NonNull Throwable cause) throws Exception { LOGGER.fine(() -> "No QueueItemAction found for node block " + getContext() + "; can't recover"); } else { LOGGER.fine(() -> "QueueItemAction with id=" + action.id + " found for node block " + getContext()); - var queueItem = Queue.getInstance().getItem(action.id); + var queueItem = action.itemInQueue(); if (queueItem == null) { LOGGER.fine(() -> "Could not find queue item " + action.id + ", rescheduling it"); start(); From d60e9c0ca751cd12f7b06c37e300955d7ca6e7ea Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Thu, 21 Nov 2024 14:48:24 +0100 Subject: [PATCH 08/16] Downgrade bom --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 09d4de9f..cedd51f0 100644 --- a/pom.xml +++ b/pom.xml @@ -78,7 +78,7 @@ io.jenkins.tools.bom bom-2.479.x - 3696.vb_b_4e2d1a_0542 + 3613.v584fca_12cf5c import pom From 86ca8dfa99ce8c521f28dff864266eb8bd726e9e Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Thu, 21 Nov 2024 16:10:14 +0100 Subject: [PATCH 09/16] Rename test to be more consistent with the impacted class --- ...tRJRTest.java => ExecutorStepExecutionRJRTest.java} | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) rename src/test/java/org/jenkinsci/plugins/workflow/support/steps/{ExecutorStepDynamicContextRJRTest.java => ExecutorStepExecutionRJRTest.java} (94%) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepDynamicContextRJRTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecutionRJRTest.java similarity index 94% rename from src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepDynamicContextRJRTest.java rename to src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecutionRJRTest.java index ebcfdc43..47f59042 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepDynamicContextRJRTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecutionRJRTest.java @@ -55,8 +55,8 @@ import org.jvnet.hudson.test.RealJenkinsRule; import org.jvnet.hudson.test.TailLog; -public class ExecutorStepDynamicContextRJRTest { - private static final Logger LOGGER = Logger.getLogger(ExecutorStepDynamicContextRJRTest.class.getName()); +public class ExecutorStepExecutionRJRTest { + private static final Logger LOGGER = Logger.getLogger(ExecutorStepExecutionRJRTest.class.getName()); @Rule public RealJenkinsRule rjr = new RealJenkinsRule().withColor(PrefixedOutputStream.Color.GREEN).withPackageLogger(ExecutorStepExecution.class, Level.FINE).withPackageLogger(FlowExecutionList.class, Level.FINE); @@ -67,14 +67,14 @@ public class ExecutorStepDynamicContextRJRTest { rjr.startJenkins(); try (var tailLog = new TailLog(rjr, "p", 1)) { iar.createAgent(rjr, InboundAgentRule.Options.newBuilder().name("J").label("mib").color(PrefixedOutputStream.Color.YELLOW).webSocket().build()); - rjr.runRemotely(ExecutorStepDynamicContextRJRTest::setupJobAndStart); + rjr.runRemotely(ExecutorStepExecutionRJRTest::setupJobAndStart); rjr.stopJenkinsForcibly(); rjr.startJenkins(); - rjr.runRemotely(ExecutorStepDynamicContextRJRTest::assertQueueItems); + rjr.runRemotely(ExecutorStepExecutionRJRTest::resumeCompleteBranch1ThenBranch2); } } - private static void assertQueueItems(JenkinsRule r) throws Throwable { + private static void resumeCompleteBranch1ThenBranch2(JenkinsRule r) throws Throwable { var p = r.jenkins.getItemByFullName("p", WorkflowJob.class); var b = p.getBuildByNumber(1); await("Waiting for agent J to reconnect").atMost(Duration.ofSeconds(30)).until(() -> r.jenkins.getComputer("J").isOnline(), is(true)); From 992592fcf43a39cb302b04688dee179e67f32637 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Thu, 21 Nov 2024 16:45:39 +0100 Subject: [PATCH 10/16] Improve logging statement Co-authored-by: Jesse Glick --- .../plugins/workflow/support/steps/ExecutorStepExecution.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 d75b8252..68d05027 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 @@ -216,7 +216,7 @@ public void stop(@NonNull Throwable cause) throws Exception { try { if (state == null) { var flowNode = getContext().get(FlowNode.class); - LOGGER.fine(() -> "No ExecutorStepDynamicContext found for node block " + getContext() + "; will attempt to recover"); + LOGGER.fine(() -> "node block " + getContext() + " not yet scheduled, checking for an existing queue item"); if (flowNode == null) { LOGGER.fine(() -> "No FlowNode found for node block " + getContext() + "; can't recover" ); } else { From 1b6e958b9e323f703581c8f6d3b3935588c9f088 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Thu, 21 Nov 2024 16:46:30 +0100 Subject: [PATCH 11/16] Match jenkins version and bom --- pom.xml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index cedd51f0..a49f733d 100644 --- a/pom.xml +++ b/pom.xml @@ -68,7 +68,8 @@ 999999-SNAPSHOT 2357.vf2a_982b_b_910f - 2.479.1 + 2.479 + ${jenkins.baseline}.1 true jenkinsci/${project.artifactId}-plugin 2.40 @@ -77,7 +78,7 @@ io.jenkins.tools.bom - bom-2.479.x + bom-${jenkins.baseline}.x 3613.v584fca_12cf5c import pom From e2516da1dadce39036982a18e934f1266a0cc865 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Thu, 21 Nov 2024 16:53:18 +0100 Subject: [PATCH 12/16] Use older bom baseline --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index a49f733d..1eefe777 100644 --- a/pom.xml +++ b/pom.xml @@ -79,7 +79,7 @@ io.jenkins.tools.bom bom-${jenkins.baseline}.x - 3613.v584fca_12cf5c + 3482.vc10d4f6da_28a_ import pom From 711657d1bde3b1acca492f53367dbe5441ebb4e4 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Thu, 21 Nov 2024 17:23:13 +0100 Subject: [PATCH 13/16] Java 17 -> we can use multiline strings --- .../support/steps/ExecutorStepExecutionRJRTest.java | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecutionRJRTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecutionRJRTest.java index 47f59042..ea4f15cc 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecutionRJRTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecutionRJRTest.java @@ -126,7 +126,18 @@ private static void proceed(List actions, String inputId, String na private static void setupJobAndStart(JenkinsRule r) throws Exception { var p = r.createProject(WorkflowJob.class, "p"); - p.setDefinition(new CpsFlowDefinition("parallel 'Branch 1': { node('mib') { input id: 'Branch1', message: 'Complete branch 1 ?' } }, 'Branch 2': { sleep 1; node('mib') { input id:'Branch2', message: 'Complete branch 2 ?' } }", true)); + p.setDefinition(new CpsFlowDefinition(""" + parallel 'Branch 1': { + node('mib') { + input id: 'Branch1', message: 'Complete branch 1 ?' + } + }, 'Branch 2': { + sleep 1 + node('mib') { + input id:'Branch2', message: 'Complete branch 2 ?' + } + } + """, true)); var b = p.scheduleBuild2(0).waitForStart(); r.waitForMessage("Complete branch 1 ?", b); assertTrue(b.isBuilding()); From 1064b9f887a19bb54b5602fe42fa6c9378e02d0f Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Fri, 22 Nov 2024 17:13:46 +0100 Subject: [PATCH 14/16] Remove existing queue item action if it points to a missing queue item Co-authored-by: Jesse Glick --- .../plugins/workflow/support/steps/ExecutorStepExecution.java | 1 + 1 file changed, 1 insertion(+) 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 68d05027..ecc69055 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 @@ -228,6 +228,7 @@ public void stop(@NonNull Throwable cause) throws Exception { var queueItem = action.itemInQueue(); if (queueItem == null) { LOGGER.fine(() -> "Could not find queue item " + action.id + ", rescheduling it"); + flowNode.removeActions(QueueItemActionImpl.class); start(); } else { LOGGER.fine(() -> "Found Queue.Item " + queueItem + " for node block " + getContext() + "; should be fine"); From d2f465ffda116a8e02ea460d7ef54e2ca5f63034 Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Fri, 22 Nov 2024 17:14:09 +0100 Subject: [PATCH 15/16] Simplify assertion Co-authored-by: Jesse Glick --- .../workflow/support/steps/ExecutorStepExecutionRJRTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecutionRJRTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecutionRJRTest.java index ea4f15cc..9f9e5b59 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecutionRJRTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecutionRJRTest.java @@ -77,7 +77,7 @@ public class ExecutorStepExecutionRJRTest { private static void resumeCompleteBranch1ThenBranch2(JenkinsRule r) throws Throwable { var p = r.jenkins.getItemByFullName("p", WorkflowJob.class); var b = p.getBuildByNumber(1); - await("Waiting for agent J to reconnect").atMost(Duration.ofSeconds(30)).until(() -> r.jenkins.getComputer("J").isOnline(), is(true)); + await("Waiting for agent J to reconnect").atMost(Duration.ofSeconds(30)).until(() -> r.jenkins.getComputer("J").isOnline()); var actions = await().until(() -> b.getActions(InputAction.class), allOf(iterableWithSize(1), hasItem(new InputActionWithId("Branch1")))); proceed(actions, "Branch1", p.getName() + "#" + b.number); // This is quicker than waitForMessage that can wait for up to 10 minutes From 680ce202a36afeb2889fe0c347ff0385d39590ef Mon Sep 17 00:00:00 2001 From: Vincent Latombe Date: Fri, 22 Nov 2024 17:15:00 +0100 Subject: [PATCH 16/16] Clarify comment Co-authored-by: Jesse Glick --- .../workflow/support/steps/ExecutorStepExecutionRJRTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecutionRJRTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecutionRJRTest.java index 9f9e5b59..74a79ebe 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecutionRJRTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/ExecutorStepExecutionRJRTest.java @@ -143,7 +143,7 @@ private static void setupJobAndStart(JenkinsRule r) throws Exception { assertTrue(b.isBuilding()); await().until(() -> r.jenkins.getQueue().getItems(), arrayWithSize(1)); LOGGER.info("Node steps: " + new DepthFirstScanner().filteredNodes(b.getExecution(), new NodeStepTypePredicate("node"))); - // "Branch 1", "Branch 2" and the second node step ? Not fully clear + // "Branch 1" step start + "Branch 1" body start + "Branch 2" step start await().until(() -> new DepthFirstScanner().filteredNodes(b.getExecution(), new NodeStepTypePredicate("node")), hasSize(3)); } }