From f6afe1a1ea06dea05aa338068f01e4c151f80bf9 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 21 Jul 2022 16:35:55 -0400 Subject: [PATCH 1/6] Demonstrate in test that `retry` without conditions does not currently work --- .../steps/AgentErrorConditionTest.java | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/AgentErrorConditionTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/AgentErrorConditionTest.java index cf07cf4e..af864090 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/AgentErrorConditionTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/AgentErrorConditionTest.java @@ -206,6 +206,32 @@ public static final class HangStep extends Step { }); } + @Ignore("TODO RemovedNodeListener implicitly passes actualInterruption=true") + @Test public void retryUnconditionally() throws Throwable { + sessions.then(r -> { + Slave s = inboundAgents.createAgent(r, "dumbo1"); + s.setLabelString("dumb"); + r.jenkins.updateNode(s); + WorkflowJob p = r.createProject(WorkflowJob.class, "p"); + p.setDefinition(new CpsFlowDefinition( + "retry(2) {\n" + + " node('dumb') {\n" + + " if (isUnix()) {sh 'sleep 10'} else {bat 'echo + sleep && ping -n 10 localhost'}\n" + + " }\n" + + "}", true)); + WorkflowRun b = p.scheduleBuild2(0).waitForStart(); + r.waitForMessage("+ sleep", b); + inboundAgents.stop("dumbo1"); + r.jenkins.removeNode(s); + r.waitForMessage(RetryThis.MESSAGE, b); + s = inboundAgents.createAgent(r, "dumbo2"); + s.setLabelString("dumb"); + r.jenkins.updateNode(s); + r.waitForMessage("Running on dumbo2 in ", b); + r.assertBuildStatusSuccess(r.waitForCompletion(b)); + }); + } + @Test public void overallBuildCancelIgnored() throws Throwable { sessions.then(r -> { r.createSlave(Label.get("remote")); From 0d126776f0741524907c023451f653ed4b68b051 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 21 Jul 2022 17:42:49 -0400 Subject: [PATCH 2/6] Fixing test --- .../plugins/workflow/support/steps/AgentErrorConditionTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/AgentErrorConditionTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/AgentErrorConditionTest.java index af864090..4c37bd4b 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/AgentErrorConditionTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/AgentErrorConditionTest.java @@ -223,7 +223,7 @@ public static final class HangStep extends Step { r.waitForMessage("+ sleep", b); inboundAgents.stop("dumbo1"); r.jenkins.removeNode(s); - r.waitForMessage(RetryThis.MESSAGE, b); + r.waitForMessage("Retrying", b); s = inboundAgents.createAgent(r, "dumbo2"); s.setLabelString("dumb"); r.jenkins.updateNode(s); From 9bdf9addc06202be13eb7aba5449733c5eea1187 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 21 Jul 2022 17:48:04 -0400 Subject: [PATCH 3/6] Using `BodyExecution.cancel(Throwable)` to pass `actualInterruption=false` --- pom.xml | 2 ++ .../plugins/workflow/support/steps/ExecutorStepExecution.java | 2 +- .../plugins/workflow/support/steps/AgentErrorConditionTest.java | 1 - 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 6a2b3085..d159ed47 100644 --- a/pom.xml +++ b/pom.xml @@ -86,6 +86,7 @@ org.jenkins-ci.plugins.workflow workflow-step-api + 999999-SNAPSHOT org.jenkins-ci.plugins @@ -102,6 +103,7 @@ org.jenkins-ci.plugins.workflow workflow-cps + 999999-SNAPSHOT test 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 205887f6..13a8955f 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 @@ -287,7 +287,7 @@ public static final class QueueTaskCancelled extends CauseOfInterruption { continue; } listener.getLogger().println("Agent " + node.getNodeName() + " was deleted; cancelling node body"); - body.cancel(new RemovedNodeCause()); + body.cancel(new FlowInterruptedException(Result.ABORTED, false, new RemovedNodeCause())); } } }, ExecutorPickle.TIMEOUT_WAITING_FOR_NODE_MILLIS, TimeUnit.MILLISECONDS); diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/AgentErrorConditionTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/AgentErrorConditionTest.java index 4c37bd4b..00374984 100644 --- a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/AgentErrorConditionTest.java +++ b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/AgentErrorConditionTest.java @@ -206,7 +206,6 @@ public static final class HangStep extends Step { }); } - @Ignore("TODO RemovedNodeListener implicitly passes actualInterruption=true") @Test public void retryUnconditionally() throws Throwable { sessions.then(r -> { Slave s = inboundAgents.createAgent(r, "dumbo1"); From 3146e974e11d58503d49e21368becddb9a3967ff Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 21 Jul 2022 17:54:04 -0400 Subject: [PATCH 4/6] More compatible variant --- .../workflow/support/steps/ExecutorStepExecution.java | 6 +++++- 1 file changed, 5 insertions(+), 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 13a8955f..35524bf9 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 @@ -287,7 +287,11 @@ public static final class QueueTaskCancelled extends CauseOfInterruption { continue; } listener.getLogger().println("Agent " + node.getNodeName() + " was deleted; cancelling node body"); - body.cancel(new FlowInterruptedException(Result.ABORTED, false, new RemovedNodeCause())); + if (Util.isOverridden(BodyExecution.class, body.getClass(), "cancel", Throwable.class)) { + body.cancel(new FlowInterruptedException(Result.ABORTED, false, new RemovedNodeCause())); + } else { // TODO remove once https://github.com/jenkinsci/workflow-cps-plugin/pull/570 is widely deployed + body.cancel(new RemovedNodeCause()); + } } } }, ExecutorPickle.TIMEOUT_WAITING_FOR_NODE_MILLIS, TimeUnit.MILLISECONDS); From c041efa346b214d7ed8e08899eaa29f1f55e13e4 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 22 Jul 2022 12:25:52 -0400 Subject: [PATCH 5/6] Incremental deployments; switched to 2.346.x due to https://github.com/jenkinsci/workflow-cps-plugin/pull/559 --- pom.xml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pom.xml b/pom.xml index d159ed47..5cf7bc0c 100644 --- a/pom.xml +++ b/pom.xml @@ -66,7 +66,7 @@ 999999-SNAPSHOT - 2.332.1 + 2.346.1 true jenkinsci/${project.artifactId}-plugin 2.40 @@ -75,7 +75,7 @@ io.jenkins.tools.bom - bom-2.332.x + bom-2.346.x 1478.v81d3dc4f9a_43 import pom @@ -86,7 +86,7 @@ org.jenkins-ci.plugins.workflow workflow-step-api - 999999-SNAPSHOT + 630.vdd28011769da_ org.jenkins-ci.plugins @@ -103,7 +103,7 @@ org.jenkins-ci.plugins.workflow workflow-cps - 999999-SNAPSHOT + 2756.v639b_85967348 test From 6ce423753f47e324d65f2cd3e8db4e53efc39452 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Fri, 22 Jul 2022 15:06:33 -0400 Subject: [PATCH 6/6] https://github.com/jenkinsci/workflow-cps-plugin/pull/570 released --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 5cf7bc0c..28818ded 100644 --- a/pom.xml +++ b/pom.xml @@ -86,7 +86,7 @@ org.jenkins-ci.plugins.workflow workflow-step-api - 630.vdd28011769da_ + 639.v6eca_cd8c04a_a_ org.jenkins-ci.plugins @@ -103,7 +103,7 @@ org.jenkins-ci.plugins.workflow workflow-cps - 2756.v639b_85967348 + 2759.v87459c4eea_ca_ test