Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-38993

Deterministic order of build condition block execution in Declarative

      The build condition responders (currently postBuild and notifications, soon to be post within a stage) currently execute in the order they're specified in the Jenkinsfile. That should be changed to a deterministic order that'll be the same every time, and that order should be well documented and exposed in the logs.

          [JENKINS-38993] Deterministic order of build condition block execution in Declarative

          Andrew Bayer created issue -

          Andrew Bayer added a comment -

          stephenconnolly Any thoughts on what the order should be?

          Andrew Bayer added a comment - stephenconnolly Any thoughts on what the order should be?

          Stephen Connolly added a comment - - edited

          OK, this may be somewhat controversial...

          The only reason for a "success" condition is if that condition runs after "always"... if it were to run before "always" then we could just replace it with steps / stage at the end of the scope that the conditions apply to.

          Thus, as we are not allowing repeats, we have to start with the most general condition and then proceed to the most specific.

          Where conditions of the same level of specificity are mutually exclusive, run the defined order should be alphanumeric sort (not that the defined order impacts anything as they are all mutually exclusive, so only one will ever run)

          1. Always - the most generic condition
          2. Changed - the second most generic condition, won't match every build, but can potentially match builds of any status
          3. The group of mutually exclusive conditions

          • Aborted
          • Failure
          • Success
          • Unstable

          This also helps with failures in more generic conditions, as the Failure / Aborted conditions will be in the final set

          Stephen Connolly added a comment - - edited OK, this may be somewhat controversial... The only reason for a "success" condition is if that condition runs after "always"... if it were to run before "always" then we could just replace it with steps / stage at the end of the scope that the conditions apply to. Thus, as we are not allowing repeats, we have to start with the most general condition and then proceed to the most specific. Where conditions of the same level of specificity are mutually exclusive, run the defined order should be alphanumeric sort (not that the defined order impacts anything as they are all mutually exclusive, so only one will ever run) 1. Always - the most generic condition 2. Changed - the second most generic condition, won't match every build, but can potentially match builds of any status 3. The group of mutually exclusive conditions Aborted Failure Success Unstable This also helps with failures in more generic conditions, as the Failure / Aborted conditions will be in the final set

          Andrew Bayer added a comment -

          Sounds good to me. Implementing now. =)

          Andrew Bayer added a comment - Sounds good to me. Implementing now. =)
          Andrew Bayer made changes -
          Status Original: Open [ 1 ] New: In Progress [ 3 ]
          Andrew Bayer made changes -
          Status Original: In Progress [ 3 ] New: In Review [ 10005 ]

          Andrew Bayer added a comment -

          Andrew Bayer added a comment - PR up for review at https://github.com/jenkinsci/pipeline-model-definition-plugin/pull/32
          Andrew Bayer made changes -
          Remote Link New: This issue links to "PR #32 (Web Link)" [ 14963 ]

          Andrew Bayer added a comment -

          Merged, will be released in 0.5.

          Andrew Bayer added a comment - Merged, will be released in 0.5.

          Code changed in jenkins
          User: Andrew Bayer
          Path:
          pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/AbstractBuildConditionResponder.groovy
          pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/conditions/Aborted.groovy
          pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/conditions/Always.groovy
          pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/conditions/Changed.groovy
          pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/conditions/Failure.groovy
          pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/conditions/Success.groovy
          pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/conditions/Unstable.groovy
          pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidatorImpl.groovy
          pipeline-model-definition/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/model/BuildCondition.java
          pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/BuildConditionResponderTest.java
          pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ValidatorTest.java
          pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/endpoints/ModelConverterActionTest.java
          pipeline-model-definition/src/test/resources/buildConditionOrdering.groovy
          http://jenkins-ci.org/commit/pipeline-model-definition-plugin/57afb524540ed8be1d0eda9969209f9791d38c5a
          Log:
          [FIXED JENKINS-38993] Add deterministic ordering of conditions.

          Ordering is:

          • Always
          • Changed
          • Aborted/Failure/Success/Unstable

          This is all done via ordinals.

          SCM/JIRA link daemon added a comment - Code changed in jenkins User: Andrew Bayer Path: pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/AbstractBuildConditionResponder.groovy pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/conditions/Aborted.groovy pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/conditions/Always.groovy pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/conditions/Changed.groovy pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/conditions/Failure.groovy pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/conditions/Success.groovy pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/model/conditions/Unstable.groovy pipeline-model-definition/src/main/groovy/org/jenkinsci/plugins/pipeline/modeldefinition/validator/ModelValidatorImpl.groovy pipeline-model-definition/src/main/java/org/jenkinsci/plugins/pipeline/modeldefinition/model/BuildCondition.java pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/BuildConditionResponderTest.java pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/ValidatorTest.java pipeline-model-definition/src/test/java/org/jenkinsci/plugins/pipeline/modeldefinition/endpoints/ModelConverterActionTest.java pipeline-model-definition/src/test/resources/buildConditionOrdering.groovy http://jenkins-ci.org/commit/pipeline-model-definition-plugin/57afb524540ed8be1d0eda9969209f9791d38c5a Log: [FIXED JENKINS-38993] Add deterministic ordering of conditions. Ordering is: Always Changed Aborted/Failure/Success/Unstable This is all done via ordinals.
          Andrew Bayer made changes -
          Resolution New: Fixed [ 1 ]
          Status Original: In Review [ 10005 ] New: Resolved [ 5 ]

            abayer Andrew Bayer
            abayer Andrew Bayer
            Votes:
            0 Vote for this issue
            Watchers:
            8 Start watching this issue

              Created:
              Updated:
              Resolved: