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

Unable to send core-review +2 from one job if 2 jobs triggered for same event

    Details

    • Similar Issues:

      Description

      Current Scenario

      A user wants to be able to send core-review +2 for a specific job even though other jobs may be triggered for the same event. Currently, the resulting core-review value sent is 0.

      Setup

      The setup required to be able to have 2 jobs (ABC and DEF) send different label values is as follows:

      • At the global level, the value for code review on Build Successful defaults to 0.
      • At the global level, the value for verified on Build Successful defaults to 1.
      • Job DEF is configured to send code-review = 2 on Build Successful

      When the jobs are triggered, the resulting command to send to Gerrit that is built by the Gerrit Trigger contains:

      --verified 1 --code-review 0

      This is perceived as incorrect since we have asked for the code-review = 2

      Problem Investigation

      However, the Gerrit Trigger works as follows:

      • The build from Job ABC finishes and the code review value is determined to be 0 since the job has not overridden the default.
      • The build from Job DEF finished and the code review value is determined to be 2 since it has overridden the default.
      • The plugin then calculates a minimum value between the 2 values and hence determines it to be 0

      In this case, it would make sense to alter the behavior to not allow jobs that have not overridden code review on Build Successful to contribute to the final value of code-review.

      Even, if we can change this behavior, the situation is further complicated by the fact that:
      The command to send is built using a command template that contains "--code-review <CODE REVIEW>"
      Gerrit Trigger will expand the above string to insert the correct value.

      In the case where 2 jobs have not overridden the default, it would make sense to remove the entire string "--code-review <CODE REVIEW>" from the command to send. Doing this would not be straightforward.

      Way forward #1

      • Alter the behavior to not allow jobs that have not overridden code review on Build Successful to contribute to the final value of code-review.
        • This means that we can have one Jenkins job setting only the Verified label to +1 or -1 and another Jenkins job setting only Code-review label to +1 or -1 for the same patch-set in Gerrit.
      • We could decide the send a default of 0 when no jobs have overridden the default.

      Way forward #2

      In order to fix this properly, major rework would be required within the Gerrit Trigger plugin. The work would be in the context of support Custom Labels and the code-review would be added and treated as such. This improvement has already been mentioned here: JENKINS-27873

      The work would involve a re-work of the config UIs and a refactoring of the Notifying classes.

      This would be very interesting as it would further enhance the integration between Jenkins and Gerrit by making the submission of changes more readily possible to automate.

        Attachments

          Issue Links

            Activity

            Hide
            rsandell rsandell added a comment -

            I just mentioned in JENKINS-30393 that providing functionality for custom/dynamic labels would be a good way forward for that "bug" as well. I would like to see that support coming into the trigger and then possibly more dynamic handling of other things would hopefully follow as a bonus.

            Show
            rsandell rsandell added a comment - I just mentioned in JENKINS-30393 that providing functionality for custom/dynamic labels would be a good way forward for that "bug" as well. I would like to see that support coming into the trigger and then possibly more dynamic handling of other things would hopefully follow as a bonus.
            Hide
            hluhr Harald Luhr added a comment -

            Hi,

            Sorry, for the delayed response.

            Not sure if I understand the "way forward #1"...sorry for my limited English.
            What does "Alter the behavior to not allow jobs that have not overridden code review on Build Successful to contribute to the final value of code-review" really mean?

            Does this solution mean that we can have one Jenkins job setting only the Verified label to +1 or -1 and another Jenkins job setting only Code-review label to +1 or -1 for the same patch-set in Gerrit ?
            If the answer is yes, then I'm happy with this solution and would like a fix for it A.S.A.P.

            BR Harald.

            Show
            hluhr Harald Luhr added a comment - Hi, Sorry, for the delayed response. Not sure if I understand the "way forward #1"...sorry for my limited English. What does "Alter the behavior to not allow jobs that have not overridden code review on Build Successful to contribute to the final value of code-review" really mean? Does this solution mean that we can have one Jenkins job setting only the Verified label to +1 or -1 and another Jenkins job setting only Code-review label to +1 or -1 for the same patch-set in Gerrit ? If the answer is yes, then I'm happy with this solution and would like a fix for it A.S.A.P. BR Harald.
            Hide
            scoheb Scott Hebert added a comment -

            The answer is yes. I have updated the description for Way Forward #1 to clarify.

            Show
            scoheb Scott Hebert added a comment - The answer is yes. I have updated the description for Way Forward #1 to clarify.
            Hide
            scm_issue_link SCM/JIRA link daemon added a comment -

            Code changed in jenkins
            User: Scott Hebert
            Path:
            src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/config/Config.java
            src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/config/IGerritHudsonTriggerConfig.java
            src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java
            src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/job/rest/BuildCompletedRestCommandJob.java
            src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/index.jelly
            src/main/webapp/help-GerritBuildFailedCodeReview.html
            src/main/webapp/help-GerritBuildFailedVerified.html
            src/main/webapp/help-GerritBuildNotBuiltCodeReview.html
            src/main/webapp/help-GerritBuildNotBuiltVerified.html
            src/main/webapp/help-GerritBuildStartedCodeReview.html
            src/main/webapp/help-GerritBuildStartedVerified.html
            src/main/webapp/help-GerritBuildSuccessfulCodeReview.html
            src/main/webapp/help-GerritBuildSuccessfulVerified.html
            src/main/webapp/help-GerritBuildUnstableCodeReview.html
            src/main/webapp/help-GerritBuildUnstableVerified.html
            src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/config/ConfigTest.java
            src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderParameterizedTest.java
            src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderSkipVoteParameterTest.java
            src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java
            src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/mock/MockGerritHudsonTriggerConfig.java
            src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/mock/Setup.java
            src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/utils/MockPluginCheckerConfig.java
            http://jenkins-ci.org/commit/gerrit-trigger-plugin/3a5273ee681ad93be9f77b038b77d6523a3be9fb
            Log:
            Allow override of code-review/verified value from job

            Currently, it is NOT possible to have one Jenkins job setting only the Verified label to +1 or -1
            and another Jenkins job setting only code-review label to +1 or -1 for the same patch-set in Gerrit.

            This change will now allow:

            • Setting a code-review or verified value to empty for the Server default value
            • Setting a job specific value for a code-review or verified value
            • Only the jobs that have overridden the value in their configuration will contribute to the code-review or verified value.

            JENKINS-30367 JENKINS-30393

            Change-Id: I08d5b77fc55b49ae2d07a4eb97bbff80aa87b46b

            Show
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Scott Hebert Path: src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/config/Config.java src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/config/IGerritHudsonTriggerConfig.java src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpander.java src/main/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/job/rest/BuildCompletedRestCommandJob.java src/main/resources/com/sonyericsson/hudson/plugins/gerrit/trigger/GerritServer/index.jelly src/main/webapp/help-GerritBuildFailedCodeReview.html src/main/webapp/help-GerritBuildFailedVerified.html src/main/webapp/help-GerritBuildNotBuiltCodeReview.html src/main/webapp/help-GerritBuildNotBuiltVerified.html src/main/webapp/help-GerritBuildStartedCodeReview.html src/main/webapp/help-GerritBuildStartedVerified.html src/main/webapp/help-GerritBuildSuccessfulCodeReview.html src/main/webapp/help-GerritBuildSuccessfulVerified.html src/main/webapp/help-GerritBuildUnstableCodeReview.html src/main/webapp/help-GerritBuildUnstableVerified.html src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/config/ConfigTest.java src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderParameterizedTest.java src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderSkipVoteParameterTest.java src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/gerritnotifier/ParameterExpanderTest.java src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/mock/MockGerritHudsonTriggerConfig.java src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/mock/Setup.java src/test/java/com/sonyericsson/hudson/plugins/gerrit/trigger/utils/MockPluginCheckerConfig.java http://jenkins-ci.org/commit/gerrit-trigger-plugin/3a5273ee681ad93be9f77b038b77d6523a3be9fb Log: Allow override of code-review/verified value from job Currently, it is NOT possible to have one Jenkins job setting only the Verified label to +1 or -1 and another Jenkins job setting only code-review label to +1 or -1 for the same patch-set in Gerrit. This change will now allow: Setting a code-review or verified value to empty for the Server default value Setting a job specific value for a code-review or verified value Only the jobs that have overridden the value in their configuration will contribute to the code-review or verified value. JENKINS-30367 JENKINS-30393 Change-Id: I08d5b77fc55b49ae2d07a4eb97bbff80aa87b46b
            Hide
            scoheb Scott Hebert added a comment - - edited

            Will be present in 2.17.0

            Show
            scoheb Scott Hebert added a comment - - edited Will be present in 2.17.0

              People

              • Assignee:
                scoheb Scott Hebert
                Reporter:
                scoheb Scott Hebert
              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: