[FIXED JENKINS-40984] Always evaluate/run all post conditions

Even if there's an error in an earlier condition execution, continue to the subsequent ones. Additionally, switch to doing a one-off check at the beginning of the post section to see if *any* conditions are satisfied at that time, and then when actually iterating through the conditions, check for satisfaction at *that* time. That's so that, for example, if the build is successful when it gets to evaluating post conditions and then there's a failure in the execution of `always`, the `success` block won't be executed, and the `failure` block will, since the build status has changed.
jenkinsci · Jan 11, 2017 · c1a7793 · c1a7793
1 parent 953af55
commit c1a7793
Show file tree

Hide file tree

Showing 7 changed files with 128 additions and 66 deletions.
diff --git a/...g/jenkinsci/plugins/pipeline/modeldefinition/model/AbstractBuildConditionResponder.groovy b/...g/jenkinsci/plugins/pipeline/modeldefinition/model/AbstractBuildConditionResponder.groovy
@@ -38,7 +38,6 @@ import org.jenkinsci.plugins.workflow.support.steps.build.RunWrapper
 public abstract class AbstractBuildConditionResponder<T extends AbstractBuildConditionResponder<T>>
     extends MappedClosure<StepsBlock,T> {
 
-
     @Override
     public void modelFromMap(Map<String,Object> inMap) {
 
@@ -52,20 +51,22 @@ public abstract class AbstractBuildConditionResponder<T extends AbstractBuildCon
         }
     }
 
-    public List<Closure> satisfiedConditions(Object runWrapperObj) {
-        RunWrapper runWrapper = (RunWrapper)runWrapperObj
-        WorkflowRun run = (WorkflowRun)runWrapper.getRawBuild()
+    public Closure closureForSatisfiedCondition(String conditionName, Object runWrapperObj) {
+        if (getMap().containsKey(conditionName)) {
+            BuildCondition condition = BuildCondition.getConditionMethods().get(conditionName)
+            if (condition != null && condition.meetsCondition(runWrapperObj)) {
+                return ((StepsBlock)getMap().get(conditionName)).getClosure()
+            }
+        }
 
-        List<Closure> closures = []
+        return null
+    }
 
+    public boolean satisfiedConditions(Object runWrapperObj) {
         Map<String,BuildCondition> conditions = BuildCondition.getConditionMethods()
 
-        BuildCondition.orderedConditionNames.each { conditionName ->
-            if (getMap().containsKey(conditionName) && conditions.get(conditionName).meetsCondition(run)) {
-                closures.add(((StepsBlock)getMap().get(conditionName)).getClosure())
-            }
+        return BuildCondition.orderedConditionNames.any { conditionName ->
+            getMap().containsKey(conditionName) && conditions.get(conditionName).meetsCondition(runWrapperObj)
         }
-
-        return closures
     }
 }
diff --git a/...finition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/Root.groovy b/...finition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/Root.groovy
@@ -117,16 +117,6 @@ public class Root implements NestedModel, Serializable {
         return m
     }
 
-    /**
-     * Returns a list of post-build closures whose conditions have been satisfied and should be run.
-     *
-     * @param runWrapperObj The {@link RunWrapper} for the build.
-     * @return a list of closures whose conditions have been satisfied.
-     */
-    List<Closure> satisfiedPostBuilds(Object runWrapperObj) {
-        return satisfiedConditionsForField(post, runWrapperObj)
-    }
-
     @Override
     public void modelFromMap(Map<String,Object> m) {
         m.each { k, v ->
@@ -135,17 +125,17 @@ public class Root implements NestedModel, Serializable {
     }
 
     /**
-     * Gets the list of satisfied build condition closures for the given responder.
+     * Returns true if at least one build condition for the given responder is satisfied currently.
      *
      * @param r an {@link AbstractBuildConditionResponder}, such as {@link PostStage} or {@link PostBuild}.
      * @param runWrapperObj The {@link RunWrapper} for the build.
-     * @return A list of closures from the responder which have had their conditions satisfied.
+     * @return True if at least one condition is satisfied, false otherwise.
      */
-    /*package*/ List<Closure> satisfiedConditionsForField(AbstractBuildConditionResponder r, Object runWrapperObj) {
+    /*package*/ boolean hasSatisfiedConditions(AbstractBuildConditionResponder r, Object runWrapperObj) {
         if (r != null) {
             return r.satisfiedConditions(runWrapperObj)
         } else {
-            return []
+            return false
         }
 
     }

diff --git a/...inition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/Stage.groovy b/...inition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/Stage.groovy
@@ -119,15 +119,4 @@ public class Stage implements NestedModel, Serializable {
             this."${k}"(v)
         }
     }
-
-    /**
-     * Returns a list of notification closures whose conditions have been satisfied and should be run.
-     *
-     * @param runWrapperObj The {@link org.jenkinsci.plugins.workflow.support.steps.build.RunWrapper} for the build.
-     * @return a list of closures whose conditions have been satisfied.
-     */
-    List<Closure> satisfiedPostStageConditions(Root root, Object runWrapperObj) {
-        return root.satisfiedConditionsForField(post, runWrapperObj)
-    }
-
 }
diff --git a/...on/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/model/BuildCondition.java b/...on/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/model/BuildCondition.java
@@ -28,6 +28,7 @@
 import hudson.ExtensionPoint;
 import org.jenkinsci.plugins.structs.SymbolLookup;
 import org.jenkinsci.plugins.workflow.job.WorkflowRun;
+import org.jenkinsci.plugins.workflow.support.steps.build.RunWrapper;
 
 import java.io.Serializable;
 import java.util.ArrayList;
@@ -46,6 +47,13 @@ public abstract class BuildCondition implements Serializable, ExtensionPoint {
 
     public abstract boolean meetsCondition(WorkflowRun r);
 
+    public boolean meetsCondition(Object runWrapperObj) {
+        RunWrapper runWrapper = (RunWrapper)runWrapperObj;
+        WorkflowRun run = (WorkflowRun)runWrapper.getRawBuild();
+
+        return meetsCondition(run);
+    }
+
     public abstract String getDescription();
 
     /**

diff --git a/...src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/ModelInterpreter.groovy b/...src/main/resources/org/jenkinsci/plugins/pipeline/modeldefinition/ModelInterpreter.groovy
@@ -366,7 +366,7 @@ public class ModelInterpreter implements Serializable {
      * @param thisStage The stage context we're running in
      */
     def executeSingleStage(Root root, Stage thisStage) throws Throwable {
-        Throwable stageError
+        Throwable stageError = null
         try {
             catchRequiredContextForNode(thisStage.agent ?: root.agent) {
                 setUpDelegate(thisStage.steps.closure)
@@ -379,22 +379,9 @@ public class ModelInterpreter implements Serializable {
             }
         } finally {
             // And finally, run the post stage steps.
-            List<Closure> postClosures = thisStage.satisfiedPostStageConditions(root, script.getProperty("currentBuild"))
-            catchRequiredContextForNode(thisStage.agent ?: root.agent) {
-                if (postClosures.size() > 0) {
-                    script.echo("Post stage")
-                    try {
-                        for (int ni = 0; ni < postClosures.size(); ni++) {
-                            setUpDelegate(postClosures.get(ni))
-                        }
-                    } catch (Exception e) {
-                        script.getProperty("currentBuild").result = Result.FAILURE
-                        Utils.markStageFailedAndContinued(thisStage.name)
-                        if (stageError == null) {
-                            stageError = e
-                        }
-                    }
-                }
+            if (root.hasSatisfiedConditions(thisStage.post, script.getProperty("currentBuild"))) {
+                script.echo("Post stage")
+                stageError = runPostConditions(thisStage.post, thisStage.agent ?: root.agent, stageError, thisStage.name)
             }
         }
 
@@ -425,27 +412,54 @@ public class ModelInterpreter implements Serializable {
      * @param root The root context we're executing in
      */
     def executePostBuild(Root root) throws Throwable {
-        Throwable stageError
-        List<Closure> postBuildClosures = root.satisfiedPostBuilds(script.getProperty("currentBuild"))
-        if (postBuildClosures.size() > 0) {
+        Throwable stageError = null
+        if (root.hasSatisfiedConditions(root.post, script.getProperty("currentBuild"))) {
+            script.stage(SyntheticStageNames.postBuild()) {
+                Utils.markSyntheticStage(SyntheticStageNames.postBuild(), Utils.getSyntheticStageMetadata().post)
+                stageError = runPostConditions(root.post, root.agent, stageError)
+            }
+        }
+
+        if (stageError != null) {
+            throw stageError
+        }
+    }
+
+    /**
+     * Actually does the execution of post actions, both post-stage and post-build.
+     * @param responder The {@link AbstractBuildConditionResponder} we're pulling conditions from.
+     * @param agentContext The {@link Agent} context we're running in.
+     * @param stageError Any existing error from earlier parts of the stage we're in, or null.
+     * @param stageName Optional - the name of the stage we're running in, so we can mark it as failed if needed.
+     * @return The stageError, which, if null when passed in and an error is hit, will be set to the first error encountered.
+     */
+    def runPostConditions(AbstractBuildConditionResponder responder,
+                          Agent agentContext,
+                          Throwable stageError,
+                          String stageName = null) {
+        List<String> orderedConditions = BuildCondition.orderedConditionNames
+        for (int i = 0; i < orderedConditions.size(); i++) {
             try {
-                script.stage(SyntheticStageNames.postBuild()) {
-                    Utils.markSyntheticStage(SyntheticStageNames.postBuild(), Utils.getSyntheticStageMetadata().post)
-                    catchRequiredContextForNode(root.agent) {
-                        for (int i = 0; i < postBuildClosures.size(); i++) {
-                            setUpDelegate(postBuildClosures.get(i))
-                        }
+                String conditionName = orderedConditions.get(i)
+
+                Closure c = responder.closureForSatisfiedCondition(conditionName, script.getProperty("currentBuild"))
+                if (c != null) {
+                    catchRequiredContextForNode(agentContext) {
+                        setUpDelegate(c)
                     }
                 }
             } catch (Exception e) {
                 script.getProperty("currentBuild").result = Result.FAILURE
-                stageError = e
+                if (stageName != null) {
+                    Utils.markStageFailedAndContinued(stageName)
+                }
+                if (stageError == null) {
+                    stageError = e
+                }
             }
         }
 
-        if (stageError != null) {
-            throw stageError
-        }
+        return stageError
     }
 
     /**

diff --git a/...test/java/org/jenkinsci/plugins/pipeline/modeldefinition/BuildConditionResponderTest.java b/...test/java/org/jenkinsci/plugins/pipeline/modeldefinition/BuildConditionResponderTest.java
@@ -47,6 +47,18 @@ public void simplePostBuild() throws Exception {
                 .go();
     }
 
+    @Test
+    public void postChecksAllConditions() throws Exception {
+        expect(Result.FAILURE, "postChecksAllConditions")
+                .logContains("[Pipeline] { (foo)",
+                        "hello",
+                        "[Pipeline] { (" + SyntheticStageNames.postBuild() + ")",
+                        "I AM FAILING NOW",
+                        "I FAILED")
+                .logNotContains("MOST DEFINITELY FINISHED")
+                .go();
+    }
+
     @Test
     public void postOnChanged() throws Exception {
         WorkflowRun b = getAndStartNonRepoBuild("postOnChangeFailed");

diff --git a/pipeline-model-definition/src/test/resources/postChecksAllConditions.groovy b/pipeline-model-definition/src/test/resources/postChecksAllConditions.groovy
@@ -0,0 +1,48 @@
+/*
+ * The MIT License
+ *
+ * Copyright (c) 2017, 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 {
+                echo "hello"
+            }
+        }
+    }
+    post {
+        always {
+            error "I AM FAILING NOW"
+        }
+        success {
+            echo "MOST DEFINITELY FINISHED"
+        }
+        failure {
+            echo "I FAILED"
+        }
+    }
+}
+
+
+