Dear Gerrit community,
during the hackathon last week, I started working on a new plugin, which implements the new Checks API [1] for Jenkins-based CI systems. I uploaded a first POC that works with the GerritCodeReview plugin for Jenkins at [2]. The POC currently works as follows:
- A project admin can configure multiple Jenkins instances and 1…n jobs in Jenkins for each instance on a project level.
- The plugin UI will send a request to get the data of the child, job that the Multibranch pipeline using the GerritCodeReview plugin created for the currently viewed patchset. This is done using the Remote Access API provided by Jenkins.
- The plugin UI will further get the build details of all builds of the found jobs and will use the retrieved information to create the CheckRun object that will be used by the Checks API to display the information of the checks.
- The data retrieved from Jenkins is not stored within Gerrit. Except for the project configuration everything runs in the client browser.
I still see quite a lot of shortcomings to the approach I took so far and also already had some discussions with Sven about it on the Kanban board used during the hackathon. Since that board is not accessible for everyone and I think that discussion could be interesting for others as well, e.g. people working on similar plugins for other CI systems, I will bring the discussion to the mailing list. A copy of the comments so far (My response to Svens last comment will be inlined as usual):
Sven Selberg
Today 09:49
I feel like static bookkeeping of all jobs that should be triggered for a project by having to add them to a config isn't really scale:able, and is complicated for non-Gerrit/Jenkins admins to comprehend.
I was considering a different approach, when I started to work on this, where the jenkins-plugins reports "build-started" to the plugin that keeps track of ongoing build in a change meta-ref.
We would need a new "build-started" API that needs to be implemented for the Gerrit and Jenkins plugins.
The FE would then query the backend for started jobs and with those URL:s it would in turn query the Jenkins'es for build status.
Jenkins (gerrit-trigger or gerrit-code-review plugin) knows which builds are triggered for a particular patch-set and is currently reporting started builds to Gerrit. If we let Jenkins continue to keep track of this and report back to Gerrit through the new "build-started" API we don't need to concern ourselves with constructing plugin specific URLs to Jenkins builds and we treat all Jenkins plugins that report to Gerrit in the same manner. Every triggered Jenkins job would automagically turn up in the checks tab of the change-screen.
WDYT
Sven Selberg
Today 09:53
If the CI reports not only the URL but also the service type like
{
"url":"https://my-ci.company.com/job/SOME-ID/job/...",
"type":"JENKINS"
}
You could use the same plugin for a (potentially) all CI systems.
Thomas Dräbing
Today 13:19
I agree full heartedly that the current solution is far from perfect. I see the following issues:
CORS has to be enabled (adds complexity) and how to handle CIs, which do not allow anonymous read access?
Gerrit requires knowledge about how multiple Jenkins plugins work
If the job/build is deleted in the CI, Gerrit will not display any information anymore, making it look, like there was never a check run.
Your proposal tackles point 2), since it wouldn't matter, what is managing the job, but that leaves 1) (your proposal would still require requests being sent from the Gerrit UI to Jenkins) and 3).
My first idea for 2) was to create another Jenkins plugin that exposes a REST API that allows to query the jobs for builds for a patchset (using the lucene-search plugin). This plugin could then know about how to find jobs created by either plugin. This would also reduce the number of required requests. It however also does not solve 1) and 3).
Continuing on your idea, we could store even more data in a changes meta-ref, like build result, result summary etc. Then we could fetch all the data from the Gerrit backend instead of Jenkins directly and it would be permanently stored. I see two issues with that: a) This is more or less what the checks-plugin does (Should it then rather be revived?); b) I don't think this was the intended design, looking at the provided examples (e.g. the buildbucket plugin of the chromium project, which I used a guideline).
Sven Selberg
Today 14:11
In my draft I was planning on proxy:ing the calls to Jenkins through the backend plugin (I never got that far before something else started burning) which would take care of the CORS issue (#1) and keep the authentication (when necessary) in the backend.
Basically:
FE: Hey BE which builds were triggered by this patch-set?
BE: These ones ...
IIUC my suggestion is quite far from the checks plugin as, with the checks-plugin, the Jenkins server needed to know about the checks concept in Gerrit. If a Jenkins job was triggered it would need to check if this job already was configured as a checker in Gerrit and if it wasn't it needed to configure itself, which I feel is closer to the current suggestion. This is one of the major issues with the checks-plugin that made $DAY_JOB opt-out from adopting it. (if you created a new job in Jenkins you would also need to configure Gerrit for each job).
Yes, I remember the discussion now. So instead of defining a checker in Gerrit, there should just be a REST endpoint protected by some project-scoped capability that allows Jenkins to feed Gerrit some data about the build for a change (status, result, etc.; Basically, it would use something similar to the CheckRun interface provided by the checks API). The data would be stored in the NoteDB and fetched by the plugin UI from there instead of Jenkins.
With my suggestion all Jenkins would need to know is how to tell Gerrit which job(s) was triggered by a particular patch-set and Gerrit would figure out the rest.
As you eluded to it will (most likely) be necessary to keep more information than just url and type (like build-results etc.) in the Gerrit backend, but this is true for any solution including the current suggestion.
It would not necessarily be required to store the data in Gerrit as long as the CI does a good job of doing so itself. If I understand the Buildbucket-plugin of the Chromium project correctly they also do not store anything in Gerrit but use another system to store the results. However, this is IMO not feasible for general purpose Cis like Jenkins, where every team with its own Jenkins using a Gerrit would have to set up such a result archive in a way that the checks-jenkins plugin could still retrieve the results to display it in the UI.
However, if a lot of similar plugins (e.g. for zuul, CircleCI etc) would also start storing data in NoteDB, there should be some agreement/contract about the format to avoid a mess within the database.
That being said, I agree that this sounds like a better solution and while the plugin could likely adopt quite a bit of code from the checks plugin, e.g. how to add the check results to NoteDB, it will be different enough to be a separate plugin.
I would also be very interested in the opinion of Ben, who wrote the design document and likely has thought the most about how such design questions should be resolved.
Thanks and best regards,
Thomas
[1] https://gerrit-review.googlesource.com/Documentation/pg-plugin-checks-api.html
[2] https://github.com/thomasdraebing/gerrit-plugins-checks-jenkins
That being said, I agree that this sounds like a better solution and while the plugin could likely adopt quite a bit of code from the checks plugin, e.g. how to add the check results to NoteDB, it will be different enough to be a separate plugin.
I would also be very interested in the opinion of Ben, who wrote the design document and likely has thought the most about how such design questions should be resolved.
--
--
To unsubscribe, email repo-discuss...@googlegroups.com
More info at http://groups.google.com/group/repo-discuss?hl=en
---
You received this message because you are subscribed to the Google Groups "Repo and Gerrit Discussion" group.
To unsubscribe from this group and stop receiving emails from it, send an email to repo-discuss...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/CAG7bb4DPj%3Db-cqW4aWrrgspCRn_L90T81m2Vs-8hji5ms-CxhA%40mail.gmail.com.
Hi Thomas and Sven,Thanks for writing the prototype and starting the discussion. I am very happy to contribute to both. :-)The fundamental design idea for the new Checks API was to not make Gerrit a database for check results. The reasons for why we think that would be a bad idea are:1. CI systems typically already have a database for their results. We would essentially be duplicating an entire database.2. While the CI is running the results are typically updated every second. Either these updates are reflected into Gerrit every second, which would put a very high write load on Gerrit. Or Gerrit is updated less frequently and then shows stale results.3. Gerrit is a code review system, not a CI system. Just because Gerrit has a NoteDB and a plugin framework does not mean that using it as a general purpose database is a good idea. There is always the alternative of creating a separate database outside of Gerrit.
For these reasons I would strongly recommend not to store check results in Gerrit.If Jenkins deletes build results at some point, then maybe it should be configured to not do so? I assume Jenkins retains results only for a certain period, because storing all results forever is not scalable, but that also applies to Gerrit in the same way. Also, maybe 90 days after submission of a change check results are not so important anymore, and deletion is just fine?
If calling CI from Gerrit is hard, then we should look into how to make that easier. Proxying frontend calls through the backend is an option. Just because calling Gerrit from CI is easier than the other way around should not cause us to duplicate the result database (see above). If need be, an additional proxying system/database between Gerrit and CI could help.
IIUC for Jenkins we need to somehow maintain a mapping between change-number+patchset-number and a list of builders/jobs/urls, so that the jenkins plugins knows what to call at the moment of loading checks in the UI. You could persist such a mapping table anywhere, in Jenkins, in Gerrit, or in a separate database/system. Not sure
what the best solution is. I don't know enough about Gerrit's backends to make a recommendation for that. If a Jenkins backend plugin can receive specific calls and can store this data in its specific ref/footer, then why not? The above arguments (duplication, load, staleness) don't apply here, so I would not be totally against storing such
metadata mappings in Gerrit.
On 19 May 2022, at 13:08, Sven Selberg <sven.s...@axis.com> wrote:
Thank you Ben and Han-Wen for your input.On Thursday, May 19, 2022 at 12:29:00 PM UTC+2 bro...@google.com wrote:Hi Thomas and Sven,Thanks for writing the prototype and starting the discussion. I am very happy to contribute to both. :-)The fundamental design idea for the new Checks API was to not make Gerrit a database for check results. The reasons for why we think that would be a bad idea are:1. CI systems typically already have a database for their results. We would essentially be duplicating an entire database.
2. While the CI is running the results are typically updated every second. Either these updates are reflected into Gerrit every second, which would put a very high write load on Gerrit. Or Gerrit is updated less frequently and then shows stale results.3. Gerrit is a code review system, not a CI system. Just because Gerrit has a NoteDB and a plugin framework does not mean that using it as a general purpose database is a good idea. There is always the alternative of creating a separate database outside of Gerrit.
What I think the Gerrit Jenkins-checks plugin needs to hold a copy of are basically build URLs and some sort of Severity (for submit-requirement calculation), and for completed builds what corresponds to CheckRun.status and CheckResult.category.1. If there's a completed run this should affect submitability even if CI system is unreachable.
2. I don't think it's feasible to keep Jenkins builds for eternity unless you happen to have unlimited storage.
And when they are deleted how do we keep track of which builds are failed/succeeded?
We have no historic record of whether tests passed or failed for already submitted changes.
If a change isn't submitted before the build is deleted it is probably for the best to trigger the build again but what is worse is that a change could become submitable once a failed build is deleted since we don't have any record of that that build needs to pass.The CI-system that keeps track of which builds should be started for a particular patch-set is f.i. the Jenkins gerrit-trigger plugin.
This plugin also keeps track of the severity of the build, i.e. should the patch-set be submit:able if the build fails.Both these things are configured in a gerrit-trigger configuration, for each job in Jenkins (with suitable defaults)gerrit-trigger is also responsible for reporting started builds back to Gerrit.
Current workflow in Jenkins with gerrit-trigger============================1. A patch-set-created event reaches the jenkins gerrit-trigger plugin.2. gerrit-trigger triggers the builds that are configured to be triggered by the patch-set.3. gerrit-trigger reports back to Gerrit, with a change-comment, that the build(s) is/are started.4. On completion the gerrit-trigger reports the result back to Gerrit according to the severity-configuration for each completed build.Currently done by setting the Verified label.This means that build-started and build-result is currently stored in gerrit in change-comments and in label-votes.
Possible future workflow with gerrit-trigger==================================1. A patch-set-created event reaches the jenkins gerrit-trigger plugin.2. gerrit-trigger triggers the builds that are configured to be triggered by the patch-set.3. gerrit-trigger reports these build-urls together with severity back to the back-end plugin.These would be stored by the gerrit-backend-plugin.
4. On completion gerrit-trigger reports the build completion to the back-end plugin.This would be stored by the gerrit-backend-plugin, we should delete these if a new build is started of the same job.
Future workflow gerrit-code-review plugin (not totally sure that this is the only use-case but)====================================================================The Jenkinsfile has code for reporting builds started, severity and build result to the gerrit-jenkins-check backend plugin.
This enables the jenkins-checks backend gerrit-plugin to:
* Block submission if there are builds that are configured to block submission on fail that aren't successfully completed.* Provide FE plugin with "rich" data as long as the Jenkins build exists.* Provide FE plugin with rudimentary data about completed builds even after they are deleted in Jenkins.* In all other aspects be totally agnostic to what jobs exists on Jenkins, which server they live on, what they are called etc.For these reasons I would strongly recommend not to store check results in Gerrit.If Jenkins deletes build results at some point, then maybe it should be configured to not do so? I assume Jenkins retains results only for a certain period, because storing all results forever is not scalable, but that also applies to Gerrit in the same way. Also, maybe 90 days after submission of a change check results are not so important anymore, and deletion is just fine?It's not feasible to keep builds forever. You probably want to be able to discern that changes weren't submitted without proper verification even if they were submitted a while back.
If calling CI from Gerrit is hard, then we should look into how to make that easier. Proxying frontend calls through the backend is an option. Just because calling Gerrit from CI is easier than the other way around should not cause us to duplicate the result database (see above). If need be, an additional proxying system/database between Gerrit and CI could help.The data I propose to store is already stored in NoteDb (se above).
I'm not married to the idea that the data should be stored in NoteDb, it could theoretically be some other DB, although for simplicity I think it wouldn't be such a bad idea to keep it in NoteDb since it's part of the submitability data.IIUC for Jenkins we need to somehow maintain a mapping between change-number+patchset-number and a list of builders/jobs/urls, so that the jenkins plugins knows what to call at the moment of loading checks in the UI. You could persist such a mapping table anywhere, in Jenkins, in Gerrit, or in a separate database/system. Not surewhat the best solution is. I don't know enough about Gerrit's backends to make a recommendation for that. If a Jenkins backend plugin can receive specific calls and can store this data in its specific ref/footer, then why not? The above arguments (duplication, load, staleness) don't apply here, so I would not be totally against storing suchmetadata mappings in Gerrit.I think we should let Jenkins tell the gerrit-plugin all these things. Keeping a mapping of project~job in Gerrit is what made the checks plugin basically unfeasible to us, since it meant you needed to configure your jobs twice, once in Jenkins and once in Gerrit.Jenkins builds are already configured to be triggered on a particular patchset (gerrit-trigger) or, IIUC, the builds are instantiated/triggered by a Jenkinsfile from the patch-set (gerrit-code-review?) with basically the same result, the Jenkinsfile that triggers the builds has a record of which patch-set triggered the Jenkinsfile and should be able to report build-URLs back to Gerrit in a similar fashion as how it reports build-started today.
Han-Wen: PostReviewInput and voting might be too late, because Jenkins would typically only vote once all jobs are complete, but the Checks UI would want to show (intermediate) results before that happens.Happy to discuss further. Feel also free to invite me to a meeting.
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/ed6ef6b1-a1ec-4018-ad16-2d3deaedae0cn%40googlegroups.com.
On 19 May 2022, at 13:08, Sven Selberg <sven.s...@axis.com> wrote:
Thank you Ben and Han-Wen for your input.On Thursday, May 19, 2022 at 12:29:00 PM UTC+2 bro...@google.com wrote:Hi Thomas and Sven,Thanks for writing the prototype and starting the discussion. I am very happy to contribute to both. :-)The fundamental design idea for the new Checks API was to not make Gerrit a database for check results. The reasons for why we think that would be a bad idea are:1. CI systems typically already have a database for their results. We would essentially be duplicating an entire database.+12. While the CI is running the results are typically updated every second. Either these updates are reflected into Gerrit every second, which would put a very high write load on Gerrit. Or Gerrit is updated less frequently and then shows stale results.3. Gerrit is a code review system, not a CI system. Just because Gerrit has a NoteDB and a plugin framework does not mean that using it as a general purpose database is a good idea. There is always the alternative of creating a separate database outside of Gerrit.Agreed, Gerrit should have a “view” on Jenkins but must not be the “long-term storage” of the CI results.What I think the Gerrit Jenkins-checks plugin needs to hold a copy of are basically build URLs and some sort of Severity (for submit-requirement calculation), and for completed builds what corresponds to CheckRun.status and CheckResult.category.1. If there's a completed run this should affect submitability even if CI system is unreachable.We should go back to when Jenkins was just giving a score: the Checks plugin experiment did not succeed in replacing it IMHO.
(e.g. we never enabled the constraint on checks on gerrit-review)
2. I don't think it's feasible to keep Jenkins builds for eternity unless you happen to have unlimited storage.
I actually presented at the Jenkins User Summit how to do it, the talk was “Jenkins forever”, remember?
And when they are deleted how do we keep track of which builds are failed/succeeded?
We have no historic record of whether tests passed or failed for already submitted changes.We should use Gerrit labels for that.
If a change isn't submitted before the build is deleted it is probably for the best to trigger the build again but what is worse is that a change could become submitable once a failed build is deleted since we don't have any record of that that build needs to pass.The CI-system that keeps track of which builds should be started for a particular patch-set is f.i. the Jenkins gerrit-trigger plugin.
This plugin also keeps track of the severity of the build, i.e. should the patch-set be submit:able if the build fails.Both these things are configured in a gerrit-trigger configuration, for each job in Jenkins (with suitable defaults)gerrit-trigger is also responsible for reporting started builds back to Gerrit.Or the Jenkins pipeline, as we do with Gerrit’s build on Jenkins.
Current workflow in Jenkins with gerrit-trigger============================1. A patch-set-created event reaches the jenkins gerrit-trigger plugin.2. gerrit-trigger triggers the builds that are configured to be triggered by the patch-set.3. gerrit-trigger reports back to Gerrit, with a change-comment, that the build(s) is/are started.4. On completion the gerrit-trigger reports the result back to Gerrit according to the severity-configuration for each completed build.Currently done by setting the Verified label.This means that build-started and build-result is currently stored in gerrit in change-comments and in label-votes.Yep, that works and will continue to work.
Possible future workflow with gerrit-trigger==================================1. A patch-set-created event reaches the jenkins gerrit-trigger plugin.2. gerrit-trigger triggers the builds that are configured to be triggered by the patch-set.3. gerrit-trigger reports these build-urls together with severity back to the back-end plugin.These would be stored by the gerrit-backend-plugin.
Are you rebuilding the checks plugin then? I would avoid to go that path again.
4. On completion gerrit-trigger reports the build completion to the back-end plugin.This would be stored by the gerrit-backend-plugin, we should delete these if a new build is started of the same job.I disagree, for the reasons that Ben reported.
Future workflow gerrit-code-review plugin (not totally sure that this is the only use-case but)====================================================================The Jenkinsfile has code for reporting builds started, severity and build result to the gerrit-jenkins-check backend plugin.No, I believe Gerrit should call the Jenkins plugin to get the status. We could have of course some caching on Gerrit, but NOT long-term storage.
If we need to record the result for submittability, let’s use the labels.
On 19 May 2022, at 13:08, Sven Selberg <sven.s...@axis.com> wrote:
Thank you Ben and Han-Wen for your input.
On Thursday, May 19, 2022 at 12:29:00 PM UTC+2 bro...@google.com wrote:Hi Thomas and Sven,Thanks for writing the prototype and starting the discussion. I am very happy to contribute to both. :-)The fundamental design idea for the new Checks API was to not make Gerrit a database for check results. The reasons for why we think that would be a bad idea are:1. CI systems typically already have a database for their results. We would essentially be duplicating an entire database.+12. While the CI is running the results are typically updated every second. Either these updates are reflected into Gerrit every second, which would put a very high write load on Gerrit. Or Gerrit is updated less frequently and then shows stale results.
3. Gerrit is a code review system, not a CI system. Just because Gerrit has a NoteDB and a plugin framework does not mean that using it as a general purpose database is a good idea. There is always the alternative of creating a separate database outside of Gerrit.Agreed, Gerrit should have a “view” on Jenkins but must not be the “long-term storage” of the CI results.What I think the Gerrit Jenkins-checks plugin needs to hold a copy of are basically build URLs and some sort of Severity (for submit-requirement calculation), and for completed builds what corresponds to CheckRun.status and CheckResult.category.1. If there's a completed run this should affect submitability even if CI system is unreachable.We should go back to when Jenkins was just giving a score: the Checks plugin experiment did not succeed in replacing it IMHO.(e.g. we never enabled the constraint on checks on gerrit-review)2. I don't think it's feasible to keep Jenkins builds for eternity unless you happen to have unlimited storage.I actually presented at the Jenkins User Summit how to do it, the talk was “Jenkins forever”, remember?
And when they are deleted how do we keep track of which builds are failed/succeeded?
We have no historic record of whether tests passed or failed for already submitted changes.We should use Gerrit labels for that.
If a change isn't submitted before the build is deleted it is probably for the best to trigger the build again but what is worse is that a change could become submitable once a failed build is deleted since we don't have any record of that that build needs to pass.The CI-system that keeps track of which builds should be started for a particular patch-set is f.i. the Jenkins gerrit-trigger plugin.
This plugin also keeps track of the severity of the build, i.e. should the patch-set be submit:able if the build fails.Both these things are configured in a gerrit-trigger configuration, for each job in Jenkins (with suitable defaults)gerrit-trigger is also responsible for reporting started builds back to Gerrit.Or the Jenkins pipeline, as we do with Gerrit’s build on Jenkins.Current workflow in Jenkins with gerrit-trigger============================1. A patch-set-created event reaches the jenkins gerrit-trigger plugin.2. gerrit-trigger triggers the builds that are configured to be triggered by the patch-set.3. gerrit-trigger reports back to Gerrit, with a change-comment, that the build(s) is/are started.4. On completion the gerrit-trigger reports the result back to Gerrit according to the severity-configuration for each completed build.Currently done by setting the Verified label.This means that build-started and build-result is currently stored in gerrit in change-comments and in label-votes.Yep, that works and will continue to work.Possible future workflow with gerrit-trigger==================================1. A patch-set-created event reaches the jenkins gerrit-trigger plugin.2. gerrit-trigger triggers the builds that are configured to be triggered by the patch-set.3. gerrit-trigger reports these build-urls together with severity back to the back-end plugin.These would be stored by the gerrit-backend-plugin.Are you rebuilding the checks plugin then? I would avoid to go that path again.
Han-Wen: PostReviewInput and voting might be too late, because Jenkins would typically only vote once all jobs are complete, but the Checks UI would want to show (intermediate) results before that happens.Happy to discuss further. Feel also free to invite me to a meeting.Yes, that would make sense.Any availability next week?
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/10836290-02BD-4334-879B-D5800CCD2688%40gmail.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/fe191f34-7dd6-4431-b214-8bd213708013n%40googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/2FC704F9-40D5-4AE9-B6A3-E3F720046943%40gmail.com.
What I think the Gerrit Jenkins-checks plugin needs to hold a copy of are basically build URLs and some sort of Severity (for submit-requirement calculation), and for completed builds what corresponds to CheckRun.status and CheckResult.category.1. If there's a completed run this should affect submitability even if CI system is unreachable.We should go back to when Jenkins was just giving a score: the Checks plugin experiment did not succeed in replacing it IMHO.(e.g. we never enabled the constraint on checks on gerrit-review)I could see a solution where the plugin adds a label for each started build and adds a submit-requirement so that all labels for blocking builds need to be OK before a change is submitted.
For status we would need to store the build URLs for ongoing, and failed builds, but we could delete that data once the change is merged or abandoned and only keep the labels for historical book-keeping.But we are in agreement that: _those labels would still be stored in NoteDB_?
On 19 May 2022, at 15:41, Ben Rohlfs <bro...@google.com> wrote:I would be available Mon-Wed 08:00-14:00 CEST.I think I am very much inline with what Luca said. The label votes are the source of truth, even after build results in Jenkins are not available anymore. Thus I still think that we can avoid storing build results in Gerrit completely: 1. Along with the label vote you could put a brief summary into the change message. 2. If the build URL does not return anything anymore, then the plugin can still produce an "INFO" check run with a message like "detailed build results not available anymore, see change message for a summary". So users should not be left too confused.I *think* submit requirements should only be based on labels and other information that is really quick to get (indexed). I also don't think that we can change submit requirements or label configs on the fly. So the easiest solution would be to have one "Jenkins-Verified" label and one submit requirement that checks that label.
On 19 May 2022, at 15:41, Ben Rohlfs <bro...@google.com> wrote:I would be available Mon-Wed 08:00-14:00 CEST.
I think I am very much inline with what Luca said. The label votes are the source of truth, even after build results in Jenkins are not available anymore. Thus I still think that we can avoid storing build results in Gerrit completely: 1. Along with the label vote you could put a brief summary into the change message. 2. If the build URL does not return anything anymore, then the plugin can still produce an "INFO" check run with a message like "detailed build results not available anymore, see change message for a summary". So users should not be left too confused.I *think* submit requirements should only be based on labels and other information that is really quick to get (indexed). I also don't think that we can change submit requirements or label configs on the fly. So the easiest solution would be to have one "Jenkins-Verified" label and one submit requirement that checks that label.+1, that is a general rule of thumb for making Gerrit reliable: *NEVER EVER* base some key functionality (e.g. submittability) on external service API.We’ve got rid of ReviewDb after many years, we don’t want an external point of failure of Gerrit.NoteDb is already very flexible in hosting lots of data, let’s use what is already out there before reinventing something new externally.The Checks API are IMHO a “way to give a view to the CI status” rather than defining yet another submittability mechanism.Luca.
1) As a user …
4) As a Jenkins admin …
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/F348546E-A66F-44A1-A7B6-7D0AC3C741BB%40gmail.com.
On Thu, 19 May 2022 at 16:53, Luca Milanesio <luca.mi...@gmail.com> wrote:On 19 May 2022, at 15:41, Ben Rohlfs <bro...@google.com> wrote:I would be available Mon-Wed 08:00-14:00 CEST.The times work for me as well on Tuesday and Wednesday.I think I am very much inline with what Luca said. The label votes are the source of truth, even after build results in Jenkins are not available anymore. Thus I still think that we can avoid storing build results in Gerrit completely: 1. Along with the label vote you could put a brief summary into the change message. 2. If the build URL does not return anything anymore, then the plugin can still produce an "INFO" check run with a message like "detailed build results not available anymore, see change message for a summary". So users should not be left too confused.I *think* submit requirements should only be based on labels and other information that is really quick to get (indexed). I also don't think that we can change submit requirements or label configs on the fly. So the easiest solution would be to have one "Jenkins-Verified" label and one submit requirement that checks that label.+1, that is a general rule of thumb for making Gerrit reliable: *NEVER EVER* base some key functionality (e.g. submittability) on external service API.We’ve got rid of ReviewDb after many years, we don’t want an external point of failure of Gerrit.NoteDb is already very flexible in hosting lots of data, let’s use what is already out there before reinventing something new externally.The Checks API are IMHO a “way to give a view to the CI status” rather than defining yet another submittability mechanism.Luca.I think there is a misunderstanding of what I suggest /want from the plugin and if I understand him correctly what Sven suggests.
There was never the suggestion to add a new submittability criterion.
For a plugin that should just serve a single CI (Jenkins) that would not make sense and of course for the reasons you mentioned. Let me reiterate the features that the plugin should have in my opinion in a more structured way here:1) As a user …
- … I want to see all the information the Checks API is able to display
- ... I want to be able to see that a build has run for a patchset, even if the build was deleted in Jenkins. This should be visible where I usually look for this information, i.e. the checks tab
- ... I want that the existing label system remains the way how Jenkins communicates whether a change is submittable (Verified+1 or similar)
2) As a project admin ...
- ... I don't want to have to do any configuration or at least as little as possible
- ... I want to be able to use Jenkins servers, which don't allow anonymous read access
- ... I don't want to have to store credentials (required to access Jenkins) in an unsecure way (e.g. plain text in refs/meta/config)
4) As a Gerrit admin ...
- ... I want to support not only central Jenkins instances, but development team out Jenkins instances as well
- ... I don't want to have to do the configuration for every team that owns a Jenkins server (this should be a self service)
- ... I don't want a ton more refs that will cause performance issues
4) As a Jenkins admin …
1. ... I don't want to do extensive changes to Jenkins to be able to provide this information to Gerrit, ideally only a plugin update is required2. ... I don't want to store large amounts of old data, in case Gerrit asks for it, if somebody opens an old change3. ... I don't want to maintain an external database to provide condensed data about builds for GerritWhile this list is likely not exhaustive it should capture the main points discussed so far, I hope.For 1.2 I like Ben's suggestion that the plugin could provide data that informs the user that the builds are not accessible anymore as a compromise, since this clearly differentiates the shown content from what is shown if there is no Jenkins job for a given project. However, we would have to find a way how the plugin differentiates between the cases a) the build was not yet created (if polling is used that can be the case for minutes) b) there is an error in retrieving the build data, but the data exists and d) the data was deleted from the CI. This does not work without storing some data in Gerrit or in Jenkins.I agree with Sven on 2.1 and that my POC does not match this point. While I think that at least for Jenkins servers without anonymous access and while using a fetch-based approach some configuration (credentials) will be unavoidable, this should be kept at a minimum.
Sven suggested that Jenkins should inform Gerrit about when a build was started
and provide it with an URL to use to fetch the check run data. This would require some kind of storage in Gerrit. I think caches would not suffice, since a change and with it the respective validation builds might be open for weeks/months.
One alternative idea that I had was to add an API to Jenkins via a plugin that can be used by the Gerrit trigger plugin and the GerritCodeReview plugin that allows to query for builds working on a given patchset. It could utilize the lucene-search plugin for Jenkins with some additional properties. That way Gerrit could more effectively query for changes in Jenkins. However, that would still require that the Jenkins server URL is configured in Gerrit. It would however at least remove the need to configure which jobs are used for checks for a given project, as in my POC.
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/6dd46a74-055d-43c0-acd4-f326d8333760n%40googlegroups.com.
Luca, Sven, Thomas: Shall we meet at Tue 10:00 CEST?.
On 20 May 2022, at 11:38, Sven Selberg <sven.s...@axis.com> wrote:On Friday, May 20, 2022 at 12:33:19 PM UTC+2 bro...@google.com wrote:Luca, Sven, Thomas: Shall we meet at Tue 10:00 CEST?.Works for me.
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/f0282835-e168-4bde-b068-237fd10e6e88n%40googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/1CE7595E-561C-410C-8F36-C961319EFA32%40gmail.com.
On 2 Jun 2022, at 22:59, Thomas Dräbing <thomas....@gmail.com> wrote:As we discussed at the end of the meeting, I would propose to create a project for the plugin on gerrit-review. I would suggest to name it 'plugins/checks-jenkins' as I did with my POC. That way, if other implementations of the checks API are added, these are easily searchable and grouped when sorting alphabetically. It also fits the scheme of the its-* plugins, which do it in the same way.As soon as the project exists, I would push the current POC as a change for review.As the discussion shows, my POC is not perfect, but with some smaller improvements it should already provide the required functionality that would be required for gerrit-review and gerrit-ci. Thus, it might allow us to get rid of the checks-plugin on gerrit-review already.Ben said that he would also work on the plugin and would be happy to provide the frontend implementation. I am currently pretty busy with other topics, but I will also set aside some time to work on this, especially on the mentioned REST API for Jenkins to make jobs queryable for changes that they are verifying.Please let me know if you are OK with creating the project.
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/CAG7bb4Ay2Q%3DQgOLinv%3DWRFT8WpjqHrtZYAS0UARhhNg-ZD-x6A%40mail.gmail.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/CAG7bb4Ay2Q%3DQgOLinv%3DWRFT8WpjqHrtZYAS0UARhhNg-ZD-x6A%40mail.gmail.com.
On 12 Sep 2022, at 09:30, Ben Rohlfs <bro...@google.com> wrote:Hi Thomas and Luca,I would like to start working on getting gerrit-review onto the new checks-jenkins plugin.As a starting point it would be great, if this change could be submitted:
https://gerrit-review.googlesource.com/c/plugins/checks-jenkins/+/339134: Initial implementation of Checks API for JenkinsThen I can take it from there. My goal would just be to get the same features as we currently have on gerrit-review. That would enable us to retire the old "checks" plugin completely.
Thanks for your help!
On 12 Sep 2022, at 09:30, Ben Rohlfs <bro...@google.com> wrote:Hi Thomas and Luca,I would like to start working on getting gerrit-review onto the new checks-jenkins plugin.As a starting point it would be great, if this change could be submitted:
https://gerrit-review.googlesource.com/c/plugins/checks-jenkins/+/339134: Initial implementation of Checks API for JenkinsThen I can take it from there. My goal would just be to get the same features as we currently have on gerrit-review. That would enable us to retire the old "checks" plugin completely.
Yes, that would be awesome !Thanks for working on that !Thanks for your help!I’ll try to allocate more time to help actively to get it done.
Ben, Luca,I'm interested in this story, as I've been considering creating a Checks API implementation for Teamcity for my organization.Some questions inline below.On Monday, September 12, 2022 at 3:41:47 PM UTC-5 lucamilanesio wrote:On 12 Sep 2022, at 09:30, Ben Rohlfs <bro...@google.com> wrote:Hi Thomas and Luca,I would like to start working on getting gerrit-review onto the new checks-jenkins plugin.As a starting point it would be great, if this change could be submitted:
https://gerrit-review.googlesource.com/c/plugins/checks-jenkins/+/339134: Initial implementation of Checks API for JenkinsThen I can take it from there. My goal would just be to get the same features as we currently have on gerrit-review. That would enable us to retire the old "checks" plugin completely.Are you planning on keeping the store of what Checkers exist for a project within Gerrit in the NoteDB? From a cursory glance at the Chromium Buildbucket Plugin, it looks like they store that information external to Gerrit, unlike the old Checks plugin. (I could be mistaken)I don't love this, from a UI standpoint - I'd rather have it like the old Checks plugin where we can configure what Checkers need to be run for a given project. That said, I wish it were in a more easily readable format like the gitconfig format (maybe set up similar to the replication plugin? With a link to the external build that runs the Checker?), rather than in NodeDB.
Yes, that would be awesome !Thanks for working on that !Thanks for your help!I’ll try to allocate more time to help actively to get it done.Depending on what your timeline is, I might wait until the Jenkins implementation is mature, then fork it with a different connector. Would you recommend I do that or start now with my own implementation? I would imagine that we shouldn't *need* a custom Checks plugin for each CI tool, but should be able to design a standard plugin with a drop-in Driver replacement for different CI tools?
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/add41e0e-484c-4c6b-a411-4f3a174b2016n%40googlegroups.com.
Hi Thomas, Luca and Chris,Thanks for helping me resolve the CORS issues. I have managed to run the plugin successfully and have started importing this into the googlesource.com Gerrit instances.If we use the plugin as is, then we will get *one* check result per change by inspecting the `result` property of `Gerrit-verifier-pipeline`. That would work, but it would be a degraded experience for Gerrit developers: At the moment the verifier pipeline is broken down into its child builds, and the Checks UI displays one chip per child build. These chips then also have a direct link to the log of the relevant build instead of just a link to the pipeline log.I don't see a generic way of solving this issue without investing into a Jenkins plugin, which is out of scope for me.We could special case the situation of the gerrit repo by fetching the `consoleText` of the pipeline job ([1] is an example). Then we can do regex matching for `Starting building: Gerrit-([a-zA-Z-_]+) #([0-9]+)` and thus find the relevant child jobs, fetch them and display their results, resulting in a similar UX to what we currently have.
I see 3 options for how I can continue:1. Stop working on checks-jenkins and wait for gerrit-review to switch to another CI system. Or for someone to write a Jenkins plugin.
On 28 Sep 2022, at 13:06, Chris Poucet <pou...@google.com> wrote:Hi Ben,On Wed, Sep 28, 2022 at 12:10 PM Ben Rohlfs <bro...@google.com> wrote:Hi Thomas, Luca and Chris,Thanks for helping me resolve the CORS issues. I have managed to run the plugin successfully and have started importing this into the googlesource.com Gerrit instances.If we use the plugin as is, then we will get *one* check result per change by inspecting the `result` property of `Gerrit-verifier-pipeline`. That would work, but it would be a degraded experience for Gerrit developers: At the moment the verifier pipeline is broken down into its child builds, and the Checks UI displays one chip per child build. These chips then also have a direct link to the log of the relevant build instead of just a link to the pipeline log.
I don't see a generic way of solving this issue without investing into a Jenkins plugin, which is out of scope for me.
We could special case the situation of the gerrit repo by fetching the `consoleText` of the pipeline job ([1] is an example). Then we can do regex matching for `Starting building: Gerrit-([a-zA-Z-_]+) #([0-9]+)` and thus find the relevant child jobs, fetch them and display their results, resulting in a similar UX to what we currently have.This feels extremely hacky.
I see 3 options for how I can continue:1. Stop working on checks-jenkins and wait for gerrit-review to switch to another CI system. Or for someone to write a Jenkins plugin.
With our move to LUCI, I believe this to be the most effective move.2. Add the special casing to checks-jenkins and for the time being label that plugin as "example implementation". Other users would be recommended to fork.
3. Add the special casing to a Google internal plugin, which I would fork from the current state of checks-jenkins.
Let me know, if you have a preference for an option, or if you have alternative ideas. Thanks!
I don't see a generic way of solving this issue without investing into a Jenkins plugin, which is out of scope for me.The issue here is the goal: do we want to write a plugin that allows to display the status of the change verification?If yes, we should display the status of the jobs as-is:- get the list of jobs associated with this change- display the status of them
It *may* look different from the current checks feedback, but that’s fine.
I see 3 options for how I can continue:1. Stop working on checks-jenkins and wait for gerrit-review to switch to another CI system. Or for someone to write a Jenkins plugin.Well, it depends how soon the LUCI integration will come. Also consider that not ALL builds will go to LUCI, we still have the problem of rendering the verification of the other non-core plugins.
3. Add the special casing to a Google internal plugin, which I would fork from the current state of checks-jenkins.What about my suggestion above? Why not just accepting that the user-experience is going to be different?At the end of the day, you don’t care about the Jenkins pipeline which is pure orchestration but the results of the associated jobs, isn’t it?
Hi Luca, thanks for your input. Replies inline.I don't see a generic way of solving this issue without investing into a Jenkins plugin, which is out of scope for me.The issue here is the goal: do we want to write a plugin that allows to display the status of the change verification?If yes, we should display the status of the jobs as-is:- get the list of jobs associated with this change- display the status of themThe problem is that I don't know how to get the "list of jobs associated with this change". At the moment the only job we know about is the pipeline. How do you define "associated"? Is Gerrit-codestyle associated with a change or not?It *may* look different from the current checks feedback, but that’s fine."Different" is fine, e.g. if the chips have different names, but showing just one "pipeline" chip instead of two (one for tests and one for codestyle) would be a substantially worse UX for Gerrit frontend developers at least. It is very helpful to know whether tests have failed or whether codestyle is the issue, and to have each console log just one click away.I see 3 options for how I can continue:1. Stop working on checks-jenkins and wait for gerrit-review to switch to another CI system. Or for someone to write a Jenkins plugin.Well, it depends how soon the LUCI integration will come. Also consider that not ALL builds will go to LUCI, we still have the problem of rendering the verification of the other non-core plugins.LUCI integration may take 1-2 quarters. I am currently mostly worried about core and plugins that we own. It would be nice, if other plugins are also covered, but that is not our main goal.3. Add the special casing to a Google internal plugin, which I would fork from the current state of checks-jenkins.What about my suggestion above? Why not just accepting that the user-experience is going to be different?At the end of the day, you don’t care about the Jenkins pipeline which is pure orchestration but the results of the associated jobs, isn’t it?Yes. How do I get at associated jobs?There is an clear rule for how to get from change number 346794 to this URL:But how can I get from change number 346794 to this URL without regex matching the console text?To be clear, I am happy to write a Gerrit frontend plugin using TypeScript for the integration of Jenkins into the Checks UI/API, but my scope will not extend to changing anything on the Jenkins side. And I will not make calls to Jenkins from a Gerrit backend plugin. At the moment, I think this is just not feasible to do with the existing Jenkins APIs.Thanks, Ben
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/340d102a-9863-421f-9d52-0041e36201a5n%40googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/2f971ed4-7bde-4fac-ae40-e54f9dbed9cen%40googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/CAEWS%2BaOtC6eXPRwxdOEw85NiDX00PiiZTX-o7GXknnKb55K%2Bmw%40mail.gmail.com.
On 11 Oct 2022, at 09:03, Thomas Dräbing <thomas....@gmail.com> wrote:I have created a first POC for a Jenkins-plugin that allows to get information on jobs working on a Gerrit patchset: https://github.com/thomasdraebing/gerritchangequery-plugin.You can just send a request like this: curl -L http://jenkins.example.com/jenkins/gerrit/check-runs?change=12345&patchset=6The plugin searches for such builds using brute force, so it is definitely not production ready, but should provide a starting point to adapt Gerrit's checks-jenkins plugin and to start developing the plugins on both sides in parallel.I will continue on this during the hackathon in November.
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/CAG7bb4BqJwTNPdZR7UCLbxgYxhKH0wpU0sMg8mLt%3D1U3wHn46Q%40mail.gmail.com.
Hi, I thought I would share what I've done to integrate Jenkins with Gerrit's checks api, in case people were interested.It's a bit simple in terms of configuration, however as well as pulling in general build information it also,
- Scans the stages via the workflow api to report the exact steps and errors that were reported
- Pulls in warnings and errors via the warnings-ng plugin api (if configured)
- Pulls in any failed tests via the testResult api endpoint.
Hopefully this will be useful as you build up the official checks-jenkins plugin.Nick
To view this discussion on the web visit https://groups.google.com/d/msgid/repo-discuss/c47af427-44c9-437b-ba6c-31972522b592n%40googlegroups.com.