Skip to content
This repository has been archived by the owner on Dec 15, 2021. It is now read-only.

Commit

Permalink
Browse files Browse the repository at this point in the history
[FIXED JENKINS-28063] BuildTriggerAction must implement FoldableActio…
…n and multiple instances checked.

Otherwise any redundant upstream builds never receive notification of the downstream build’s completion, and hang.
  • Loading branch information
jglick committed Apr 30, 2015
1 parent d0065d2 commit 90f9e44
Show file tree
Hide file tree
Showing 6 changed files with 48 additions and 20 deletions.
3 changes: 2 additions & 1 deletion CHANGES.md
Expand Up @@ -4,7 +4,8 @@ Only noting significant user-visible or major API changes, not internal code cle

## 1.6 (upcoming)

* [JENKINS-27571](https://issues.jenkins-ci.org/browse/JENKINS-27571) Fixed link in build sidepanel.
* [JENKINS-28063](https://issues.jenkins-ci.org/browse/JENKINS-28063): `build` step did not properly handle the case that two upstream builds could trigger the same downstream build.
* [JENKINS-27571](https://issues.jenkins-ci.org/browse/JENKINS-27571): Fixed link in build sidepanel.
* API addition: `LauncherDecorator` can now be used in block-scoped steps, and there is more flexibility in handling exits from durable tasks.

## 1.5 (Apr 01 2015)
Expand Down
Expand Up @@ -240,4 +240,25 @@ public void cancelBuildQueue() throws Exception {
j.assertLogContains("build var: override", j.assertBuildStatusSuccess(us.scheduleBuild2(0)));
}

@Issue("JENKINS-28063")
@Test public void coalescedQueue() throws Exception {
FreeStyleProject ds = j.createFreeStyleProject("ds");
ds.setConcurrentBuild(true);
ds.getBuildersList().add(new SleepBuilder(3000));
WorkflowJob us = j.jenkins.createProject(WorkflowJob.class, "us");
us.setDefinition(new CpsFlowDefinition("echo \"triggered #${build('ds').number}\"", true));
QueueTaskFuture<WorkflowRun> us1F = us.scheduleBuild2(0);
us1F.waitForStart(); // make sure we do not coalesce the us `Queue.Item`s
QueueTaskFuture<WorkflowRun> us2F = us.scheduleBuild2(0);
WorkflowRun us1 = us1F.get();
assertEquals(1, us1.getNumber());
j.assertLogContains("triggered #1", us1);
WorkflowRun us2 = us2F.get();
assertEquals(2, us2.getNumber());
j.assertLogContains("triggered #1", us2);
FreeStyleBuild ds1 = ds.getLastBuild();
assertEquals(1, ds1.getNumber());
assertEquals(2, ds1.getCauses().size()); // 2× UpstreamCause
}

}
Expand Up @@ -12,8 +12,7 @@ public class BuildQueueListener extends QueueListener {
@Override
public void onLeft(Queue.LeftItem li) {
if(li.isCancelled()){
BuildTriggerAction action = li.getAction(BuildTriggerAction.class);
if(action != null) {
for (BuildTriggerAction action : li.getActions(BuildTriggerAction.class)) {
action.getStepContext().onFailure(new Exception(String.format("Build %s was cancelled.", li.task.getName())));
}
}
Expand Down
@@ -1,9 +1,13 @@
package org.jenkinsci.plugins.workflow.support.steps.build;

import hudson.model.Action;
import hudson.model.InvisibleAction;
import hudson.model.Queue;
import hudson.model.queue.FoldableAction;
import java.util.List;
import org.jenkinsci.plugins.workflow.steps.StepContext;

public class BuildTriggerAction extends InvisibleAction {
public class BuildTriggerAction extends InvisibleAction implements FoldableAction {

private final StepContext context;
private final Boolean propagate;
Expand All @@ -21,4 +25,8 @@ public boolean isPropagate() {
return propagate != null ? propagate : /* old serialized record */ true;
}

@Override public void foldIntoExisting(Queue.Item item, Queue.Task owner, List<Action> otherActions) {
item.addAction(this); // there may be >1 upstream builds (or other unrelated causes) for a single downstream build
}

}
Expand Up @@ -8,16 +8,12 @@

import javax.annotation.Nonnull;

/**
* @author Vivek Pandey
*/
@Extension
public class BuildTriggerListener extends RunListener<Run<?,?>>{

@Override
public void onCompleted(Run run, @Nonnull TaskListener listener) {
BuildTriggerAction action = run.getAction(BuildTriggerAction.class);
if (action != null) {
public void onCompleted(Run<?,?> run, @Nonnull TaskListener listener) {
for (BuildTriggerAction action : run.getActions(BuildTriggerAction.class)) {
if (!action.isPropagate() || run.getResult() == Result.SUCCESS) {
action.getStepContext().onSuccess(new RunWrapper(run, false));
} else {
Expand All @@ -27,9 +23,8 @@ public void onCompleted(Run run, @Nonnull TaskListener listener) {
}

@Override
public void onDeleted(Run run) {
BuildTriggerAction action = run.getAction(BuildTriggerAction.class);
if(action != null) {
public void onDeleted(Run<?,?> run) {
for (BuildTriggerAction action : run.getActions(BuildTriggerAction.class)) {
action.getStepContext().onFailure(new Exception(run.getBuildStatusSummary().message));
}
}
Expand Down
Expand Up @@ -86,9 +86,13 @@ public void stop(Throwable cause) {
// if the build is still in the queue, abort it.
// BuildTriggerListener will report the failure, so this method shouldn't call getContext().onFailure()
for (Queue.Item i : q.getItems()) {
BuildTriggerAction bta = i.getAction(BuildTriggerAction.class);
if (bta!=null && bta.getStepContext().equals(getContext())) {
q.cancel(i);
for (BuildTriggerAction bta : i.getActions(BuildTriggerAction.class)) {
if (bta.getStepContext().equals(getContext())) {
// Note that it is a little questionable to cancel the queue item in case it has other causes,
// but in the common case that this is the only cause, it is most intuitive to do so.
// The same applies to aborting the actual build once started.
q.cancel(i);
}
}
}

Expand All @@ -100,10 +104,10 @@ public void stop(Throwable cause) {
Queue.Executable exec = e.getCurrentExecutable();
if (exec instanceof Run) {
Run<?,?> b = (Run) exec;

BuildTriggerAction bta = b.getAction(BuildTriggerAction.class);
if (bta!=null && bta.getStepContext().equals(getContext())) {
e.interrupt(Result.ABORTED, new BuildTriggerCancelledCause(cause));
for (BuildTriggerAction bta : b.getActions(BuildTriggerAction.class)) {
if (bta.getStepContext().equals(getContext())) {
e.interrupt(Result.ABORTED, new BuildTriggerCancelledCause(cause));
}
}
}
}
Expand Down

0 comments on commit 90f9e44

Please sign in to comment.