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

    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

          marcesher Marc Esher created issue -

          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?

          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?
          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

          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
          davidvanlaatum David van Laatum made changes -
          Field Original Value New Value
          Summary 2nd failure emails being sent even when build is successful 2nd failure emails being sent even when build is successful with job-dsl-plugin
          davidvanlaatum David van Laatum made changes -
          Component/s job-dsl-plugin [ 16720 ]

          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

          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

          2.47 released which should fix

          davidvanlaatum David van Laatum added a comment - 2.47 released which should fix
          davidvanlaatum David van Laatum made changes -
          Resolution Fixed [ 1 ]
          Status Open [ 1 ] Closed [ 6 ]
          marcesher Marc Esher added a comment -

          Thanks David!

          marcesher Marc Esher added a comment - Thanks David!
          daspilker Daniel Spilker made changes -
          Component/s job-dsl-plugin [ 16720 ]
          davidvanlaatum David van Laatum made changes -
          Component/s job-dsl-plugin [ 16720 ]

          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

          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

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

            Dates

              Created:
              Updated:
              Resolved: