-
Improvement
-
Resolution: Fixed
-
Minor
-
None
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.
- relates to
-
-
- Closed
-
- links to
[JENKINS-38993] Deterministic order of build condition block execution in Declarative
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
PR up for review at https://github.com/jenkinsci/pipeline-model-definition-plugin/pull/32
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.
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/198a42351cdc43045d6ab15c4df94a186d087672
Log:
Merge pull request #32 from abayer/jenkins-38993
[FIXED JENKINS-38993] Add deterministic ordering of conditions.
The deterministic order is irritating to deal with when having to do something like this because Macs that have static workspaces and working with multiple nodes.
I would prefer to have it follow my order or actually lower the always to something below success and failure...
I could do the success as part of the stages but success provide me with a logical block to put certain events into the proper block
post {
failure {
archiveArtifacts 'fastlane/logs/**/*.log'
}
success {
script {
if (env.BRANCH_NAME == 'development') {
stash name: 'debug-ipa', includes: "output/*.ipa"
}
}
}
always {
delete()
}
}
scott carlson
added a comment - Just want to add we currently feel the same was casz feels above, follow my order or at the least always would be better at the end. Thankfully we have a way around it for now.
scott carlson
added a comment - Just want to add we currently feel the same was casz feels above, follow my order or at the least always would be better at the end. Thankfully we have a way around it for now. abayer Agree with the two previous comments, for a similar reason:
post {
always {
deleteDir()
}
success {
emailext to: 'some@address.mail',
subject: 'Build was successful!',
mimeType: 'text/html',
body: readFile('releaseNotes.html') -- file is already deleted by "always" post-condition above
}
}
I think we should reopen this issue, if you disagree Andrew, go ahead and close it.
Yeah, I'm closing this. JENKINS-41519 covers documenting the order, since that's not actually documented at this point.
stephenconnolly Any thoughts on what the order should be?