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

No comments to Gerrit 2.13.1 for Change Merged trigger

    Details

    • Type: Bug
    • Status: Resolved (View Workflow)
    • Priority: Major
    • Resolution: Fixed
    • Component/s: gerrit-trigger-plugin
    • Labels:
      None
    • Environment:
      Jenkins 2.26, gerrit-trigger-plugin 2.22.0, Gerrit 2.13.1
    • Similar Issues:

      Description

      After upgrading of Gerrit from 2.12 to 2.13.1 and Jenkins to 2.26, we have lost comments for Change Merged triggered builds. We have both "Patchset created" and "Change Merged" triggers configured. "Patchset created" trigger votes Verified=1 and Code-Review=1, but "Change Merged" trigger votes Verified=0 and Code-Review=0 and Gerrit refuses this review (comment) with error "change is closed".

      Gerrit log
      [2016-10-20 08:03:05,924 +0200] 62f9143f jenkins a/4 LOGIN FROM 10.20.220.21
      [2016-10-20 08:03:06,298 +0200] 62f9143f jenkins a/4 gerrit.review.120,1.--message.Build Successful
      
      https://*****/25/ : SUCCESS.--label.Build-check=1.--code-review.1 1ms 85ms 0
      [2016-10-20 08:03:06,472 +0200] 62f9143f jenkins a/4 LOGOUT
      
      -----------------------------
      [2016-10-20 08:04:08,437 +0200] 623ef439 jenkins a/4 LOGIN FROM 10.20.220.21
      [2016-10-20 08:04:08,733 +0200] 623ef439 jenkins a/4 gerrit.review.120,1.--message.Build Successful
      
      https://*****/26/ : SUCCESS.--label.Build-check=0.--code-review.0 1ms 9ms 1
      [2016-10-20 08:04:08,921 +0200] 623ef439 jenkins a/4 LOGOUT
      
      Jenkins log
      Oct 20, 2016 8:03:03 AM com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.ToGerritRunListener onTriggered
      INFO: Project [Test] triggered by Gerrit: [[ManualPatchsetCreated Change: Change-Id for #120: Ib30b882237ecc6e6e55c26cd0763862a81e4ab9d PatchSet: PatchSet: 1]]
      Oct 20, 2016 8:03:03 AM com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.EventListener schedule
      INFO: Project Test Build Scheduled: true By event: 120/1
      Oct 20, 2016 8:03:03 AM com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.ToGerritRunListener onStarted
      INFO: Gerrit build [Test #25] Started for cause: [GerritCause: [ManualPatchsetCreated Change: Change-Id for #120: Ib30b882237ecc6e6e55c26cd0763862a81e4ab9d PatchSet: PatchSet: 1] silent: false].
      Oct 20, 2016 8:03:03 AM com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.ToGerritRunListener onStarted
      INFO: MemoryStatus:
        Project/Build: [Test]: [#25: null] Completed: false
      
      Oct 20, 2016 8:03:05 AM hudson.model.Run execute
      INFO: Test #25 main build action completed: SUCCESS
      Oct 20, 2016 8:03:05 AM com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.ToGerritRunListener allBuildsCompleted
      INFO: All Builds are completed for cause: GerritCause: [ManualPatchsetCreated Change: Change-Id for #120: Ib30b882237ecc6e6e55c26cd0763862a81e4ab9d PatchSet: PatchSet: 1] silent: false
      Oct 20, 2016 8:03:05 AM com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.GerritNotifier buildCompleted
      INFO: Notifying BuildCompleted to gerrit: gerrit review 120,1 --message 'Build Successful 
      
      https://*****/25/ : SUCCESS' --label Build-check=1 --code-review 1
      Oct 20, 2016 8:04:00 AM com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.ToGerritRunListener onTriggered
      INFO: Project [Test] triggered by Gerrit: [com.sonymobile.tools.gerrit.gerritevents.dto.events.ChangeMerged@c3398b77]
      Oct 20, 2016 8:04:00 AM com.sonyericsson.hudson.plugins.gerrit.trigger.hudsontrigger.EventListener schedule
      INFO: Project Test Build Scheduled: true By event: 120/1
      Oct 20, 2016 8:04:06 AM com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.ToGerritRunListener onStarted
      INFO: Gerrit build [Test #26] Started for cause: [GerritCause: com.sonymobile.tools.gerrit.gerritevents.dto.events.ChangeMerged@c3398b77 silent: false].
      Oct 20, 2016 8:04:06 AM com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.ToGerritRunListener onStarted
      INFO: MemoryStatus:
        Project/Build: [Test]: [#26: null] Completed: false
      
      Oct 20, 2016 8:04:07 AM hudson.model.Run execute
      INFO: Test #26 main build action completed: SUCCESS
      Oct 20, 2016 8:04:07 AM com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.ToGerritRunListener allBuildsCompleted
      INFO: All Builds are completed for cause: GerritCause: com.sonymobile.tools.gerrit.gerritevents.dto.events.ChangeMerged@c3398b77 silent: false
      Oct 20, 2016 8:04:07 AM com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.GerritNotifier buildCompleted
      INFO: Notifying BuildCompleted to gerrit: gerrit review 120,1 --message 'Build Successful 
      
      https://*****/26/ : SUCCESS' --label Build-check=0 --code-review 0
      Oct 20, 2016 8:04:08 AM com.sonymobile.tools.gerrit.gerritevents.workers.cmd.AbstractSendCommandJob sendCommand
      SEVERE: Could not run command gerrit review 120,1 --message 'Build Successful 
      
      https://*****/26/ : SUCCESS' --label Build-check=0 --code-review 0
      java.io.IOException: Error during sending command
      	at com.sonymobile.tools.gerrit.gerritevents.workers.cmd.AbstractSendCommandJob.sendCommand2(AbstractSendCommandJob.java:118)
      	at com.sonymobile.tools.gerrit.gerritevents.workers.cmd.AbstractSendCommandJob.sendCommand(AbstractSendCommandJob.java:79)
      	at com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.GerritNotifier.buildCompleted(GerritNotifier.java:118)
      	at com.sonyericsson.hudson.plugins.gerrit.trigger.gerritnotifier.job.ssh.BuildCompletedCommandJob.run(BuildCompletedCommandJob.java:71)
      	at java.util.concurrent.Executors$RunnableAdapter.call(Unknown Source)
      	at java.util.concurrent.FutureTask.run(Unknown Source)
      	at java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
      	at java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
      	at java.lang.Thread.run(Unknown Source)
      Caused by: com.sonymobile.tools.gerrit.gerritevents.ssh.SshException: error: fatal: change is closed
      
      fatal: one or more reviews failed; review output above (1)
      	at com.sonymobile.tools.gerrit.gerritevents.ssh.SshConnectionImpl.executeCommand(SshConnectionImpl.java:254)
      	at com.sonymobile.tools.gerrit.gerritevents.workers.cmd.AbstractSendCommandJob.sendCommand2(AbstractSendCommandJob.java:116)
      	... 8 more
      

        Attachments

          Activity

          Show
          engy Jiří Engelthaler added a comment - It's because "Change Merged" is not scorable https://github.com/sonyxperiadev/gerrit-events/blob/master/src/main/java/com/sonymobile/tools/gerrit/gerritevents/dto/events/ChangeMerged.java#L70 and gerrit-trigger-plugins leave values set to zero https://github.com/jenkinsci/gerrit-trigger-plugin/blob/master/src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java#L527 But Gerrit refuses different votes after the change is merged.
          Hide
          engy Jiří Engelthaler added a comment -

          No news? Please fix the issue.

          Show
          engy Jiří Engelthaler added a comment - No news? Please fix the issue.
          Hide
          rsandell rsandell added a comment -

          There used to be a -f or --force flag to the review command that would add the comment even though the votes couldn't be applied, specifically added for this occasion. But I can't find it in the Gerrit documentation, perhaps it has been removed.

          Show
          rsandell rsandell added a comment - There used to be a -f or --force flag to the review command that would add the comment even though the votes couldn't be applied, specifically added for this occasion. But I can't find it in the Gerrit documentation, perhaps it has been removed.
          Hide
          engy Jiří Engelthaler added a comment - - edited

          I use this Verified command
          gerrit review <CHANGE>,<PATCHSET> --message 'Build Successful <BUILDS_STATS>' --label Build-check=<VERIFIED> --code-review <CODE_REVIEW>
          and <CODE_REVIEW> is always 0. When I call
          ssh ... gerrit review 122,1 --message X1 --label Build-check=0 --code-review=0
          on closed review, it' reported as

          error: fatal: change is closed

          fatal: one or more reviews failed; review output above

          but this works
          ssh ... gerrit review 122,1 --message X1 --code-review=0
          ssh ... gerrit review 122,1 --message X1 --label Build-check=999 --code-review=0

          I have too look inside gerrit source code, but the solution in gerrit trigger plugin can be removing --label and --code-review for not scorable comments.

          Show
          engy Jiří Engelthaler added a comment - - edited I use this Verified command gerrit review <CHANGE>,<PATCHSET> --message 'Build Successful <BUILDS_STATS>' --label Build-check=<VERIFIED> --code-review <CODE_REVIEW> and <CODE_REVIEW> is always 0. When I call ssh ... gerrit review 122,1 --message X1 --label Build-check=0 --code-review=0 on closed review, it' reported as error: fatal: change is closed fatal: one or more reviews failed; review output above but this works ssh ... gerrit review 122,1 --message X1 --code-review=0 ssh ... gerrit review 122,1 --message X1 --label Build-check=999 --code-review=0 I have too look inside gerrit source code, but the solution in gerrit trigger plugin can be removing --label and --code-review for not scorable comments.
          Hide
          engy Jiří Engelthaler added a comment -

          I found a commit in Gerrit that prevents from changing votes on closed commits

          https://gerrit-review.googlesource.com/#/c/72154/

          Show
          engy Jiří Engelthaler added a comment - I found a commit in Gerrit that prevents from changing votes on closed commits https://gerrit-review.googlesource.com/#/c/72154/
          Hide
          engy Jiří Engelthaler added a comment -

          This

          ssh ... gerrit review 122,1 --message X1 --label Build-check=999 --code-review=0
          

          works, because Gerrit ignores votes out of range for labels.

          Show
          engy Jiří Engelthaler added a comment - This ssh ... gerrit review 122,1 --message X1 --label Build-check=999 --code-review=0 works, because Gerrit ignores votes out of range for labels.
          Hide
          engy Jiří Engelthaler added a comment -

          I found that it should accept votes for closed changes unless --strict-labels is used

          --label
          
              Set a label by name to the value 'N'. Invalid votes (invalid label or invalid value) and votes that are not permitted for the user are silently ignored.
          
          --strict-labels
          
              Require ability to vote on all specified labels before reviewing change. If the vote is invalid (invalid label or invalid name), the vote is not permitted for the user, or the vote is on an outdated or closed patch set, return an error instead of silently discarding the vote.
          
          
          Show
          engy Jiří Engelthaler added a comment - I found that it should accept votes for closed changes unless --strict-labels is used --label Set a label by name to the value 'N' . Invalid votes (invalid label or invalid value) and votes that are not permitted for the user are silently ignored. --strict-labels Require ability to vote on all specified labels before reviewing change. If the vote is invalid (invalid label or invalid name), the vote is not permitted for the user, or the vote is on an outdated or closed patch set, return an error instead of silently discarding the vote.
          Hide
          engy Jiří Engelthaler added a comment - - edited

          The issue for Gerrit created https://bugs.chromium.org/p/gerrit/issues/detail?id=5046
          But gerrit-trigger-plugin may be also fixed

          Show
          engy Jiří Engelthaler added a comment - - edited The issue for Gerrit created https://bugs.chromium.org/p/gerrit/issues/detail?id=5046 But gerrit-trigger-plugin may be also fixed
          Hide
          tykeal Andrew Grimberg added a comment -

          It would be really nice if this could see some movement. My java skills are seriously lacking or I would take a try at it. Here's what I think should happen though:

          1. Add another configuration option for just the message so that it's still overridable, label it as merged only comment, or maybe failsafe comment.
          2. If a gerrit comment / review fails, fall back and try using that. If that fails, then log / throw the exception that's currently happening.
          Show
          tykeal Andrew Grimberg added a comment - It would be really nice if this could see some movement. My java skills are seriously lacking or I would take a try at it. Here's what I think should happen though: Add another configuration option for just the message so that it's still overridable, label it as merged only comment, or maybe failsafe comment. If a gerrit comment / review fails, fall back and try using that. If that fails, then log / throw the exception that's currently happening.
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Alex
          Path:
          src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java
          src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java
          http://jenkins-ci.org/commit/gerrit-trigger-plugin/48e0ddc13d72ca57b884f1fe0f7c1f5b479d39c2
          Log:
          [FIX JENKINS-39132] No comments to Gerrit 2.13.1 for Change Merged trigger

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Alex Path: src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java http://jenkins-ci.org/commit/gerrit-trigger-plugin/48e0ddc13d72ca57b884f1fe0f7c1f5b479d39c2 Log: [FIX JENKINS-39132] No comments to Gerrit 2.13.1 for Change Merged trigger
          Hide
          scm_issue_link SCM/JIRA link daemon added a comment -

          Code changed in jenkins
          User: Robert Sandell
          Path:
          src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java
          src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java
          http://jenkins-ci.org/commit/gerrit-trigger-plugin/c24533f38a19c0abe32d321388b8d85ff7cead06
          Log:
          Merge pull request #330 from Jimilian/JENKINS_39132

          [FIX JENKINS-39132] No comments to Gerrit 2.13.1 for Change Merged trigger

          Compare: https://github.com/jenkinsci/gerrit-trigger-plugin/compare/aac6a0a53ad7...c24533f38a19

          Show
          scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Robert Sandell Path: src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java http://jenkins-ci.org/commit/gerrit-trigger-plugin/c24533f38a19c0abe32d321388b8d85ff7cead06 Log: Merge pull request #330 from Jimilian/JENKINS_39132 [FIX JENKINS-39132] No comments to Gerrit 2.13.1 for Change Merged trigger Compare: https://github.com/jenkinsci/gerrit-trigger-plugin/compare/aac6a0a53ad7...c24533f38a19
          Hide
          engy Jiří Engelthaler added a comment - - edited

          Not working for custom label names such as
          gerrit review <CHANGE>,<PATCHSET> --message 'Build Successful <BUILDS_STATS>' --label Build-check=<VERIFIED> --code-review <CODE_REVIEW>

          The command issued to Gerrit looks like
          [2017-12-07 13:12:24,180 +0100] 1de0b61a jenkins a/4 gerrit.review.144,1.--message.Build Successful https://xxxxx/jenkins/job/test/96/ : SUCCESS. --label.Build-check=null 2ms 9ms 1

          Show
          engy Jiří Engelthaler added a comment - - edited Not working for custom label names such as gerrit review <CHANGE>,<PATCHSET> --message 'Build Successful <BUILDS_STATS>' --label Build-check=<VERIFIED> --code-review <CODE_REVIEW> The command issued to Gerrit looks like [2017-12-07 13:12:24,180 +0100] 1de0b61a jenkins a/4 gerrit.review.144,1.--message.Build Successful https://xxxxx/jenkins/job/test/96/ : SUCCESS. --label. Build-check=null 2ms 9ms 1

            People

            • Assignee:
              rsandell rsandell
              Reporter:
              engy Jiří Engelthaler
            • Votes:
              4 Vote for this issue
              Watchers:
              7 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: