[FIXED JENKINS-40136] Properly support failFast in parallel.

jenkinsci · Dec 1, 2016 · 41074d2 · 41074d2
1 parent e27901b
commit 41074d2
Show file tree

Hide file tree

Showing 12 changed files with 255 additions and 29 deletions.
diff --git a/...l-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTStage.java b/...l-api/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/ast/ModelASTStage.java
@@ -21,6 +21,7 @@ public final class ModelASTStage extends ModelASTElement {
     private ModelASTWhen when;
     private ModelASTTools tools;
     private ModelASTEnvironment environment;
+    private Boolean failFast;
 
     public ModelASTStage(Object sourceLocation) {
         super(sourceLocation);
@@ -35,6 +36,9 @@ public JSONObject toJSON() {
         JSONObject o = new JSONObject();
         o.accumulate("name", name);
         o.accumulate("branches", a);
+        if (failFast != null) {
+            o.accumulate("failFast", failFast);
+        }
         if (agent != null) {
             o.accumulate("agent", agent.toJSON());
         }
@@ -110,6 +114,9 @@ public String toGroovy() {
                 result.append('\n');
                 result.append(branch.getName()).append(": {\n").append(branch.toGroovy()).append("\n}");
             }
+            if (failFast != null && failFast) {
+                result.append(",\nfailFast: true");
+            }
             result.append("\n)\n");
         } else if (branches.size() == 1) {
             result.append(branches.get(0).toGroovy());
@@ -205,6 +212,14 @@ public void setEnvironment(ModelASTEnvironment environment) {
         this.environment = environment;
     }
 
+    public Boolean getFailFast() {
+        return failFast;
+    }
+
+    public void setFailFast(Boolean f) {
+        this.failFast = f;
+    }
+
     @Override
     public String toString() {
         return "ModelASTStage{" +
@@ -215,6 +230,7 @@ public String toString() {
                 ", post=" + post +
                 ", tools=" + tools +
                 ", environment=" + environment +
+                ", failFast=" + failFast +
                 "}";
     }
 
@@ -245,6 +261,9 @@ public boolean equals(Object o) {
             return false;
         }
 
+        if (getFailFast() != null ? !getFailFast().equals(that.getFailFast()) : that.getFailFast() != null) {
+            return false;
+        }
         if (getTools() != null ? !getTools().equals(that.getTools()) : that.getTools() != null) {
             return false;
         }
@@ -265,6 +284,7 @@ public int hashCode() {
         result = 31 * result + (getPost() != null ? getPost().hashCode() : 0);
         result = 31 * result + (getTools() != null ? getTools().hashCode() : 0);
         result = 31 * result + (getEnvironment() != null ? getEnvironment().hashCode() : 0);
+        result = 31 * result + (getFailFast() != null ? getFailFast().hashCode() : 0);
         return result;
     }
 }
diff --git a/pipeline-model-api/src/main/resources/ast-schema.json b/pipeline-model-api/src/main/resources/ast-schema.json
@@ -326,6 +326,9 @@
         "name": {
           "type": "string"
         },
+        "failFast": {
+          "type": "boolean"
+        },
         "agent": {
           "$ref": "#/definitions/agent"
         },

diff --git a/...n/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/JSONParser.groovy b/...n/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/JSONParser.groovy
@@ -151,6 +151,10 @@ class JSONParser {
             stage.branches.add(parseBranch(o))
         }
 
+        if (j.has("failFast") && stage.branches.size() > 1) {
+            stage.failFast = j.getBoolean("failFast")
+        }
+
         if (j.has("environment")) {
             stage.environment = parseEnvironment(j.getJSONArray("environment"))
         }

diff --git a/.../src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ModelParser.groovy b/.../src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ModelParser.groovy
@@ -306,7 +306,25 @@ class ModelParser {
                             break
                         case 'steps':
                             def stepsBlock = matchBlockStatement(s);
-                            stage.branches.addAll(parseStepsBlock(asBlock(stepsBlock.body.code)))
+                            BlockStatement block = asBlock(stepsBlock.body.code)
+
+                            // Handle parallel as a special case
+                            if (block.statements.size()==1) {
+                                def parallel = matchParallel(block.statements[0]);
+
+                                if (parallel != null) {
+                                    parallel.args.each { k, v ->
+                                        stage.branches.add(parseBranch(k, asBlock(v.code)));
+                                    }
+                                    stage.failFast = parallel.failFast
+                                } else {
+                                    // otherwise it's a single line of execution
+                                    stage.branches.add(parseBranch("default", block));
+                                }
+                            } else {
+                                // otherwise it's a single line of execution
+                                stage.branches.add(parseBranch("default", block));
+                            }
                             break
                         case 'post':
                             stage.post = parsePostStage(s)
@@ -326,26 +344,6 @@ class ModelParser {
         return stage
     }
 
-    protected List<ModelASTBranch> parseStepsBlock(BlockStatement block) {
-        List<ModelASTBranch> branches = []
-
-        // Handle parallel as a special case
-        if (block.statements.size()==1) {
-            def parallel = matchParallel(block.statements[0]);
-            if (parallel != null) {
-                parallel.args.each { k, v ->
-                    branches.add(parseBranch(k, asBlock(v.code)));
-                }
-                return branches
-            }
-        }
-
-        // otherwise it's a single line of execution
-        branches.add(parseBranch("default", block));
-
-        return branches
-    }
-
     /**
      * Parses a block of code into {@link ModelASTBranch}
      */
@@ -853,21 +851,32 @@ class ModelParser {
 
                 def args = (TupleExpression)whole.arguments; // list of arguments. in this case it should be just one
                 int sz = args.expressions.size();
-                def r = new ParallelMatch(whole);
+                Boolean failFast = null
+                Map<String,ClosureExpression> parallelArgs = new LinkedHashMap<>()
                 if (sz==1) {
                     def branches = castOrNull(NamedArgumentListExpression, args.getExpression(sz - 1));
                     if (branches!=null) {
                         for (MapEntryExpression e : branches.mapEntryExpressions) {
-                            ClosureExpression value = castOrNull(ClosureExpression, e.valueExpression);
-                            if (value == null) {
-                                errorCollector.error(new ModelASTKey(e.keyExpression, null), "Expected closure")
+                            String keyName = matchStringLiteral(e.keyExpression)
+                            if (keyName != null && keyName.equals("failFast")) {
+                                ConstantExpression exp = castOrNull(ConstantExpression.class, e.valueExpression)
+                                if (exp == null || !(exp.value instanceof Boolean)) {
+                                    errorCollector.error(new ModelASTKey(e.keyExpression), "Expected a boolean with failFast")
+                                } else {
+                                    failFast = exp.value
+                                }
                             } else {
-                                r.args[parseStringLiteral(e.keyExpression)] = value;
+                                ClosureExpression value = castOrNull(ClosureExpression, e.valueExpression);
+                                if (value == null) {
+                                    errorCollector.error(new ModelASTKey(e.keyExpression), "Expected closure or failFast")
+                                } else {
+                                    parallelArgs[parseStringLiteral(e.keyExpression)] = value;
+                                }
                             }
                         }
                     }
                 }
-                return r;
+                return new ParallelMatch(whole, parallelArgs, failFast);
             }
         }
 

diff --git a/...rc/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ParallelMatch.groovy b/...rc/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/parser/ParallelMatch.groovy
@@ -29,7 +29,14 @@ public class ParallelMatch {
      */
     public final Map<String,ClosureExpression> args = new LinkedHashMap<String, ClosureExpression>();
 
-    public ParallelMatch(MethodCallExpression whole) {
+    /**
+     * Whether this parallel invocation should fail fast
+     */
+    public final Boolean failFast;
+
+    public ParallelMatch(MethodCallExpression whole, Map<String,ClosureExpression> args, Boolean failFast) {
         this.whole = whole;
+        this.args.putAll(args)
+        this.failFast = failFast
     }
 }
diff --git a/...on/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AbstractModelDefTest.java b/...on/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/AbstractModelDefTest.java
@@ -129,7 +129,8 @@ public void setUp() throws Exception {
             "toolsInStage",
             "environmentInStage",
             "basicWhen",
-            "skippedWhen"
+            "skippedWhen",
+            "parallelPipelineWithFailFast"
     );
 
     public static Iterable<Object[]> configsWithErrors() {

diff --git a/...ition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/BasicModelDefTest.java b/...ition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/BasicModelDefTest.java
@@ -160,6 +160,13 @@ public void parallelPipeline() throws Exception {
                 .go();
     }
 
+    @Test
+    public void parallelPipelineWithFailFast() throws Exception {
+        expect("parallelPipelineWithFailFast")
+                .logContains("[Pipeline] { (foo)", "[first] { (Branch: first)", "[second] { (Branch: second)")
+                .go();
+    }
+
     @Test
     public void executionModelAction() throws Exception {
         prepRepoWithJenkinsfile("executionModelAction");

diff --git a/...efinition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ValidatorTest.java b/...efinition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ValidatorTest.java
@@ -172,6 +172,20 @@ public void emptyParallel() throws Exception {
                 .go();
     }
 
+    @Test
+    public void parallelPipelineWithInvalidFailFast() throws Exception {
+        expect(Result.FAILURE, "errors", "parallelPipelineWithInvalidFailFast")
+                .logContains("Expected a boolean with failFast")
+                .go();
+    }
+
+    @Test
+    public void parallelPipelineWithInvalidExtraKey() throws Exception {
+        expect(Result.FAILURE, "errors", "parallelPipelineWithInvalidExtraKey")
+                .logContains("Expected closure or failFast")
+                .go();
+    }
+
     @Test
     public void missingAgent() throws Exception {
         expect(Result.FAILURE, "errors", "missingAgent")

diff --git a/...ine-model-definition/src/test/resources/errors/parallelPipelineWithInvalidExtraKey.groovy b/...ine-model-definition/src/test/resources/errors/parallelPipelineWithInvalidExtraKey.groovy
@@ -0,0 +1,43 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2016, 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.
+ */
+
+pipeline {
+    agent none
+    stages {
+        stage("foo") {
+            steps {
+                parallel(first: {
+                    echo "First branch"
+                },
+                    second: {
+                        echo "Second branch"
+                    },
+                    failReallySlow: true)
+            }
+        }
+    }
+}
+
+
+
diff --git a/...ine-model-definition/src/test/resources/errors/parallelPipelineWithInvalidFailFast.groovy b/...ine-model-definition/src/test/resources/errors/parallelPipelineWithInvalidFailFast.groovy
@@ -0,0 +1,43 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2016, 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.
+ */
+
+pipeline {
+    agent none
+    stages {
+        stage("foo") {
+            steps {
+                parallel(first: {
+                    echo "First branch"
+                },
+                    second: {
+                        echo "Second branch"
+                    },
+                    failFast: "banana")
+            }
+        }
+    }
+}
+
+
+
diff --git a/pipeline-model-definition/src/test/resources/json/parallelPipelineWithFailFast.json b/pipeline-model-definition/src/test/resources/json/parallelPipelineWithFailFast.json
@@ -0,0 +1,32 @@
+{"pipeline": {
+  "stages": [  {
+    "name": "foo",
+    "branches":     [
+      {
+        "name": "second",
+        "steps": [        {
+          "name": "echo",
+          "arguments":           {
+            "isLiteral": true,
+            "value": "Second branch"
+          }
+        }]
+      },
+      {
+        "name": "first",
+        "steps": [        {
+          "name": "echo",
+          "arguments":           {
+            "isLiteral": true,
+            "value": "First branch"
+          }
+        }]
+      }
+    ],
+    "failFast": true
+  }],
+  "agent":   {
+    "isLiteral": true,
+    "value": "none"
+  }
+}}