Merge pull request #124 from stuartrowe/JENKINS-71961

jglick · web-flow · commit 9ca388a9087f · 2023-12-08T16:34:02.000-05:00
[JENKINS-71961] Fix waitForBuild Step does not abort downstream build when upstream job aborted
diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/WaitForBuildListener.java b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/WaitForBuildListener.java
@@ -1,12 +1,14 @@
 package org.jenkinsci.plugins.workflow.support.steps.build;
 
-import edu.umd.cs.findbugs.annotations.NonNull;
+import hudson.AbortException;
 import hudson.Extension;
 import hudson.console.ModelHyperlinkNote;
 import hudson.model.Result;
 import hudson.model.Run;
 import hudson.model.TaskListener;
 import hudson.model.listeners.RunListener;
+import jenkins.util.Timer;
+
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import org.jenkinsci.plugins.workflow.actions.WarningAction;
@@ -20,7 +22,7 @@ public class WaitForBuildListener extends RunListener<Run<?,?>> {
     private static final Logger LOGGER = Logger.getLogger(WaitForBuildListener.class.getName());
 
     @Override
-    public void onCompleted(Run<?,?> run, @NonNull TaskListener listener) {
+    public void onFinalized(Run<?,?> run) {
         for (WaitForBuildAction action : run.getActions(WaitForBuildAction.class)) {
             StepContext context = action.context;
             LOGGER.log(Level.FINE, "completing {0} for {1}", new Object[] {run, context});
@@ -46,4 +48,11 @@ public void onCompleted(Run<?,?> run, @NonNull TaskListener listener) {
         }
         run.removeActions(WaitForBuildAction.class);
     }
+
+    @Override
+    public void onDeleted(final Run<?,?> run) {
+        for (WaitForBuildAction action : run.getActions(WaitForBuildAction.class)) {
+            Timer.get().submit(() -> action.context.onFailure(new AbortException(run.getFullDisplayName() + " was deleted")));
+        }
+    }
 }
diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/WaitForBuildStep.java b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/WaitForBuildStep.java
@@ -24,6 +24,7 @@ public class WaitForBuildStep extends Step {
 
     private final String runId;
     private boolean propagate = false;
+    private boolean propagateAbort = false;
 
     @DataBoundConstructor
     public WaitForBuildStep(String runId) {
@@ -42,6 +43,14 @@ public boolean isPropagate() {
         this.propagate = propagate;
     }
 
+    public boolean isPropagateAbort() {
+        return propagateAbort;
+    }
+
+    @DataBoundSetter public void setPropagateAbort(boolean propagateAbort) {
+        this.propagateAbort = propagateAbort;
+    }
+
     @Override
     public StepExecution start(StepContext context) throws Exception {
         return new WaitForBuildStepExecution(this, context);
diff --git a/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/WaitForBuildStepExecution.java b/src/main/java/org/jenkinsci/plugins/workflow/support/steps/build/WaitForBuildStepExecution.java
@@ -3,17 +3,27 @@
 import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.AbortException;
 import hudson.console.ModelHyperlinkNote;
+import hudson.model.Computer;
+import hudson.model.Executor;
+import hudson.model.Queue;
 import hudson.model.Result;
 import hudson.model.Run;
 import hudson.model.TaskListener;
+import jenkins.model.Jenkins;
 import org.jenkinsci.plugins.workflow.steps.AbstractStepExecutionImpl;
 import org.jenkinsci.plugins.workflow.steps.FlowInterruptedException;
 import org.jenkinsci.plugins.workflow.steps.StepContext;
 
+import java.io.IOException;
+import java.util.logging.Level;
+import java.util.logging.Logger;
+
 public class WaitForBuildStepExecution extends AbstractStepExecutionImpl {
 
     private static final long serialVersionUID = 1L;
 
+    private static final Logger LOGGER = Logger.getLogger(WaitForBuildStepExecution.class.getName());
+
     private final transient WaitForBuildStep step;
 
     public WaitForBuildStepExecution(WaitForBuildStep step, @NonNull StepContext context) {
@@ -55,4 +65,51 @@ public boolean start() throws Exception {
         }
     }
 
+    @Override
+    public void stop(@NonNull Throwable cause) throws Exception {
+        StepContext context = getContext();
+        Jenkins jenkins = Jenkins.getInstanceOrNull();
+        if (jenkins == null) {
+            context.onFailure(cause);
+            return;
+        }
+
+        boolean interrupted = false;
+
+        if (step.isPropagateAbort()) {
+            // if there's any in-progress build already, abort that.
+            // when the build is actually aborted, WaitForBuildListener will take notice and report the failure,
+            // so this method shouldn't call getContext().onFailure()
+            for (Computer c : jenkins.getComputers()) {
+                for (Executor e : c.getAllExecutors()) {
+                interrupted |= maybeInterrupt(e, cause, context);
+                }
+            }
+        }
+
+        if(!interrupted) {
+            super.stop(cause);
+        }
+    }
+
+    private static boolean maybeInterrupt(Executor e, Throwable cause, StepContext context) throws IOException, InterruptedException {
+        boolean interrupted = false;
+        Queue.Executable exec = e.getCurrentExecutable();
+        if (exec instanceof Run) {
+            Run<?, ?> downstream = (Run<?, ?>) exec;
+            for(WaitForBuildAction waitForBuildAction : downstream.getActions(WaitForBuildAction.class)) {
+                if (waitForBuildAction.context.equals(context)) {
+                    e.interrupt(Result.ABORTED, new BuildTriggerCancelledCause(cause));
+                    try {
+                        downstream.save();
+                    } catch (IOException x) {
+                        LOGGER.log(Level.WARNING, "failed to save interrupt cause on " + exec, x);
+                    }
+                    interrupted = true;
+                }
+            }
+        }
+        return interrupted;
+    }
+
 }
diff --git a/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/WaitForBuildStepTest.java b/src/test/java/org/jenkinsci/plugins/workflow/support/steps/build/WaitForBuildStepTest.java
@@ -3,6 +3,7 @@
 import hudson.model.FreeStyleProject;
 import hudson.model.Result;
 import hudson.model.Run;
+
 import org.jenkinsci.plugins.workflow.actions.WarningAction;
 import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition;
 import org.jenkinsci.plugins.workflow.cps.nodes.StepAtomNode;
@@ -11,6 +12,7 @@
 import org.jenkinsci.plugins.workflow.graph.FlowNode;
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
+import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
@@ -31,25 +33,60 @@ public class WaitForBuildStepTest {
 
     @Test public void waitForBuild() throws Exception {
         Result dsResult = Result.FAILURE;
-        createWaitingDownStreamJob(dsResult);
+        WorkflowJob ds = createWaitingDownStreamJob("wait", dsResult);
         WorkflowJob us = j.jenkins.createProject(WorkflowJob.class, "us");
         us.setDefinition(new CpsFlowDefinition(
             "def ds = build job: 'ds', waitForStart: true\n" +
+            "semaphore 'scheduled'\n" + 
             "def dsRunId = \"${ds.getFullProjectName()}#${ds.getNumber()}\"\n" +
             "def completeDs = waitForBuild runId: dsRunId\n" +
             "echo \"'ds' completed with status ${completeDs.getResult()}\"", true));
-        j.assertLogContains("'ds' completed with status " + dsResult.toString(), j.buildAndAssertSuccess(us));
+
+        // schedule upstream
+        WorkflowRun usRun = us.scheduleBuild2(0).waitForStart();
+
+        // wait for ds to be scheduled
+        SemaphoreStep.waitForStart("scheduled/1", usRun);
+        SemaphoreStep.success("scheduled/1", true);
+
+        // signal the downstream run to complete after it has been waited on
+        WorkflowRun dsRun = ds.getBuildByNumber(1);
+        SemaphoreStep.waitForStart("wait/1", dsRun);
+        waitForWaitForBuildAction(dsRun);
+        SemaphoreStep.success("wait/1", true);
+
+        // assert upstream build status
+        WorkflowRun completedUsRun = j.waitForCompletion(usRun);
+        j.assertBuildStatusSuccess(completedUsRun);
+        j.assertLogContains("'ds' completed with status " + dsResult.toString(), completedUsRun);
     }
 
     @Test public void waitForBuildPropagte() throws Exception {
         Result dsResult = Result.FAILURE;
-        createWaitingDownStreamJob(dsResult);
+        WorkflowJob ds = createWaitingDownStreamJob("wait", dsResult);
         WorkflowJob us = j.jenkins.createProject(WorkflowJob.class, "us");
         us.setDefinition(new CpsFlowDefinition(
             "def ds = build job: 'ds', waitForStart: true\n" +
+            "semaphore 'scheduled'\n" + 
             "def dsRunId = \"${ds.getFullProjectName()}#${ds.getNumber()}\"\n" +
             "waitForBuild runId: dsRunId, propagate: true", true));
-        j.assertLogContains("completed with status " + dsResult.toString(), j.buildAndAssertStatus(dsResult, us));
+
+        // schedule upstream
+        WorkflowRun usRun = us.scheduleBuild2(0).waitForStart();
+        
+        // wait for ds to be scheduled
+        SemaphoreStep.waitForStart("scheduled/1", usRun);
+        SemaphoreStep.success("scheduled/1", true);
+
+        // signal the downstream run to complete after it has been waited on
+        WorkflowRun dsRun = ds.getBuildByNumber(1);
+        waitForWaitForBuildAction(dsRun);
+        SemaphoreStep.success("wait/1", true);
+
+        // assert upstream build status
+        WorkflowRun completedUsRun = j.waitForCompletion(usRun);
+        j.assertBuildStatus(dsResult, completedUsRun);
+        j.assertLogContains("completed with status " + dsResult.toString(), completedUsRun);
     }
 
     @SuppressWarnings("rawtypes")
@@ -82,24 +119,127 @@ public class WaitForBuildStepTest {
     @Issue("JENKINS-70983")
     @Test public void waitForUnstableBuildWithWarningAction() throws Exception {
         Result dsResult = Result.UNSTABLE;
-        createWaitingDownStreamJob(dsResult);
+        WorkflowJob ds = createWaitingDownStreamJob("wait", dsResult);
         WorkflowJob us = j.jenkins.createProject(WorkflowJob.class, "us");
         us.setDefinition(new CpsFlowDefinition(
             "def ds = build job: 'ds', waitForStart: true\n" +
+            "semaphore 'scheduled'\n" +
             "def dsRunId = \"${ds.getFullProjectName()}#${ds.getNumber()}\"\n" +
             "try {\n" +
             "    waitForBuild runId: dsRunId, propagate: true\n" +
             "} finally {\n" +
             "    echo \"'ds' completed with status ${ds.getResult()}\"\n" +
             "}", true));
-        j.assertLogContains("'ds' completed with status " + dsResult.toString(), j.buildAndAssertStatus(dsResult, us));
-        WorkflowRun lastUpstreamRun = us.getLastBuild();
-        FlowNode buildTriggerNode = findFirstNodeWithDescriptor(lastUpstreamRun.getExecution(), WaitForBuildStep.DescriptorImpl.class);
+
+        // schedule upstream
+        WorkflowRun usRun = us.scheduleBuild2(0).waitForStart();
+        
+        // wait for ds to be scheduled
+        SemaphoreStep.waitForStart("scheduled/1", usRun);
+        SemaphoreStep.success("scheduled/1", true);
+
+        // signal the downstream run to complete after it has been waited on
+        WorkflowRun dsRun = ds.getBuildByNumber(1);
+        waitForWaitForBuildAction(dsRun);
+        SemaphoreStep.success("wait/1", true);
+
+        // assert upstream build status
+        WorkflowRun completedUsRun = j.waitForCompletion(usRun);
+        j.assertBuildStatus(dsResult, completedUsRun);
+        j.assertLogContains("'ds' completed with status " + dsResult.toString(), completedUsRun);
+
+        FlowNode buildTriggerNode = findFirstNodeWithDescriptor(completedUsRun.getExecution(), WaitForBuildStep.DescriptorImpl.class);
         WarningAction action = buildTriggerNode.getAction(WarningAction.class);
         assertNotNull(action);
         assertEquals(action.getResult(), Result.UNSTABLE);
     }
 
+    @Issue("JENKINS-71961")
+    @Test public void abortBuild() throws Exception {
+        WorkflowJob ds = createWaitingDownStreamJob("wait", Result.SUCCESS);
+        WorkflowJob us = j.jenkins.createProject(WorkflowJob.class, "us");
+        us.setDefinition(new CpsFlowDefinition(
+            "def ds = build job: 'ds', waitForStart: true\n" +
+            "semaphore 'scheduled'\n" + 
+            "def dsRunId = \"${ds.getFullProjectName()}#${ds.getNumber()}\"\n" +
+            "def completeDs = waitForBuild runId: dsRunId, propagate: true\n" +
+            "echo \"'ds' completed with status ${completeDs.getResult()}\"", true));
+
+        // schedule upstream
+        WorkflowRun usRun = us.scheduleBuild2(0).waitForStart();
+        
+        // wait for ds to be scheduled
+        SemaphoreStep.waitForStart("scheduled/1", usRun);
+        SemaphoreStep.success("scheduled/1", true);
+
+        WorkflowRun dsRun = ds.getBuildByNumber(1);
+        waitForWaitForBuildAction(dsRun);
+
+        // Abort the downstream build
+        dsRun.getExecutor().interrupt();
+
+        j.assertBuildStatus(Result.ABORTED, j.waitForCompletion(dsRun));
+        j.assertBuildStatus(Result.ABORTED, j.waitForCompletion(usRun));
+    }
+
+    @Issue("JENKINS-71961")
+    @Test public void interruptFlowPropagateAbort() throws Exception {
+        WorkflowJob ds = createWaitingDownStreamJob("wait", Result.SUCCESS);
+        WorkflowJob us = j.jenkins.createProject(WorkflowJob.class, "us");
+        us.setDefinition(new CpsFlowDefinition(
+            "def ds = build job: 'ds', waitForStart: true\n" +
+            "semaphore 'scheduled'\n" +
+            "def dsRunId = \"${ds.getFullProjectName()}#${ds.getNumber()}\"\n" +
+            "def completeDs = waitForBuild runId: dsRunId, propagate: true, propagateAbort: true\n" +
+            "echo \"'ds' completed with status ${completeDs.getResult()}\"", true));
+
+        // schedule upstream
+        WorkflowRun usRun = us.scheduleBuild2(0).waitForStart();
+        
+        // wait for ds to be scheduled
+        SemaphoreStep.waitForStart("scheduled/1", usRun);
+        SemaphoreStep.success("scheduled/1", true);
+
+        WorkflowRun dsRun = ds.getBuildByNumber(1);
+        waitForWaitForBuildAction(dsRun);
+
+        // Abort the upstream build
+        usRun.doStop();
+
+        j.assertBuildStatus(Result.ABORTED, j.waitForCompletion(dsRun));
+        j.assertBuildStatus(Result.ABORTED, j.waitForCompletion(usRun));
+    }
+
+    @Issue("JENKINS-71961")
+    @Test public void interruptFlowNoPropagateAbort() throws Exception {
+        WorkflowJob ds = createWaitingDownStreamJob("wait", Result.SUCCESS);
+        WorkflowJob us = j.jenkins.createProject(WorkflowJob.class, "us");
+        us.setDefinition(new CpsFlowDefinition(
+            "def ds = build job: 'ds', waitForStart: true\n" +
+            "semaphore 'scheduled'\n" +
+            "def dsRunId = \"${ds.getFullProjectName()}#${ds.getNumber()}\"\n" +
+            "def completeDs = waitForBuild runId: dsRunId, propagate: true, propagateAbort: false\n" +
+            "echo \"'ds' completed with status ${completeDs.getResult()}\"", true));
+
+        // schedule upstream
+        WorkflowRun usRun = us.scheduleBuild2(0).waitForStart();
+        
+        // wait for ds to be scheduled
+        SemaphoreStep.waitForStart("scheduled/1", usRun);
+        SemaphoreStep.success("scheduled/1", true);
+
+        WorkflowRun dsRun = ds.getBuildByNumber(1);
+        waitForWaitForBuildAction(dsRun);
+
+        // Abort the upstream build
+        usRun.doStop();
+        j.assertBuildStatus(Result.ABORTED, j.waitForCompletion(usRun));
+
+        // Allow the downstream to complete
+        SemaphoreStep.success("wait/1", true);
+        j.assertBuildStatus(Result.SUCCESS, j.waitForCompletion(dsRun));
+    }
+
     private static FlowNode findFirstNodeWithDescriptor(FlowExecution execution, Class<WaitForBuildStep.DescriptorImpl> cls) {
         for (FlowNode node : new FlowGraphWalker(execution)) {
             if (node instanceof StepAtomNode) {
@@ -112,22 +252,23 @@ private static FlowNode findFirstNodeWithDescriptor(FlowExecution execution, Cla
         return null;
     }
 
-    private WorkflowJob createWaitingDownStreamJob(Result result) throws Exception {
+    private WorkflowJob createWaitingDownStreamJob(String semaphoreName, Result result) throws Exception {
         WorkflowJob ds = j.jenkins.createProject(WorkflowJob.class, "ds");
         ds.setDefinition(new CpsFlowDefinition(
-            "import org.jenkinsci.plugins.workflow.support.steps.build.WaitForBuildAction\n" +
-            "@NonCPS\n" + 
-            "boolean hasWaitForBuildAction() {\n" +
-            "    return currentBuild.getRawBuild().getAction(WaitForBuildAction.class) != null\n" +
-            "}\n" +
-            "while(!hasWaitForBuildAction()) {\n" +
-            "    sleep(time: 100, unit: 'MILLISECONDS')\n" +
-            "}\n" +
+            "semaphore('" + semaphoreName + "')\n" +
             "catchError(buildResult: '" + result.toString() + "') {\n" +
             "    error('')\n" +
             "}", false));
         return ds;
-        
+    }
+
+    private void waitForWaitForBuildAction(WorkflowRun r) throws Exception {
+        while(true) {
+            if (r.getAction(WaitForBuildAction.class) != null) {
+                break;
+            }
+            Thread.sleep(10);
+        }
     }
 
 }
