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

2nd failure emails being sent even when build is successful with job-dsl-plugin

    XMLWordPrintable

    Details

    • Similar Issues:

      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

          Hide
          davidvanlaatum David van Laatum added a comment -

          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?

          Show
          davidvanlaatum David van Laatum added a comment - 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?
          Hide
          marcesher 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

          Show
          marcesher 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
          Hide
          davidvanlaatum David van Laatum added a comment -

          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

          Show
          davidvanlaatum David van Laatum added a comment - 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
          Hide
          davidvanlaatum David van Laatum added a comment -

          2.47 released which should fix

          Show
          davidvanlaatum David van Laatum added a comment - 2.47 released which should fix
          Hide
          marcesher Marc Esher added a comment -

          Thanks David!

          Show
          marcesher Marc Esher added a comment - Thanks David!
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          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

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - 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

            People

            • Assignee:
              davidvanlaatum David van Laatum
              Reporter:
              marcesher Marc Esher
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: