Testing multiple commits in one job with Gerrit
Hi all,
I'm gradually setting up Zuul for my org, and basically I have just one impedance mismatch between Zuul's behaviour and my requirements: Due to pressure on our test hardware I don't think running full end-to-end tests on every individual commit will scale.
What I'd ultimately like is a check/gate setup where jobs in the check pipeline runs on every single commit, but jobs in the gate pipeline only run once for an entire "stack" of commits, and then that entire stack can be merged at once.
IIUC this is basically the behaviour you get already with GitHub, where the zuul notion of "change" maps to a whole Pull Request rather than an individual commit. I started looking into ways something similar can be achieved with Gerrit.
After some comments from #zuul and starting at the code a bit this strategy is the one I'm considering: enable one Zuul change to encapsulate multiple Gerrit changes. (Gerrit and Zuul both use the word "change", which works because Gerrit changes and Zuul changes are currently 1:1. That wouldn't be the caes any more with my proposal so I'll always say "Gerrit change" or "Zuul change").
I haven't figured out exactly how the Gerrit changes should be grouped together, and how this should be configured in the Gerrit trigger, but I think the basic idea would be to inject extra events into the Gerrit event stream (somewhere around GerritEventConnector._handleEvent). One example might be that we detect when an entire stack of Gerrit changes has received some common set of approvals, we inject an event called (say) "zuul-change-stack-approved". The user can then have a pipeline.trigger.gerrit entry with this event, and that trigger would create a Zuul change encapsulating the entire stack of Gerrit changes. These wouldn't need to actually be treated in the code like events coming from Gerrit, but this basic idea would allow expressing this behaviour with the existing structure of the Gerrit trigger configuration.
I think it would then be a matter of finding the places where the Scheduler and pipeline Managers interact with the Gerrit driver using references to Zuul changes, and ensuring that those parts of the Gerrit driver can handling being passed the new class of Zuul change. I think most of these would involve checking for if the Zuul change is of the new type and then iterating over the constituent Gerrit changes to performe some action against each (e.g. GerritConnection.canMerge, GerritRefFilter.matches, and, with a little more complication, GerritReporter.report).
Does that seem like a reasonable approach to the problem?
Cheers, Brendan
On 2018-11-26 13:47:48 +0000 (+0000), Brendan Jackman wrote: [...]
this is basically the behaviour you get already with GitHub, where the zuul notion of "change" maps to a whole Pull Request rather than an individual commit.
[...]
By way of explanation, this design choice was basically driven by a difference in the predominant development workflows between Gerrit and GitHub: to iterate on a change for a repository under Gerrit, the author typically replaces the old commit with a new one and reviewers look at inter-patch diffs from the change history to see what's been altered; by comparison to iterate on a change for a repository under GitHub, the author typically proposes subsequent commits in a lightweight branch which are reviewed as a part of that same change (I expect this is because GitHub lacks any other sort of revision tracking for pull requests).
The usual expectation from GitHub users is that the commits which make up a PR may not themselves be complete and testable changes. Gerrit, on the other hand, doesn't really have a concrete unit of work which aggregates multiple changes since the relationships between changes within a dependent change series can be altered at any time through rebasing. That is to say, Zuul's behavior in aggregating multiple commits for a single GitHub pull request is to a great extent enabled by the difference between GitHub's data model and Gerrit's, so to assume discrete Gerrit changes and individual commits in a GitHub PR are implemented in Zuul as comparable constructs is a significant leap. GitHub merges the set of commits from a pull request, Gerrit merges each change one at a time.
the basic idea would be to inject extra events into the Gerrit event stream (somewhere around GerritEventConnector._handleEvent). One example might be that we detect when an entire stack of Gerrit changes has received some common set of approvals, we inject an event called (say) "zuul-change-stack-approved". The user can then have a pipeline.trigger.gerrit entry with this event, and that trigger would create a Zuul change encapsulating the entire stack of Gerrit changes. These wouldn't need to actually be treated in the code like events coming from Gerrit, but this basic idea would allow expressing this behaviour with the existing structure of the Gerrit trigger configuration.
[...]
If this is a goal, it probably needs to start with figuring out how upstream Gerrit might at least eventually support such a grouping concept (design of cross-project dependencies for Zuul 2.0 resulted in a lot of the same sorts of design discussion, though in the end we settled on metadata in commit messages).
My initial logistical questions are, how does Zuul calculate the boundaries of the set to know when it has "all" the changes expected (can that set be altered over time or does it have to be declared only once completely assembled)? If alterable and open-ended, how should Zuul behave when alteration occurs between approval and merge (abort testing for the full set, test additions as a separate set)? And what should the behavior be if testing for that group of changes fails (report failure to all of them, retest a subset)?
I'm not personally convinced it makes sense as a feature, but it's at least an interesting thought exercise.
On 26/11/2018 21:54, Jeremy Stanley wrote:
On 2018-11-26 13:47:48 +0000 (+0000), Brendan Jackman wrote: [...]
this is basically the behaviour you get already with GitHub, where the zuul notion of "change" maps to a whole Pull Request rather than an individual commit.
[...]
By way of explanation, this design choice was basically driven by a difference in the predominant development workflows between Gerrit and GitHub: to iterate on a change for a repository under Gerrit, the author typically replaces the old commit with a new one and reviewers look at inter-patch diffs from the change history to see what's been altered; by comparison to iterate on a change for a repository under GitHub, the author typically proposes subsequent commits in a lightweight branch which are reviewed as a part of that same change (I expect this is because GitHub lacks any other sort of revision tracking for pull requests).
The usual expectation from GitHub users is that the commits which make up a PR may not themselves be complete and testable changes. Gerrit, on the other hand, doesn't really have a concrete unit of work which aggregates multiple changes since the relationships between changes within a dependent change series can be altered at any time through rebasing. That is to say, Zuul's behavior in aggregating multiple commits for a single GitHub pull request is to a great extent enabled by the difference between GitHub's data model and Gerrit's, so to assume discrete Gerrit changes and individual commits in a GitHub PR are implemented in Zuul as comparable constructs is a significant leap. GitHub merges the set of commits from a pull request, Gerrit merges each change one at a time.
Sure, it definitely makes sense how the differences in the workflows & models led to the differences in behaviour. In fact my org currently uses Gitlab, which in its current incarnation, is basically a Github clone with a basic revision-tracking feature. So I considered just writing a Gitlab driver for Zuul. Maybe that's a more pragmatic choice but Gerrit is (IMO...) a better code review tool than Gitlab, and my test HW availability issues would be a sad reason to be prevented from using it. A way to reduce pressure on test resources seems like something that would be useful to others too so that's why I wanted ot investigate this idea despite it introducing a bit of friction against Gerrit's natural model.
the basic idea would be to inject extra events into the Gerrit event stream (somewhere around GerritEventConnector._handleEvent). One example might be that we detect when an entire stack of Gerrit changes has received some common set of approvals, we inject an event called (say) "zuul-change-stack-approved". The user can then have a pipeline.trigger.gerrit entry with this event, and that trigger would create a Zuul change encapsulating the entire stack of Gerrit changes. These wouldn't need to actually be treated in the code like events coming from Gerrit, but this basic idea would allow expressing this behaviour with the existing structure of the Gerrit trigger configuration.
[...]
If this is a goal, it probably needs to start with figuring out how upstream Gerrit might at least eventually support such a grouping concept (design of cross-project dependencies for Zuul 2.0 resulted in a lot of the same sorts of design discussion, though in the end we settled on metadata in commit messages).
My initial feeling is solving a problem that is outside of Gerrit's problem domain - I don't think it would make sense as a specific feature inside Gerrit. But I have thought about how topics relate to this (see below).
My initial logistical questions are, how does Zuul calculate the boundaries of the set to know when it has "all" the changes expected (can that set be altered over time or does it have to be declared only once completely assembled)? If alterable and open-ended, how should Zuul behave when alteration occurs between approval and merge (abort testing for the full set, test additions as a separate set)?
I haven't though them through in detail but there are 3 ways I considered this could potentially be done:
1. Time-based chunking: if there are multiple proposed commits in a dependency chain and the first Gerrit change gets the required approval, Zuul waits N seconds to see if the second one gets an approval. If it doesn't it tests and potentially merges the first commit. If it does then it resets the timer and waits to see if the next Gerrit change gets the approval. If the timer runs out or we reach a Gerrit change with no child, the whole chain gets tested as one Zuul change and potentially merged. This strategy feels a little wild to me to to configure/explain but it could be quite ergonomic once set up. It also feels a bit like a fundamental change to Zuul's model, just jimmied into the Gerrit driver instead of where it really belongs.
2. You just say if a Gerrit change has a topic matching a regex then every other change with that topic has to get the approvals before testing. Then they all become a single Zuul change. Not sure how this one would work with cross-repo dependencies or if there were no Depends-On but a topic's changes spanned multiple repos. Or if a change has multiple topics matching the regex. Or if it makes any sense for non-linear dependency chains.
3. (This is the one I mainly considered:) In the example I gave above you would just inject the zuul-change-stack-approved event whenever you saw a chain of proposed commits that started with a child of (a commit in) the target branch and ended with a commit with no child, that all have a common approval.
If between approval and merge another Gerrit change was added to that chain and then it also got the approval, another event would be generated and a new Zuul change would enter the pipeline, representing the new entirety of the chaing of Gerrit changes. I guess this would be similar to two Github PRs being approved in quick succession where one contains all the commits of the other, plus an extra commit.
If between approval and merge new patch versions were pushed then approved then I think a new Zuul change could just enter the pipeline. If configured to do so then the old one would be dequeued (in its entirety - it would be a singular Zuul change from the pipeline's PoV), if not it would continue and then just fail to merge at the end. Just like for a singular Gerrit change.
I must admit as I wrote this I started to see some complexity I hadn't' considered wrt. the Change.isUpdateOf logic. I will keep thinking about it.
And what should the behavior be if testing for that group of changes fails (report failure to all of them, retest a subset)?
My assumption with all of these is that if the tests fail you report a failure against every Gerrit change (the tip commits gets a "this test failed gate checks" and the ancestors get a "this test was tested as part of X and failed - if it is useful in its own right, you could resubmit it indepdendently")
I'm not personally convinced it makes sense as a feature, but it's at least an interesting thought exercise.
Thanks for considering it!
- Brendan
[Sorry, my mail client butchered the other reply, let me try to make it readable]
On 26/11/2018 21:54, Jeremy Stanley wrote:
On 2018-11-26 13:47:48 +0000 (+0000), Brendan Jackman wrote: [...]
this is basically the behaviour you get already with GitHub, where the zuul notion of "change" maps to a whole Pull Request rather than an individual commit.
[...]
By way of explanation, this design choice was basically driven by a difference in the predominant development workflows between Gerrit and GitHub: to iterate on a change for a repository under Gerrit, the author typically replaces the old commit with a new one and reviewers look at inter-patch diffs from the change history to see what's been altered; by comparison to iterate on a change for a repository under GitHub, the author typically proposes subsequent commits in a lightweight branch which are reviewed as a part of that same change (I expect this is because GitHub lacks any other sort of revision tracking for pull requests).
The usual expectation from GitHub users is that the commits which make up a PR may not themselves be complete and testable changes. Gerrit, on the other hand, doesn't really have a concrete unit of work which aggregates multiple changes since the relationships between changes within a dependent change series can be altered at any time through rebasing. That is to say, Zuul's behavior in aggregating multiple commits for a single GitHub pull request is to a great extent enabled by the difference between GitHub's data model and Gerrit's, so to assume discrete Gerrit changes and individual commits in a GitHub PR are implemented in Zuul as comparable constructs is a significant leap. GitHub merges the set of commits from a pull request, Gerrit merges each change one at a time.
Sure, it definitely makes sense how the differences in the workflows & models led to the differences in behaviour. In fact my org currently uses Gitlab, which in its current incarnation, is basically a Github clone with a basic revision-tracking feature. So I considered just writing a Gitlab driver for Zuul. Maybe that's a more pragmatic choice but Gerrit is (IMO...) a better code review tool than Gitlab, and my test HW availability issues would be a sad reason to be prevented from using it. A way to reduce pressure on test resources seems like something that would be useful to others too so that's why I wanted to investigate this idea despite it introducing a friction against Gerrit's natural model.
the basic idea would be to inject extra events into the Gerrit event stream (somewhere around GerritEventConnector._handleEvent). One example might be that we detect when an entire stack of Gerrit changes has received some common set of approvals, we inject an event called (say) "zuul-change-stack-approved". The user can then have a pipeline.trigger.gerrit entry with this event, and that trigger would create a Zuul change encapsulating the entire stack of Gerrit changes. These wouldn't need to actually be treated in the code like events coming from Gerrit, but this basic idea would allow expressing this behaviour with the existing structure of the Gerrit trigger configuration.
[...]
If this is a goal, it probably needs to start with figuring out how upstream Gerrit might at least eventually support such a grouping concept (design of cross-project dependencies for Zuul 2.0 resulted in a lot of the same sorts of design discussion, though in the end we settled on metadata in commit messages).
My initial feeling is solving a problem that is outside of Gerrit's problem domain - I don't think it would make sense as a specific feature inside Gerrit. But I have thought about how topics relate to this (see below)
My initial logistical questions are, how does Zuul calculate the boundaries of the set to know when it has "all" the changes expected (can that set be altered over time or does it have to be declared only once completely assembled)? If alterable and open-ended, how should Zuul behave when alteration occurs between approval and merge (abort testing for the full set, test additions as a separate set)?
I haven't though them through in detail but there are 3 ways I considered this could potentially be done:
1. Time-based chunking: if there are multiple proposed commits in a dependency chain and the first Gerrit change gets the required approval, Zuul waits N seconds to see if the second one gets an approval. If it doesn't it tests and potentially merges the first commit. If it does then it resets the timer and waits to see if the next Gerrit change gets the approval. If the timer runs out or we reach a Gerrit change with no child, the whole chain gets tested as one Zuul change and potentially merged. This strategy feels a little wild to me to to configure/explain but it could be quite ergonomic once set up. It also feels a bit like a fundamental change to Zuul's model, just jimmied into the Gerrit driver instead of where it really belongs.
2. You just say if a Gerrit change has a topic matching a regex then every other change with that topic has to get the approvals before testing. Then they all become a single Zuul change. Not sure how this one would work with cross-repo dependencies or if there were no Depends-On but a topic's changes spanned multiple repos. Or if a change has multiple topics matching the regex. Or if it makes any sense for non-linear dependency chains.
3. (This is the one I mainly considered:) In the example I gave above you would just inject the zuul-change-stack-approved event whenever you saw a chain of proposed commits that started with a child of (a commit in) the target branch and ended with a commit with no child, that all have a common approval.
If between approval and merge another Gerrit change was added to that chain and then it also got the approval, another event would be generated and a new Zuul change would enter the pipeline, representing the new entirety of the chaing of Gerrit changes. I guess this would be similar to two Github PRs being approved in quick succession where one contains all the commits of the other, plus an extra commit.
If between approval and merge new patch versions were pushed then approved then I think a new Zuul change could just enter the pipeline. If configured to do so then the old one would be dequeued (in its entirety - it would be a singular Zuul change from the pipeline's PoV), if not it would continue and then just fail to merge at the end. Just like for a singular Gerrit change.
I must admit as I wrote this I started to see some complexity I hadn't' considered wrt. the Change.isUpdateOf logic. I will keep thinking about it.
And what should the behavior be if testing for that group of changes fails (report failure to all of them, retest a subset)?
My assumption with all of these is that if the tests fail you report a failure against every Gerrit change (the tip commits gets a "this test failed gate checks" and the ancestors get a "this test was tested as part of X and failed - if it is useful in its own right, you could resubmit it indepdendently")
I'm not personally convinced it makes sense as a feature, but it's at least an interesting thought exercise.
Thanks for considering it!
- Brendan
Brendan Jackman brendan.jackman@bluwirelesstechnology.com writes:
Sure, it definitely makes sense how the differences in the workflows & models led to the differences in behaviour. In fact my org currently uses Gitlab, which in its current incarnation, is basically a Github clone with a basic revision-tracking feature. So I considered just writing a Gitlab driver for Zuul. Maybe that's a more pragmatic choice but Gerrit is (IMO...) a better code review tool than Gitlab, and my test HW availability issues would be a sad reason to be prevented from using it. A way to reduce pressure on test resources seems like something that would be useful to others too so that's why I wanted to investigate this idea despite it introducing a friction against Gerrit's natural model.
We'd love a gitlab driver and there are other folks who would be interested in helping if you decided to go that direction.
I haven't though them through in detail but there are 3 ways I considered this could potentially be done:
- Time-based chunking: if there are multiple proposed commits in a dependency
chain and the first Gerrit change gets the required approval, Zuul waits N seconds to see if the second one gets an approval. If it doesn't it tests and potentially merges the first commit. If it does then it resets the timer and waits to see if the next Gerrit change gets the approval. If the timer runs out or we reach a Gerrit change with no child, the whole chain gets tested as one Zuul change and potentially merged. This strategy feels a little wild to me to to configure/explain but it could be quite ergonomic once set up. It also feels a bit like a fundamental change to Zuul's model, just jimmied into the Gerrit driver instead of where it really belongs.
This is similar to something else that's come up recently in discussion, which I'm calling the bisecting pipeline manager. It would be a dependent pipeline manager (so it can gate) which, rather than testing each commit, would test batches of commits (largely determined by the timing of when it runs), merging them all, or, if they fail, splitting the batch in half on each failure (modulo their internal dependencies). It's not exactly what you described, but it is a similar mechanism where the goal is to use fewer test resources when gating (and therefore support longer test times). It could also actually be combined with the NNFI algorithm to operate in parallel as well (if, say, you wanted to support two or more batches running at the same time).
It's still not exactly a trivial problem, however, it is generally useful and isn't Gerrit specific.
But back to your situation -- I think we've beet talking mostly about a gate pipeline -- what do you want to do about check (where Zuul runs jobs on patchset upload)? Do you want to avoid having a check pipeline? Have a check pipeline, but not require a result from it in order to gate and put it at very low precedence (so that your system will get around to checking changes as capacity permits)? Or do you also want to modify check pipeline behavior so that it somehow only runs jobs on the end of a dependency chain? Or something else?
-Jim
On 27/11/2018 22:57, James E. Blair wrote:
Brendan Jackman brendan.jackman@bluwirelesstechnology.com writes:
Sure, it definitely makes sense how the differences in the workflows & models led to the differences in behaviour. In fact my org currently uses Gitlab, which in its current incarnation, is basically a Github clone with a basic revision-tracking feature. So I considered just writing a Gitlab driver for Zuul. Maybe that's a more pragmatic choice but Gerrit is (IMO...) a better code review tool than Gitlab, and my test HW availability issues would be a sad reason to be prevented from using it. A way to reduce pressure on test resources seems like something that would be useful to others too so that's why I wanted to investigate this idea despite it introducing a friction against Gerrit's natural model.
We'd love a gitlab driver and there are other folks who would be interested in helping if you decided to go that direction.
I haven't though them through in detail but there are 3 ways I considered this could potentially be done:
- Time-based chunking: if there are multiple proposed commits in a dependency
chain and the first Gerrit change gets the required approval, Zuul waits N seconds to see if the second one gets an approval. If it doesn't it tests and potentially merges the first commit. If it does then it resets the timer and waits to see if the next Gerrit change gets the approval. If the timer runs out or we reach a Gerrit change with no child, the whole chain gets tested as one Zuul change and potentially merged. This strategy feels a little wild to me to to configure/explain but it could be quite ergonomic once set up. It also feels a bit like a fundamental change to Zuul's model, just jimmied into the Gerrit driver instead of where it really belongs.
This is similar to something else that's come up recently in discussion, which I'm calling the bisecting pipeline manager. It would be a dependent pipeline manager (so it can gate) which, rather than testing each commit, would test batches of commits (largely determined by the timing of when it runs), merging them all, or, if they fail, splitting the batch in half on each failure (modulo their internal dependencies). It's not exactly what you described, but it is a similar mechanism where the goal is to use fewer test resources when gating (and therefore support longer test times). It could also actually be combined with the NNFI algorithm to operate in parallel as well (if, say, you wanted to support two or more batches running at the same time).
It's still not exactly a trivial problem, however, it is generally useful and isn't Gerrit specific.
That's really interesting - what you've described is certainly a more elegant and generic version of what I was imagining, it would certainly solve my issue. Maybe that's the way to go. I'll try to dig up the discusson and read up on the idea.
(The only negative thing that comes to mind is that when you (or, when someone with programming habits like mine) post a patch series, the first N-1 are often nothing but preparatory refactoring to enable the final commit's diff to be focussed on adding the feature/fixing the bug. It would be a little annoying if the gate job found that the business end of the patch-stack was broken in an unforeseen way, but then went ahead and merged all the preparatory refactoring commits anyway. Maybe you need to re-write the final commit and change the approach to all your refactoring. But that's a problem (if you see it as a problem) with the current dependent manager anyway, obviously not a dealbreaker.)
But back to your situation -- I think we've beet talking mostly about a gate pipeline -- what do you want to do about check (where Zuul runs jobs on patchset upload)? Do you want to avoid having a check pipeline? Have a check pipeline, but not require a result from it in order to gate and put it at very low precedence (so that your system will get around to checking changes as capacity permits)? Or do you also want to modify check pipeline behavior so that it somehow only runs jobs on the end of a dependency chain? Or something else?
I haven't thought too much about how my proposal applies to the independent pipeline manager. In my scenario the check pipeline jobs don't put much pressure on test resources - they don't take very long and often don't require special HW (unit/module tests, compilation, formatting checks), it's the sort of thing I'd want to run against every commit. I guess I assumed that's how
everyone uses the check/gate model.
Cheers,
Brendan
On 2018-11-28 05:23:56 +0000 (+0000), Brendan Jackman wrote: [...]
In my scenario the check pipeline jobs don't put much pressure on test resources - they don't take very long and often don't require special HW (unit/module tests, compilation, formatting checks), it's the sort of thing I'd want to run against every commit. I guess I assumed that's how everyone uses the check/gate model.
For OpenStack projects, as a counter-point, we generally run the exact same set of jobs in both check and gate pipelines. The check pipeline is more about providing immediate feedback to change authors and reviewers, while the gate pipeline is there to make sure that the jobs which ran in check who-knows-how-long-ago are still passing with the latest state of the target branch (and with any changes which happen to get approved ahead of it but haven't yet merged). Having a change pass check jobs but then fail in the gate is frustrating and time-consuming for both change authors and reviewers, so providing them with the earliest possible picture of whether a change is likely to merge has been taken as a priority in that workflow.
With limited CI resources at your disposal I can see where you might make the compromise to reserve your more resource-intensive jobs only for approved changes, but it's definitely not how everyone uses Zuul and wasn't actually the reason for the check+gate design.
participants (3)
-
Brendan Jackman
-
corvus＠inaugust.com
-
Jeremy Stanley