First-class CI integration

411 views
Skip to first unread message

Alice Kober-Sotzek

unread,
Feb 13, 2019, 2:34:42 PM2/13/19
to Repo and Gerrit Discussion
FYI: We are working on adding first-class CI integration in Gerrit. See this link for the full picture. You'll likely see some Gerrit changes for this popping up soon.

Nasser Grainawi

unread,
Feb 13, 2019, 3:13:49 PM2/13/19
to Alice Kober-Sotzek, Repo and Gerrit Discussion
On Feb 13, 2019, at 12:34 PM, 'Alice Kober-Sotzek' via Repo and Gerrit Discussion <repo-d...@googlegroups.com> wrote:

FYI: We are working on adding first-class CI integration in Gerrit. See this link for the full picture. You'll likely see some Gerrit changes for this popping up soon.

Thanks for the heads up. Reading that link, I see a lot of similarities with the task plugin [1].

Did you include in your research an analysis of currently available options for solving the problems you outlined? I'd be very interested to learn how you think the first-class CI support would co-exist with or replace the task plugin.

Nasser



--
--
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.
For more options, visit https://groups.google.com/d/optout.

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, 
a Linux Foundation Collaborative Project

Luca Milanesio

unread,
Feb 13, 2019, 3:57:01 PM2/13/19
to Nasser Grainawi, Alice Kober-Sotzek, Luca Milanesio, Repo and Gerrit Discussion

On 13 Feb 2019, at 20:13, Nasser Grainawi <nas...@codeaurora.org> wrote:


On Feb 13, 2019, at 12:34 PM, 'Alice Kober-Sotzek' via Repo and Gerrit Discussion <repo-d...@googlegroups.com> wrote:

FYI: We are working on adding first-class CI integration in Gerrit. See this link for the full picture. You'll likely see some Gerrit changes for this popping up soon.

Thanks for the heads up. Reading that link, I see a lot of similarities with the task plugin [1].

Did you include in your research an analysis of currently available options for solving the problems you outlined? I'd be very interested to learn how you think the first-class CI support would co-exist with or replace the task plugin.

Oh yes, some consolidation would be *super useful* and more productive for everyone :-)

@Alice why not creating a markdown document in the design folder of the Gerrit project and accepting reviews? GerritForge is maintaining a native integration with Jenkins (see https://github.com/jenkinsci/gerrit-code-review-plugin) and we would be more than happy to contribute to the initiative, we can put resources into it also.

Luca.

Sven Selberg

unread,
Feb 14, 2019, 5:05:56 AM2/14/19
to Repo and Gerrit Discussion
Thanks for the design document Alice.

1. According to the design document it looks like this is all meant to be implemented in core Gerrit. In the spirit of limiting core Gerrit's responsibilities I would strongly suggest to move all of it into a plugin and only create 1-2 extension points in Gerrit.

2. Looking at the design Gerrit "needs to know" a lot about the surrounding CI system, who is meant to run which test/check etc., this model seems fragile to me and it also goes against limiting Gerrit's responsibilities and the effort to modularize tasks that lies outside of those core responsibilities.
Core Gerrit doesn't know how-to; replicate git projects to other servers, supply hooks and webhooks, delete a project, set access rights for a single user etc. But with this change it would more or less take on the responsibility of job-scheduler for the CI system.

I think it's a great idea to introduce the "check/checker" concept in Gerrit but I think Gerrit should only be concerned with:
* Are there checks that needs to be performed for this change/ps?
* Are any of those checks blocking submission?
* What are the statuses of those checks?

Apart from these three main questions there should be some data that Gerrit is not concerned with but might be valuable to the user like:
* How many checks will be performed?
* What are they checking?
* How are they progressing?
* Where are these checks run?

Infrastructure
Communication between entities might be best done through a messaging service.
The way I picture this whole thing to come together is with these components:

Gerrit:
    * Check if there are checks that needs to be run before created change/ps is submitable.
    * Checking the state of said checks.
    * Expose data from checks through REST API (f.i. as a property in ChangeInfo, as suggested in the design document)
    * Responsible for storing data for checks.

CI-Plugin:
    * Keeps track of which endpoint to ask for checks.
    * Asks this endpoint for any tests that should be run before a created change/ps can be submitted.
    * Relay the meta-data and state for these checks back to core Gerrit

Router/Scheduler:
    * This could be implemented by another plugin but most likely an external service.
    * Keeps track of which types of jobs/tests should be run for a set of triggers (project, filetypes, branch... up to the scheduler).
    * OPTIONAL: Also keeps track of which jobs/tests are available. (F.i. this change needs for pylint, pep8 etc to be run but you might also start an integration job or similar and this could potentially be started from Gerrit UI if the information is available).
    * Implements the Scheduler-API so that the CI-Plugin can ask for/trigger jobs through the Scheduler.

Build Service:
    *  Just responsible for building what the Router/Scheduler says and report back to the scheduler or directly to Gerrit depending on what is chosen by each individual set-up.

UI
Not clear to me if PolyGerrit or Ci-Plugin should be responsible for the UI parts (displaying status of checks, restart checks etc.).
The CI-Plugin might be a better fit since you could tailor the UI to the specific CI-System that handles the checks.

WORKFLOW
A workflow could look like this.

Change/ps created.

1 [Gerrit]: Are there any implementations of the CI-Plugin?
2. If so, ask this/these implementations for checks asynchronous and block submission until replied.
3. [CI-Plugin]: Ask for checks from Scheduler.
4. [Scheduler]: Reply with a set of checks.
5a. [Scheduler] Schedule said checks in build system and report back status of checks to CI-Plugin as they are updated (started, running, done(success|failure), step 2/9 complete etc.)
5b [CI-Plugin]: Populate the emtpy place-holder collection of checks with the checks supplied by scheduler.
6 [Gerrit]: block submission until all checks that are blocking have finished successfully.

Alternative:
3. [CI-Plugin]: Project not configured for checks. Scheduler has no configured checks.
4. [Gerrit]: Un-block submission.

On Wednesday, February 13, 2019 at 9:57:01 PM UTC+1, lucamilanesio wrote:

On 13 Feb 2019, at 20:13, Nasser Grainawi <nas...@codeaurora.org> wrote:


On Feb 13, 2019, at 12:34 PM, 'Alice Kober-Sotzek' via Repo and Gerrit Discussion <repo-d...@googlegroups.com> wrote:

FYI: We are working on adding first-class CI integration in Gerrit. See this link for the full picture. You'll likely see some Gerrit changes for this popping up soon.

Thanks for the heads up. Reading that link, I see a lot of similarities with the task plugin [1].

Did you include in your research an analysis of currently available options for solving the problems you outlined? I'd be very interested to learn how you think the first-class CI support would co-exist with or replace the task plugin.

Oh yes, some consolidation would be *super useful* and more productive for everyone :-)

@Alice why not creating a markdown document in the design folder of the Gerrit project and accepting reviews? GerritForge is maintaining a native integration with Jenkins (see https://github.com/jenkinsci/gerrit-code-review-plugin) and we would be more than happy to contribute to the initiative, we can put resources into it also.

Luca.


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.
For more options, visit https://groups.google.com/d/optout.

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, 
a Linux Foundation Collaborative Project

Han-Wen Nienhuys

unread,
Feb 14, 2019, 6:12:04 AM2/14/19
to Sven Selberg, Repo and Gerrit Discussion
How would your approach work with the changes dashboard? The dashboard is powered by the index, and the index is not extensible by plugins.

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.
For more options, visit https://groups.google.com/d/optout.


--

Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich

Registergericht und -nummer: Hamburg, HRB 86891

Sitz der Gesellschaft: Hamburg

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado

Sven Selberg

unread,
Feb 14, 2019, 6:51:13 AM2/14/19
to Repo and Gerrit Discussion
I don't think I quite understand "work with the changes dashboard", but assuming you mean visualize check status in dashboard:
If Gerrit understand "checks" it should be able to index them when they are available (for a change), in the same way that Gerrit understands reviewer and review-label and can index those if they are available (for a change).

Han-Wen Nienhuys

unread,
Feb 14, 2019, 10:31:48 AM2/14/19
to Sven Selberg, Repo and Gerrit Discussion
On Thu, Feb 14, 2019 at 11:06 AM Sven Selberg <sven.s...@axis.com> wrote:
Thanks for the design document Alice.

1. According to the design document it looks like this is all meant to be implemented in core Gerrit. In the spirit of limiting core Gerrit's responsibilities I would strongly suggest to move all of it into a plugin and only create 1-2 extension points in Gerrit.


But how many plugins would we have?  Do you want a plugin per CI system (one for Jenkins, one for Buildkite, one for Travis, etc.)? Or the same idea as in this design, but then running in plugin?
  
2. Looking at the design Gerrit "needs to know" a lot about the surrounding CI system, who is meant to run which test/check etc., this model seems fragile to me and it also goes against limiting Gerrit's responsibilities and the effort to modularize tasks that lies outside of those core responsibilities.
Core Gerrit doesn't know how-to; replicate git projects to other servers, supply hooks and webhooks, delete a project, set access rights for a single user etc. But with this change it would more or less take on the responsibility of job-scheduler for the CI system.

I think it's a great idea to introduce the "check/checker" concept in Gerrit but I think Gerrit should only be concerned with:
* Are there checks that needs to be performed for this change/ps?
* Are any of those checks blocking submission?
* What are the statuses of those checks?

Apart from these three main questions there should be some data that Gerrit is not concerned with but might be valuable to the user like:
* How many checks will be performed?
* What are they checking?
* How are they progressing?
* Where are these checks run?

Infrastructure
Communication between entities might be best done through a messaging service.
The way I picture this whole thing to come together is with these components:

Gerrit:
    * Check if there are checks that needs to be run before created change/ps is submitable.
    * Checking the state of said checks.
    * Expose data from checks through REST API (f.i. as a property in ChangeInfo, as suggested in the design document)
    * Responsible for storing data for checks.

CI-Plugin:
    * Keeps track of which endpoint to ask for checks.
    * Asks this endpoint for any tests that should be run before a created change/ps can be submitted.
    * Relay the meta-data and state for these checks back to core Gerrit

I don't understand what the CI-plugins should do. Do you mean that it should contact (say) a Jenkins instance, and that it is specific to a CI system?

What if the CI system is down, can we then still display the Change screen? Can we still submit changes?

If we have to build a plugin for each CI system separately, that means that many users of Gerrit cannot use the CI system of their choice until somebody (us?) creates plugin for that CI system and deploys it on the Gerrit instance. This approach does not scale well in the number of different CI systems. 

(apologies if I am misunderstanding the suggestion.)

We run a pretty big deployment of Gerrit at Google, and it's been our experience that plugins are a source of hassle: if they are contributed, we still have to vet their code; if they crash, we are the only ones able to debug them. It's a model that doesn't scale well when the number of customers/host admins increases. This is why are taking a direction that emphasizes extensibility through well thought out, stable core APIs over plugins.
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.
For more options, visit https://groups.google.com/d/optout.


--

Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich

Dave Borowitz

unread,
Feb 14, 2019, 10:50:41 AM2/14/19
to Sven Selberg, Repo and Gerrit Discussion
I think there is a strong argument to be made for introducing this functionality in core. It is absolutely true that we as a community are resistant to scope creep within core Gerrit. However, that doesn't mean that we should never introduce large new features--it just means that we need to carefully consider the requirements and scope up front, so we don't spend a lot of effort building something that's only useful to a small subset of the community.

I like to think of Gerrit core as serving the same role in the plugin ecosystem as Java does in the ecosystem of JVM languages. Gerrit, like Java, is pretty conservative in what it accepts into core (at least in terms of workflow changes, I'm not going to comment on config options:). Through our plugin framework, we provide a platform for developers to build and experiment. It's our role as maintainers to keep an eye on the ecosystem so we can learn what is important to users. There is a whole subset of plugins that deal with integrating with different systems, and I don't think that's going to go away; we're never going to say that you have to use Gitiles or JIRA or what have you. But that leaves a still large set of plugins for adding additional workflow. I don't think it's necessarily a good thing to make it so that the workflow of Gerrit can only grow by adding a plugin. When we find substantial commonalities between plugins for the same basic use cases in different parts of the community, we have to ask ourselves: could we build something in core that satisfies this need? In our Java analogy, the rise in popularity of functional languages on the JVM directly led to the core Java team to add JVM features (invokedynamic) and language features (lambdas) to core, thus bringing those functional paradigms to a broader audience.

That's more of a general philosophy; I want to also talk about why I think building CI functionality specifically into Gerrit core is good for users and good for developers. One of the most important reasons in my mind is to work towards unifying the UX across different Gerrit installations. Looking at the various servers at Google and externally, the #1 biggest difference in UX/UI (other than perhaps version skew) is caused by how those servers integrate with their CI system of choice. This means that even if someone has experience with "Gerrit", they still have a big learning curve when coming to a new Gerrit server (and context-switching cost if they use multiple servers regularly): they have a completely different UI for understanding what needs to be done before they can submit their change. I think that by building a single, relatively simple UI and submit rule framework, we can make Gerrit simpler and easier to use, while still having equivalently powerful CI system to a hand-rolled implementation.

Beyond end-user UX, I truly believe that moving some functionality to core will make life easier for authors of plugins and of external CI systems. It is tempting to wedge increasingly-complex UI elements into Gerrit, but let's face it, practically every CI system already has a web UI of its own, and most likely it's written by different people in a different language/framework. Those people are going to put their best work into their standalone web UI, and maintaining feature parity with the plugin-provided sub-UI in Gerrit is an ongoing burden. On top of that, the CI system authors now need to worry about the build and deployment process of two separate web apps. What we are saying with the CI integration in Gerrit is: CI authors, you worry about your UI. We will surface just the pieces of the CI workflow (e.g. check state, robot comments) that are useful in the context of code review, and we will build a UI that solves that problem. If your UI needs to solve additional problems, great: we will make it as easy as possible to link out to your UI.

Finally, I want to call out what a great job Alice has done on investigating the ecosystem while building this design. Beyond the open-source CI integrations, she met with pretty much every team she could find at Google doing anything in the CI or static analysis space so she could understand the commonalities in data model and workflow, and I think the design reflects that. I am pretty confident that any external CI system will be able to integrate with Gerrit using this design. I will let her speak to any further specifics about the design and why it works this way as opposed to some other way.


To unsubscribe, email repo-discuss...@googlegroups.com

Martin Fick

unread,
Feb 14, 2019, 12:10:33 PM2/14/19
to repo-d...@googlegroups.com, Dave Borowitz, Sven Selberg
On Thursday, February 14, 2019 7:50:24 AM MST 'Dave Borowitz' via Repo and
Gerrit Discussion wrote:
> When we find substantial commonalities between plugins for the same basic
> use cases in different parts of the community,

Is this the case? Can you provide examples?

Pick the most common, enhance it and make it a core plugin? This would provide
an incremental approach.


> That's more of a general philosophy; I want to also talk about why I think
> building CI functionality specifically into Gerrit core is good for users
> and good for developers. One of the most important reasons in my mind is to
> work towards unifying the UX across different Gerrit installations. Looking
> at the various servers at Google and externally,

I would stipulate that is what core plugins are for?

-Martin

--
The Qualcomm Innovation Center, Inc. is a member of Code
Aurora Forum, hosted by The Linux Foundation

Martin Fick

unread,
Feb 14, 2019, 12:19:54 PM2/14/19
to repo-d...@googlegroups.com, Han-Wen Nienhuys, Sven Selberg
On Thursday, February 14, 2019 12:11:46 PM MST 'Han-Wen Nienhuys' via Repo and
Gerrit Discussion wrote:
> How would your approach work with the changes dashboard? The dashboard is
> powered by the index, and the index is not extensible by plugins.

Enhancing Gerrit to make this possible has been discussed at several
hackathons. This would be super valuable for Gerrit as it would allow plugins
to do all sorts of useful things, not just the immediate goal. I would hope
that we start moving more towards enhancing Gerrit to be able to work better
with plugins instead of opting to put more and more things in core just
because there currently isn't an extension for plugins to do something. This
does require a bit more foresight in the development of extensions, but
ultimately should lead to a more useful result, and help limits feature creep
in core,

Nasser Grainawi

unread,
Feb 14, 2019, 5:44:13 PM2/14/19
to Alice Kober-Sotzek, Repo and Gerrit Discussion
On Feb 13, 2019, at 12:34 PM, 'Alice Kober-Sotzek' via Repo and Gerrit Discussion <repo-d...@googlegroups.com> wrote:

FYI: We are working on adding first-class CI integration in Gerrit. See this link for the full picture. You'll likely see some Gerrit changes for this popping up soon.


Doesn't feel very open source or collaborative. I expect better in the Gerrit community.

To be clear, I'm all for improving Gerrit's CI integration ability. I've spent years doing it. Those improvements can start with 1 change that solves a problem or makes something better. Then we work together as a community to solve the next problem. I think many in the community would prefer that approach to shared problem spaces like this.

Nasser


--
--
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.
For more options, visit https://groups.google.com/d/optout.

lucamilanesio

unread,
Feb 14, 2019, 6:14:29 PM2/14/19
to Repo and Gerrit Discussion


On Wednesday, February 13, 2019 at 8:57:01 PM UTC, lucamilanesio wrote:


On 13 Feb 2019, at 20:13, Nasser Grainawi <nas...@codeaurora.org> wrote:


On Feb 13, 2019, at 12:34 PM, 'Alice Kober-Sotzek' via Repo and Gerrit Discussion <repo-d...@googlegroups.com> wrote:

FYI: We are working on adding first-class CI integration in Gerrit. See this link for the full picture. You'll likely see some Gerrit changes for this popping up soon.

Thanks for the heads up. Reading that link, I see a lot of similarities with the task plugin [1].

Did you include in your research an analysis of currently available options for solving the problems you outlined? I'd be very interested to learn how you think the first-class CI support would co-exist with or replace the task plugin.

Oh yes, some consolidation would be *super useful* and more productive for everyone :-)

@Alice why not creating a markdown document in the design folder of the Gerrit project and accepting reviews? GerritForge is maintaining a native integration with Jenkins (see https://github.com/jenkinsci/gerrit-code-review-plugin) and we would be more than happy to contribute to the initiative, we can put resources into it also.

Re-iterating again, maybe my message was lost in the thread.

*Can we have* a proper review in Gerrit of the design?

1. Export the document into a ci-integration-design.md under /Documentation
2. Push to gerrit-review.googlesource.com as a Change in [RFC]
3. Have a review and amendments based on what the community needs about a CI integration

and then when we have agreed a "common ground", work together towards a shared solution?

There is a lot of code that has been written here and there, as plugins or scripts.
It would be a huge waste to NOT reuse the experience, code, and know-how of the people that have been using it for years.

We all need to show that Gerrit Code Review is not only a tool, but a way of collaborating and working together, based on common exchange and review of ideas and code.

Hope to see the 1. 2. and 3. points becoming reality soon.

And thanks again for sharing the document, looking forward to contributing to it in terms of ideas, code, work :-)

Luca.
 

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+unsubscribe@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

-- 
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, 
a Linux Foundation Collaborative Project


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+unsubscribe@googlegroups.com.

Dave Borowitz

unread,
Feb 14, 2019, 6:21:35 PM2/14/19
to Nasser Grainawi, Alice Kober-Sotzek, Repo and Gerrit Discussion
On Thu, Feb 14, 2019 at 2:44 PM Nasser Grainawi <nas...@codeaurora.org> wrote:

On Feb 13, 2019, at 12:34 PM, 'Alice Kober-Sotzek' via Repo and Gerrit Discussion <repo-d...@googlegroups.com> wrote:

FYI: We are working on adding first-class CI integration in Gerrit. See this link for the full picture. You'll likely see some Gerrit changes for this popping up soon.


Please don't read too much into the fact that we did a bit of prototyping a few days before releasing the design doc. Look at the author date on PS1 of the first change in Edwin's series: we wrote the first line of code in this project literally 2 weeks ago. The only reason that we didn't start pushing code for non-private review immediately was to give us time to polish the complete design doc. It's not because we're trying to keep the open source community out of the loop, in fact it's quite the opposite: the reason we synchronized the release of the code and the design doc is because we don't want people to be surprised by this new code. We want to share what we're working on with the community, and we would be doing the community a disservice if we just started throwing code over the wall before communicating the broader plans.

If you want an example of what I feel was an insufficient amount of communication with the community about future plans, look no further than my very first NoteDb commit. Someone not familiar with Google's plans would look at that and say: "Huh? Why is there a 'git-notes-based database'? Who thought that was a good idea? When is this gonna ship?" In retrospect, for this and other reasons, I really wish we had specced out the NoteDb design and roadmap quite a bit more than we did, and shared more with the community sooner. What I'm hoping for from this process is more of exactly the technical discussions we're having here.

Doesn't feel very open source or collaborative. I expect better in the Gerrit community.

We absolutely want to collaborate with the community on this project. We have a good idea of what the roadmap is going to look like here, but it's not set in stone, and everything we do is going to be subject to discussion and refinement. We just wanted to think through some of our ideas and prototype in a smaller group before broadening the discussion. But like I said, the total amount of time we've spent talking amongst ourselves without sharing is really not very long.
 
To be clear, I'm all for improving Gerrit's CI integration ability. I've spent years doing it. Those improvements can start with 1 change that solves a problem or makes something better. Then we work together as a community to solve the next problem. I think many in the community would prefer that approach to shared problem spaces like this.

With respect, pushing out small independent changes to solve one minor problem at a time is an approach that works well for some features and doesn't work well for others. Some features are bigger and call for more detailed planning so that we can make sure we are building the right pieces to put together into a coherent whole. CI integration very much falls in the latter category.

Patrick Hiesel

unread,
Feb 15, 2019, 10:32:03 AM2/15/19
to Repo and Gerrit Discussion, Dave Borowitz, Nasser Grainawi, Alice Kober-Sotzek

Hi all,


When I joined the Gerrit project three years ago, I was proud to work on a truly open-source code review system that is not only on-par with other offerings, but a thought leader in various areas. Today, I still am.


Gerrit has features that other systems implemented only recently or that they just don’t have. GitHub just announced draft pull requests. Gerrit has two concepts for that for about two years and we had drafts for a very long time before that. Gerrit supports cross-repo submissions and has the most fine grained per-ref-permission model in the ecosystem. Most other systems don’t even dare to implement that.


But the reality is also that Gerrit as a project is behind on some important recent developments. From where I stand, CI/CD is the biggest of those. If you look at other systems, they all offer first class CI integration and the users expect no less than that. I had the pleasure to watch a talk at FOSDEM earlier this month that showed how to integrate GitLab with 26 different cloud providers using OpenStack to run tests and a deployment. This is the state of the art. Users expect their code review system to integrate into their DevOps stack seamlessly as part of it's core functionality and this is where we have to deliver as a project.


Learning is not a one-way street and the other projects are obviously learning from us, so I firmly believe we should learn from them as well. This doesn’t mean that we are copying anyone, but it really shouldn’t stop us from adopting best-practices.


I don’t want to get too deep into the technical parts of the discussion of doing this as a 1st party vs plugin here. But let me say one thing: The single most frequent argument I hear from people switching off of Gerrit to other systems is that using Gerrit is not ‘seamless’ it comes with friction from configuration and integration overhead. I think we have the opportunity to improve on that point with CI/CD and we should use it.


Thanks for keeping the discussion focussed around how we can serve users best.


Patrick


Alice Kober-Sotzek

unread,
Feb 15, 2019, 11:35:56 AM2/15/19
to Repo and Gerrit Discussion, Dave Borowitz, Nasser Grainawi, Patrick Hiesel
Because of our combined efforts, Gerrit is a great code review tool and I think that we all have the intention of improving it even further. There are some aspects which are not ideal yet and for which our users deserve to get a better experience. CI integration (as well as analyzer integration) is one of them. We can certainly continue with the way of just providing label support in core and having first-class CI integration in a plugin but I don’t think that this is the right way for Gerrit in the future.

Core Gerrit should be a platform which serves the basic needs for a code review tool out of the box and provides a consistent user experience across hosts. CI integration might not have been a basic need in the past but considering other code review tools, the amount of custom plugin solutions for this, and the changing expectations of users regarding good integrations between systems, I think the time has come to include CI/analyzer integration into those basic needs.

I am aware that such an integration can look very different for different use cases. Nevertheless, other code review tools have already mastered that challenge and I think we can do so, too. Existing solutions have been a great inspiration to me as well as learning about the needs, use cases, and implicit expectations (e.g. performance, resilience, consistency) of our users. Combined with finding an approach which works for Gerrit and which feels Gerrit-like to me, I came to the design I sent around.

To allow all these varying use cases, the design is deliberately flexible and extensible in many regards, which I hoped is demonstrated by the provided examples of possible future extensions. I’m sure that I didn’t think of all your individual use cases but I believe that extensions should be possible for most of them. There are also some parts (e.g. blocking conditions) which could make sense to transform into an extension point, so that further customizations are possible.

I’m happy to hear feedback from you directly on the design. As asked, I published it as a Gerrit change. Please excuse that I didn’t have the time to create a nicely formatted markdown document.

Would it be helpful to meet virtually next week and directly answer questions and share opinions? What about next Tuesday (Feb 19th) from 7pm to 8pm (UTC+1)?

Nasser Grainawi

unread,
Feb 15, 2019, 5:46:02 PM2/15/19
to Alice Kober-Sotzek, Repo and Gerrit Discussion, Dave Borowitz, Patrick Hiesel
Hi folks,

I think we'll all always find things to improve in Gerrit, at least I have for the past 10 years and I don't expect that to change :-). As you say, user expectations change and we want to stay relevant and useful. I'm hoping to dispel any notion that I think Gerrit's features, extensions, or core functionality should remain static. I do hope we can improve Gerrit in ways that solve specific problems and don't short-change the amazing flexibility Gerrit has. As Patrick noted, Gerrit does things that other systems don't even try for, and I agree that its CI integration should be just as powerful and flexible as the rest.

Meeting at that time next week would be nice. I look forward to talking to you then and I'll try to prime the discussion with some review comments on the uploaded design change (thanks for pushing that).

Alice Kober-Sotzek

unread,
Feb 18, 2019, 10:13:48 AM2/18/19
to Nasser Grainawi, Repo and Gerrit Discussion, Dave Borowitz, Patrick Hiesel
On Fri, Feb 15, 2019 at 11:46 PM Nasser Grainawi <nas...@codeaurora.org> wrote:
Hi folks,

I think we'll all always find things to improve in Gerrit, at least I have for the past 10 years and I don't expect that to change :-). As you say, user expectations change and we want to stay relevant and useful. I'm hoping to dispel any notion that I think Gerrit's features, extensions, or core functionality should remain static. I do hope we can improve Gerrit in ways that solve specific problems and don't short-change the amazing flexibility Gerrit has. As Patrick noted, Gerrit does things that other systems don't even try for, and I agree that its CI integration should be just as powerful and flexible as the rest.

Meeting at that time next week would be nice. I look forward to talking to you then and I'll try to prime the discussion with some review comments on the uploaded design change (thanks for pushing that).
Great. :-)

I set up a meeting and sent you an invite. Could anybody else who would like to join please send me a message so that I can add you too?
For anybody deciding on short notice: You can directly join via https://meet.google.com/gnt-vuaf-dme during the meeting.

Sven Selberg

unread,
Feb 18, 2019, 12:39:20 PM2/18/19
to Repo and Gerrit Discussion
Hi,

Let me clarify my post and my main concerns since it seems to have caused much confusion and disarray.
I have been bogged down from work the past few days and haven't had the chance to respond earlier.

First:
* Great initiative (if anyone doubted that I thought otherwise).
* CI integration, all for it.
* Visualization of CI progress, all for it.


From my reading of the design document my main concerns was the protocol between Gerrit and the CI system that seems to give Gerrit the responsibility of deciding which checks/tests that should be run, in dialog the protocol from the design document looks something like this:

Gerrit: A change was created!
CI-System: What tests checks should I do?
Gerrit: You should do A,B and C.

Which looks to me like you have to configure your CI workflow in Gerrit.
Without trying to be smart about it but I agree with Patricks reflection:
The single most frequent argument I hear from people switching off of Gerrit to other systems is that using Gerrit is not ‘seamless’ it comes with friction from configuration and integration overhead.
We would love to have a tight integration where Gerrit can visualize the progress of ongoing builds, the possibility to restart builds, start more checks, start integration process from the gerrit UI etc.
But, I don't see how it would be sustainable for us to configure all CI checks and tests in Gerrit.
I would much prefer a protocol that looks like:

Gerrit: A change was created!
Gerrit: Hey CI-System, what tests/checks do you need to run before I can let this change be submitted?
CI-something: Let me get back to you on that one...
Gerrit: Ok, I wont let anyone submit the change until I here from you again.
...
CI-something: I need to run A,B and C.
Gerrit: I'll let the users know and hold off the submit until those tests have passed. Here's how I will know them [ A:123, B:456, C:789].

If CI-Something is an plugin extension point you can pretty much write a core plugin that uses the protocol above to talk to the actual CI-System. Or anyone could implement the extension point to integrate with they're CI-System of choice through any API they choose.

But if the protocol is like the one in the design document I'm afraid we (and perhaps many others) wouldn't be able to benefit at all from all the nice visualization of progress and possibility to restart tests, start integration pipeline since it would be impossible to maintain and Developers and QA alike would not be interested in going to Gerrit to configure the CI-Process.

On Monday, February 18, 2019 at 4:13:48 PM UTC+1, Alice Kober-Sotzek wrote:
On Fri, Feb 15, 2019 at 11:46 PM Nasser Grainawi <nas...@codeaurora.org> wrote:
Hi folks,

I think we'll all always find things to improve in Gerrit, at least I have for the past 10 years and I don't expect that to change :-). As you say, user expectations change and we want to stay relevant and useful. I'm hoping to dispel any notion that I think Gerrit's features, extensions, or core functionality should remain static. I do hope we can improve Gerrit in ways that solve specific problems and don't short-change the amazing flexibility Gerrit has. As Patrick noted, Gerrit does things that other systems don't even try for, and I agree that its CI integration should be just as powerful and flexible as the rest.

Meeting at that time next week would be nice. I look forward to talking to you then and I'll try to prime the discussion with some review comments on the uploaded design change (thanks for pushing that).
Great. :-)

I set up a meeting and sent you an invite. Could anybody else who would like to join please send me a message so that I can add you too?
For anybody deciding on short notice: You can directly join via https://meet.google.com/gnt-vuaf-dme during the meeting.

@Alice
Could you send me the time of the meeting?
 

Nasser

Han-Wen Nienhuys

unread,
Feb 18, 2019, 3:18:26 PM2/18/19
to Luca Milanesio, Nasser Grainawi, Alice Kober-Sotzek, Repo and Gerrit Discussion
On Wed, Feb 13, 2019 at 9:57 PM Luca Milanesio <luca.mi...@gmail.com> wrote:
Thanks for the heads up. Reading that link, I see a lot of similarities with the task plugin [1].

Did you include in your research an analysis of currently available options for solving the problems you outlined? I'd be very interested to learn how you think the first-class CI support would co-exist with or replace the task plugin.

Oh yes, some consolidation would be *super useful* and more productive for everyone :-)

@Alice why not creating a markdown document in the design folder of the Gerrit project and accepting reviews? GerritForge is maintaining a native integration with Jenkins (see https://github.com/jenkinsci/gerrit-code-review-plugin) and we would be more than happy to contribute to the initiative, we can put resources into it also.


Hi there,


As further input to the discussion, I would like to present the storage format that we intend to use. It was written by Edwin, who is on holidays, so I’m uploading it for him.


https://gerrit-review.googlesource.com/c/gerrit/+/214745


As you can see we have anticipated scaling up this design to 100s of checks per repository and 10,000s of checks per Gerrit instance, and we have further ideas on how we could support millions of checks.


So we think that this design will be able to address even the most heavily loaded Gerrit instances without performance concerns.

cheers,


Sven Selberg

unread,
Feb 19, 2019, 3:28:02 AM2/19/19
to Repo and Gerrit Discussion
One other thing.
The fact that Patrick brings up Gitlab as a model for SCM - CI integration makes me curious.
Gitlab has great integration with it's own, built-in, CI system whereas you couldn't say the same thing for any other similar service (like Jenkins) last time I had anything to do with it.

Are there plans to have a built-in CI-Service in Gerrit?

/Sven

Alice Kober-Sotzek

unread,
Feb 19, 2019, 8:27:54 AM2/19/19
to Sven Selberg, Repo and Gerrit Discussion
Hi Sven,

The meeting is today from 7pm to 8pm (UTC+1). I also sent you an invite in case you want to come as well.

I'll try to write up an explanation why Gerrit doesn't reach out via a plugin to the CI system but I don't know whether I can finish it before the meeting. Some of your other concerns should be addressable with the described design, though I see that this might not be obvious at first glance. I'll also cover that in the same explanation.

Sven Selberg

unread,
Feb 19, 2019, 10:33:39 AM2/19/19
to Repo and Gerrit Discussion
I got the invite, thanks!

We can probably continue most of the technical discussions in the change you uploaded.

Martin Fick

unread,
Feb 21, 2019, 3:17:46 PM2/21/19
to repo-d...@googlegroups.com, Nasser Grainawi, Alice Kober-Sotzek
I would like to highlight some ideas that we have had in the past for the task
plugin that might be related to the ongoing CI discussion. I want to highlight
them because it does sound like some of these ideas are similar to some of the
improvements the google team wants to make. Common ideas are likely evidence
that they are potentially valuable ideas. I also realize that we have not done
the best job explaining our future(and maybe even past) ideas of where the
task plugin and our (Qualcomm Innovations Center's) CI might go. So sharing
our thoughts might inspire some feedback on them, or inspire similar
approaches in your code/workflows.

To highlight some things from the CI discussions:

"Checker Registering"

There is a desire to see more in Gerrit about which CI systems may be
configured for each change. This is a desire we definitely share with google,
and we currently use the task plugin to do so. While the task plugin is not
ideal at this yet and could be improved in this area, I do believe it is an
appropriate place to do so.

The google proposal mentioned the idea of registering "checkers" via Gerrit
queries. Since the task plugin does not operate on "checkers", but rather it
operates on the "tasks" that "checkers" or other systems and humans need to
perform, it only allows the registering of "tasks" (side note: the task vs.
checker is an abstraction that you might want to consider even though it may
not be immediately obvious to your team now with your current CI needs, this
abstraction embraces the idea of "needs" for changes, regardless of how the
"need" will be accomplished). So even though what we are registering is
different from what google proposes, we do share the idea that it should be
based on queries, and then exposed to users in the UI in Gerrit (we have found
that our users know how to "speak" Gerrit queries, and we have found many ways
to expose all sorts of extra information about changes via queries by using
plugins). This has been powerful for us, and very flexible. We believe users
should be able to know more about what tasks need to be done before a change
is ready without them having to know about various CI configurations.

An existing weakness of our approach of registering "tasks" instead of
"checkers" is that our CI system needs to know how to look for the tasks they
will need to work on. Currently this is done via convention about where a task
is placed in our task hierarchy.

1) We would like to improve the correlation between tasks and "checkers" by
exposing a property of some sort on tasks that would indicate who/what the
"actor" is that needs to perform a task. The "actor" in this case would likely
correlate to "checker" as you have been using the word, but it could also be
something like "developer", or "approver", or something to help automated
systems and humans know who/what is expected to complete a task. This would
thus come closer to the idea of registering "checkers" with changes, by
registering the "checker" to the "task".

We may also consider exposing extra config data to checkers this way (via task
properties), in case a common checker needs to behave differently for
different changes. An example of this might be two different projects that use
the same format checker, but have different indentation preferences (2 vs. 4
spaces). Instead of having to define a new wrapper for the checker which wants
4 instead of 2 spaces, and registering it separately, the checker could read
the "indentation" property from the task to know how many spaces to expect for
the current change.

"Named Destinations"

As the google proposal suggested registering "checkers" based on queries
using projects and branches, I would like to point out an existing, but likely
little known Gerrit feature that might be helpful to those wanting to register
"checkers" or "tasks" using queries. You can do that based on projects and
branches, but also based on predefined sets of these by using "User Named
Destinations"
https://gerrit-review.googlesource.com/Documentation/user-named-destinations.html

2) Another area for improvement would be to create group-named-destinations so
that instead of requiring a user (generally a service account) to store
destinations, we could use Gerrit groups to define sets of related project
branches.

"Checker Status"

In the task plugin, we currently support showing when a task is running by
registering a "running" query with a task. Since we tend to "patch set lock'
changes during "merge" verification: https://gerrit-review.googlesource.com/
Documentation/config-labels.html#_patch_set_lock_example we can query for that
lock label to determine if a merge verification is running or not.

3) We have had requests to show more status about tasks. Some requests would
like us to show a position in the CI system's "queue" before the current
change will run. I envision that this could be achieved by registering some
custom per CI system java code, but this has not been thought through very
much. We are working on ways to register methods in other plugins in the task
config so that they can be called to help provide useful task input from a
custom plugin without having to tie that plugin to the task plugin or vice
versa. I believe this would be an acceptable way of accessing queue position
or other custom CI system specific data from the task plugin. Since we already
have experience making many of our Gerrit plugins work together and yet stay
independent, I am confident that we know how to do this.

"Checker Result Data"

I am not sure that I have a clear picture of all the data that the google team
wants to store related to CI Systems, but I suspect almost everybody wants to
store and make job links easily available to users in the Gerrit UI.

4) I would also like to allow CI system's to register a link to a status
result. However, this is one feature I do think might be worth implementing in
core. It would be great if Gerrit could allow a URL to be registered with an
approval vote. Gerrit could then turn the negative "X"s into a link to a job
that failed, and the positive "V" check marks into links to jobs that passed.
This is a very simple result status approach that I suspect would cover a lot
of CI use cases in Gerrit core.

Lacking (or in addition to) the ability to add links to approvals, it would be
nice to make the task plugin able to display each completed or running task as
a link. This would require the ability to store change specific data
somewhere the plugin can get to. It might be helpful if core Gerrit could
store structured data beyond approvals on a per change basis. This way plugins
could store a link to results in a way that it could then be accessed by the
plugin. Perhaps it would make sense to add support for storing named
properties on a change? Perhaps these properties would be namespaced per
plugin?

Lacking core support for storing properties, it might make sense to use Robo
comments to do so (it is after all robo data). A specially formatted robo
comment could be parsed and interpreted by the task plugin as the data needed
to create this link.

Lastly, as a last alternative, a plugin could manage and store this data
itself.

"Dependencies"

It has occurred to us that tasks are a form of dependency graph, and we have
already taken advantage of this to help manage not only "checkers" and human
approvals (and also things like states in our bug tracking system), but we
have also realized that it can also help manage dependencies to other changes.

One thing that makes this particularly appealing is that we have different
teams with different ideas of how dependencies should work. I have (mostly)
given up on trying to convince others that there is a "right" way to manage
dependencies, and I have been very surprised at some of the creative ideas
others have had in these areas (should it be destined for the same product,
should it already be merged into the current product, should a lower level
library like jgit be able to depend on a higher level change such as a change
to Gerrit? ...). I have come to accept that even change dependency management
policies is something that needs to be customizable.

I have found that often times the boundaries determining what policies to use
for change dependencies tends to be aligned with the CI system merging those
changes. These means that if the task.config file manages the CI system, then
it likely is the ideal place to manage change dependencies policies also. It
would be more inconvenient to manage these policies elsewhere because any
other location would likely require duplicating the boundary rules of which
changes the policies apply to. The task plugin has thus not only become a
place to register tasks, but a place to enforce valid change dependency
policies. Another way to see this is: if a change has invalid dependencies, it
should be blocked and someone has the "task" to fix these dependencies before
the change can be merged (it "needs dependencies fixed").

We are making use of this ability to manage change dependencies in the task
plugin, and we would like to expand the task plugin's utility in managing
them, more specifics on that below...

"Git Change Dependencies"

Currently we are able to use the task plugin to verify that a change's git
dependencies are "valid". By valid, we generally mean destined for the correct
(same) branch, are current (do not have a later patchset). This helps us avoid
merging change's that have invalid git dependencies, and it helps notify users
of this. This is a good example of a task that is a human task, a human needs
to correct this invalid dependency before the change is ready for merging.

5) We would like to expand the use of the task plugin to be able to include
the actual git dependencies and all their tasks as sub tasks on the current
change. This would allow us to not only prevent changes with invalid git
dependencies from being merged, but it would allow us to display to the user
that a change may be blocked because a parent change may not be ready for
merging (perhaps the parent change still needs CR+2, or is not formatted
correctly). The desired improvement is to be able to insert a list of changes,
as defined by a gerrit query, into the task tree of the current change, and
the task plugin would then also insert the tasks for those change returned by
the query into the tree beneath each change it inserted. This requires
improving the task plugin to treat changes as tasks, and it requires creating
an operator that returns a change's git dependencies.

"Non-Git Change Dependencies"

We allow users to specify dependencies on other changes via Gerrit comments.
We have a plugin which can parse these comments and make them available via
Gerrit searches. With the task plugin we are also able to verify various
criteria of these non git dependencies to determine if they are valid or not.
Such criteria might be: "are they destined for the same set of project/
branches as the current change which get verified together?", if not "are they
merged in other project/branches in our repo manifest already?" This later
check is performed using search operators provided by our manifest plugin
which can analyze the content of a manifest and determine whether a change is
merged in the history of one of the project/branch combinations listed in the
manifest (assuming it is on the same Gerrit server).

Suggested improvement #5 should help us here too. Currently our analysis is
done on the set of changes that are dependencies. In the case of invalid
dependencies the user is not currently notified which changes are invalid,
just that the set of dependencies is not fully valid. With improvement #5,
each dependency would get listed as a subtask of the current change. These
changes' subtasks would once again be added to the task tree giving much
greater user visibility into the state of dependencies and what it will take
to merge the current change.

"Submit"

Since our CI systems tend to look at the task plugin to determine if a change
is eligible for verification and then we verifies and merges them using our
batch plugin, we end up bypassing all the Gerrit submit rules. Although we
likely have coded those submit rules into the subtasks anyway, we do run the
risk that we could miss something.

6) One improvement we have considered is to make it possible to add the
existing Gerrit submit rules as sub-tasks automatically somehow, we haven't
figure this out yet. This would allow the task rules to work with Gerrit
submit rules which would be particularly nice if people want to define submit
rules using plugins, and then have them honored by CI systems using the task
and batch plugins.

7) Additionally, we have concluded that it would be nice to be able to mark
certain tasks as submit rules. This would make the task plugin usable without
a CI system whatsoever as a way to define submit rules using queries (a
feature that is oddly missing from Gerrit currently).

The trick of course, is to ensure that #6 and #7 don't interfere with each
other, or loop.

"Time Stats"

Management would like to know which stages in a change's lifespan take the
longest, and thus which areas could benefit most from workflow of process
improvements. Developers would like to know when their changes will likely get
merged.

8) We would like to add time based statistics next to each task to help with
the above desires. This would require a data store that could store data per
change and per task, and a fast way to aggregate that data so that it can be
displayed on every change page. Once again if Gerrit had a way to store per
change/per plugin properties, and if they could additionally make it into the
index to be able to ask questions such as: "what is the average, min, and max
values of that property", that would be super helpful.

Lacking support for such data in Gerrit, the task plugin could create its own
index and store this data.

"Task WUI"

We currently have a task WUI internally, it is GWT based and it has not yet
been ported to polygerrit. It is fairly trivial, it mainly lists all the tasks
which need to be done using the ready-hint or failed-hints. There is also a
list all tasks view (even done ones).

9) We do hope to port this to polygerrit sooner than later. We are reaching a
point in our "eliminate the fork" work, that we might be able to focus on UI
stuff fairly soon (yay!). Naturally we welcome any hints on how to create
something like this from a plugin. We mainly just need a table of line items
returned from the query rest API. Eventually we would like it to look more
like a GANT chart, but a flat table will work for now, and a small step up
would be a tree.


I apologize for the length of the email above, but I thought it might be good
to get some of our visions and ideas on paper for others to see. Maybe it will
inspire other better ideas. And maybe, someone might want to take advantage
of the existing task plugin, or if I am very lucky even enhance it themselves!
:)

Thanks for starting the dialog,

Martin Fick

unread,
Feb 25, 2019, 7:36:58 PM2/25/19
to repo-d...@googlegroups.com, Nasser Grainawi, Alice Kober-Sotzek
On Thursday, February 21, 2019 1:17:38 PM MST Martin Fick wrote:
...
Here is an implementation of what I had in mind for this improvement:

https://gerrit-review.googlesource.com/c/plugins/task/+/215578

Nasser Grainawi

unread,
Mar 1, 2019, 1:42:07 PM3/1/19
to Martin Fick, Repo and Gerrit Discussion, Alice Kober-Sotzek
On Feb 21, 2019, at 1:17 PM, Martin Fick <mf...@codeaurora.org> wrote:

"Task WUI"

We currently have a task WUI internally, it is GWT based and it has not yet
been ported to polygerrit. It is fairly trivial, it mainly lists all the tasks
which need to be done using the ready-hint or failed-hints. There is also a
list all tasks view (even done ones).

9) We do hope to port this to polygerrit sooner than later. We are reaching a
point in our "eliminate the fork" work, that we might be able to focus on UI
stuff fairly soon (yay!). Naturally we welcome any hints on how to create
something like this from a plugin. We mainly just need a table of line items
returned from the query rest API. Eventually we would like it to look more
like a GANT chart, but a flat table will work for now, and a small step up
would be a tree.


I found some time last week to port a simple version of this UI to PolyGerrit. Anyone interested should take a look at https://gerrit-review.googlesource.com/c/plugins/task/+/215672 as it's working and I'll probably merge it early next week.

Nasser


--
The Qualcomm Innovation Center, Inc. is a member of Code
Aurora Forum, hosted by The Linux Foundation

Reply all
Reply to author
Forward
0 new messages