Details
-
Bug
-
Status: Closed (View Workflow)
-
Minor
-
Resolution: Fixed
-
None
-
Jenkins 2+, running on linux
Description
I have just configured some jobs to send emails on "Failure - 2nd" and "Fixed".
The behavior I'm seeing is that if the job has ever failed in the past, then the "Failure - 2nd" email gets sent on every subsequent build, even if the build is successful.
Here's the output I see in Jenkins:
Email was triggered for: Failure - 2nd Trigger Failure - Any was overridden by another trigger and will not send an email. Trigger Failure - Still was overridden by another trigger and will not send an email. Sending email for trigger: Failure - 2nd Sending email to: ...... Finished: SUCCESS
The expected behavior, according to the plugin doc, is that it would only be sent on successive failures. Note that this build is neither a failure, nor a successive failure.
Attachments
Activity
Marc Esher
added a comment - Hi David,
I believe I've gotten to the bottom of this. I'm not sure what the exact right fix is, because it seems there might be several. Bear with me, this might take a while:
We use job-dsl-plugin to configure our jobs. And when I added a secondFailure trigger via job-dsl-plugin, that's when I observed the behavior I described. If I then re-saved the job in the Jenkins UI, the problem went away. This led me to investigate the difference in config.xml between the job-dsl generated version and the JenkinsUI generated version, and it was this:
in the job-dsl generated job, I saw: "<failureCount>0</failureCount>" for the SecondFailureTrigger element.
But in the Jenkins UI generated job, I saw "<failureCount>2</failureCount>" for the SecondFailureTrigger element.
I then added some debugging to the email-ext and uploaded, and saw that in https://github.com/jenkinsci/email-ext-plugin/blob/master/src/main/java/hudson/plugins/emailext/plugins/trigger/NthFailureTrigger.java#L35-L53, because failureCount was 0, then that entire loop was skipped, and it'd hit the last line:
return run == null || run.getResult() == Result.SUCCESS || run.getResult() == Result.UNSTABLE;
And since the result was SUCCESS, it'd return true, and the email would get triggered.
So there are 2 problems:
1) for some reason, the job-dsl version generates a failureCount of 0, which causes the for loop to get skipped
2) the return seems, to me, to have incorrect logic. I can't figure out why it'd return true if the result was SUCCESS.
So to fix this, I noticed that FirstFailureTrigger has a readResolve() block that seems to cover this exact condition. Thus, I added a readResolve() block to SecondFailureTrigger, just replacing "1" with "2"
Re-uploaded the plugin, and that fixed everything.
So to me, that seems like a sensible fix and I'm happy to submit a PR if you think that's correct.
Still, though, that return in trigger() seems off to me, so you probably want to check that out. If you think the logic should be Result.FAILURE instead of Result.SUCCESS, let me know and I can add that to the PR
Marc Esher
added a comment - Hi David,
I believe I've gotten to the bottom of this. I'm not sure what the exact right fix is, because it seems there might be several. Bear with me, this might take a while:
We use job-dsl-plugin to configure our jobs. And when I added a secondFailure trigger via job-dsl-plugin, that's when I observed the behavior I described. If I then re-saved the job in the Jenkins UI, the problem went away. This led me to investigate the difference in config.xml between the job-dsl generated version and the JenkinsUI generated version, and it was this:
in the job-dsl generated job, I saw: "<failureCount>0</failureCount>" for the SecondFailureTrigger element.
But in the Jenkins UI generated job, I saw "<failureCount>2</failureCount>" for the SecondFailureTrigger element.
I then added some debugging to the email-ext and uploaded, and saw that in https://github.com/jenkinsci/email-ext-plugin/blob/master/src/main/java/hudson/plugins/emailext/plugins/trigger/NthFailureTrigger.java#L35-L53 , because failureCount was 0, then that entire loop was skipped, and it'd hit the last line:
return run == null || run.getResult() == Result.SUCCESS || run.getResult() == Result.UNSTABLE;
And since the result was SUCCESS, it'd return true, and the email would get triggered.
So there are 2 problems:
1) for some reason, the job-dsl version generates a failureCount of 0, which causes the for loop to get skipped
2) the return seems, to me, to have incorrect logic. I can't figure out why it'd return true if the result was SUCCESS.
So to fix this, I noticed that FirstFailureTrigger has a readResolve() block that seems to cover this exact condition. Thus, I added a readResolve() block to SecondFailureTrigger, just replacing "1" with "2"
Re-uploaded the plugin, and that fixed everything.
So to me, that seems like a sensible fix and I'm happy to submit a PR if you think that's correct.
Still, though, that return in trigger() seems off to me, so you probably want to check that out. If you think the logic should be Result.FAILURE instead of Result.SUCCESS, let me know and I can add that to the PR That was done rather poorly 
1) It was using a class variable that didn't follow the correct data bound pattern
2) It's checking for the nth failure after a success so 3 failures in a row the second will trigger the email but the third will not
Marc Esher
added a comment - Thanks David! Code changed in jenkins
User: David van Laatum
Path:
src/main/java/hudson/plugins/emailext/plugins/trigger/FirstFailureTrigger.java
src/main/java/hudson/plugins/emailext/plugins/trigger/NthFailureTrigger.java
src/main/java/hudson/plugins/emailext/plugins/trigger/SecondFailureTrigger.java
src/test/java/hudson/plugins/emailext/plugins/trigger/FirstFailureTriggerTest.java
src/test/resources/hudson/plugins/emailext/plugins/trigger/oldformat.xml
http://jenkins-ci.org/commit/email-ext-plugin/f75318d75a5129a0d91773906962efc24337fdcf
Log:
JENKINS-37188 2nd failure emails being sent even when build is successful with job-dsl-plugin
I am unable to reproduce are you able to reproduce on a simple job with just this plugin and a shell step to make the build pass or fail?