Skip to content

Commit

Permalink
Merge pull request #4 from svanoort/fix-parallel-JENKINS-38464
Browse files Browse the repository at this point in the history 
Fix use of milestone step outside parallel [JENKINS-38464]
  • Loading branch information
@svanoort
svanoort committed Oct 5, 2016
2 parents 6295778 + b7492d5 commit 1c90cd1
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 37 deletions.
6 changes: 3 additions & 3 deletions pom.xml
View file
Expand Up @@ -30,13 +30,13 @@
<repositories>
<repository>
<id>repo.jenkins-ci.org</id>
<url>http://repo.jenkins-ci.org/public/</url>
<url>https://repo.jenkins-ci.org/public/</url>
</repository>
</repositories>
<pluginRepositories>
<pluginRepository>
<id>repo.jenkins-ci.org</id>
<url>http://repo.jenkins-ci.org/public/</url>
<url>https://repo.jenkins-ci.org/public/</url>
</pluginRepository>
</pluginRepositories>
<properties>
Expand All @@ -51,7 +51,7 @@
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
<artifactId>workflow-api</artifactId>
<version>1.15</version>
<version>2.4</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins.workflow</groupId>
Expand Down
48 changes: 14 additions & 34 deletions src/main/java/org/jenkinsci/plugins/pipeline/milestone/MilestoneStepExecution.java
View file
Expand Up @@ -26,6 +26,7 @@
import static java.util.logging.Level.WARNING;

import java.io.IOException;
import java.util.Collections;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
Expand All @@ -37,13 +38,16 @@

import javax.annotation.CheckForNull;

import com.google.common.base.Predicate;
import org.jenkinsci.plugins.workflow.actions.ThreadNameAction;
import org.jenkinsci.plugins.workflow.flow.FlowExecutionOwner;
import org.jenkinsci.plugins.workflow.graph.BlockEndNode;
import org.jenkinsci.plugins.workflow.graph.BlockStartNode;
import org.jenkinsci.plugins.workflow.graph.FlowGraphWalker;
import org.jenkinsci.plugins.workflow.graph.FlowNode;
import org.jenkinsci.plugins.workflow.graph.FlowStartNode;
import org.jenkinsci.plugins.workflow.graphanalysis.FlowScanningUtils;
import org.jenkinsci.plugins.workflow.graphanalysis.LinearScanner;
import org.jenkinsci.plugins.workflow.steps.AbstractSynchronousStepExecution;
import org.jenkinsci.plugins.workflow.steps.FlowInterruptedException;
import org.jenkinsci.plugins.workflow.steps.StepContext;
Expand Down Expand Up @@ -85,30 +89,15 @@ public Void run() throws Exception {
* Gets the next ordinal and throw {@link AbortException} the milestone lives inside a parallel step branch.
*/
private synchronized int processOrdinal() throws AbortException {
// TODO: use FlowNodeSerialWalker when released (if possible)
FlowGraphWalker walker = new FlowGraphWalker();
walker.addHead(node);
Integer previousOrdinal = null;
int parallelDetectionEnabled = 0;
for (FlowNode n : walker) {

if (parallelDetectionEnabled <= 0 && n.getAction(ThreadNameAction.class) != null) {
throw new AbortException("Using a milestone step inside parallel is not allowed");
}

if (n instanceof BlockEndNode) {
parallelDetectionEnabled++;
} else if (n instanceof BlockStartNode && !(n instanceof FlowStartNode)) {
parallelDetectionEnabled--;
}

OrdinalAction a = n.getAction(OrdinalAction.class);
if (a != null) {
previousOrdinal = a.ordinal;
break;
}
List<FlowNode> heads = node.getExecution().getCurrentHeads();
if (heads.size() > 1) { // TA-DA! We're inside a parallel, which is forbidden.
throw new AbortException("Using a milestone step inside parallel is not allowed");
}

Predicate<FlowNode> ordinalMatcher = FlowScanningUtils.hasActionPredicate(OrdinalAction.class);
FlowNode lastOrdinalNode = new LinearScanner().findFirstMatch(heads, ordinalMatcher);
Integer previousOrdinal = (lastOrdinalNode != null) ? lastOrdinalNode.getAction(OrdinalAction.class).ordinal : null;

// If step.ordinal is set then use it and check order with the previous one
// Otherwise use calculated ordinal (previousOrdinal + 1)
int nextOrdinal = 0;
Expand Down Expand Up @@ -242,18 +231,9 @@ private static Integer getLastOrdinalInBuild(FlowExecutionOwner.Executable r) {
try {
List<FlowNode> heads = owner.get().getCurrentHeads();
if (heads.size() == 1) {
FlowGraphWalker walker = new FlowGraphWalker();
walker.addHead(heads.get(0));
for (FlowNode n : walker) {
OrdinalAction action = n.getAction(OrdinalAction.class);
if (action != null) {
lastMilestoneOrdinal = action.ordinal;
break;
}
}
} else {
LOGGER.log(Level.WARNING, "Trying to get last ordinal for a build still in progress?");
return null;
Predicate<FlowNode> ordinalMatcher = FlowScanningUtils.hasActionPredicate(OrdinalAction.class);
FlowNode lastOrdinalNode = new LinearScanner().findFirstMatch(heads, ordinalMatcher);
return (lastOrdinalNode != null) ? lastOrdinalNode.getAction(OrdinalAction.class).ordinal : null;
}
} catch (IOException e) {
LOGGER.log(Level.WARNING, "Failed to traverse flow graph to search the last milestone ordinal", e);
Expand Down
19 changes: 19 additions & 0 deletions src/test/java/org/jenkinsci/plugins/pipeline/milestone/MilestoneStepTest.java
View file
Expand Up @@ -9,6 +9,7 @@
import org.junit.Test;
import org.junit.runners.model.Statement;
import org.jvnet.hudson.test.BuildWatcher;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.RestartableJenkinsRule;

import static org.junit.Assert.*;
Expand Down Expand Up @@ -177,6 +178,24 @@ public void milestoneNotAllowedInsideParallel() {
});
}

@Issue("JENKINS-38464")
@Test
public void milestoneAllowedOutsideParallel() {
story.addStep(new Statement() {
@Override public void evaluate() throws Throwable {
WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p");
p.setDefinition(new CpsFlowDefinition(
"node {\n" +
" parallel one: { echo 'First' }, two: { \n" +
" echo 'In-node'\n" +
" }\n" +
" milestone 1\n" +
"}"));
story.j.assertBuildStatusSuccess(p.scheduleBuild2(0));
}
});
}

@Test
public void ordinals() {
story.addStep(new Statement() {
Expand Down

0 comments on commit 1c90cd1

Please sign in to comment.