[ovirt-devel] Gerrit parallel patch handling and CI (Or, why did my code fail post-merge)

[Vojtech Szocs]

----- Original Message -----
> From: "Barak Korren" <bkorren at redhat.com>
> To: "Vojtech Szocs" <vszocs at redhat.com>
> Cc: "devel" <devel at ovirt.org>
> Sent: Thursday, December 1, 2016 9:10:19 AM
> Subject: Re: [ovirt-devel] Gerrit parallel patch handling and CI (Or, why did my code fail post-merge)
> 
> On 30 November 2016 at 19:12, Vojtech Szocs <vszocs at redhat.com> wrote:
> >
> > So before we adopt auto-submit-after-ci-pass (e.g. using Zuul, as Eyal
> > mentioned in that thread), we should be able to manually trigger heavy
> > CI (check-merged) job from Gerrit web interface, is that correct?
> 
> We could make it a two-step operation where you indicate you want to
> submit, wait for the CI output and then click submit - but why not
> just let the system submit for you?
> 
> One road I don't think we should go down is to have 3 CI stages:
> "light" check_patch, "heavy" check_patch and "pre-merge", this looks
> like a design smell to me.
> 
> WRT Zuul:
> 
> Unless you are using a strict policy like ff-only (which so far seems
> no to scale, because it is essentially a GIL on merging), Zuul is
> essential in order to test the right code in CI (even just for basic
> check_patch testing). So we are working to bring it in (See OVIRT-734
> [1] and OVIRT-882 [2]).

Thanks for sharing those links.

I didn't know you're already working on adopting Zuul, my bad =)

So the maintainer can simply express his/her intent to merge the given
patch, and CI infra takes care of the rest (run heavy tests and submit
changes if successful).

> 
> WRT merge gating - Zuul tries to make the process "fire and forget" as
> far as maintainers are concerned - if failures are found, it actually
> tries out different combinations of patches to see if, from a set of
> patches that were asked to be merged, some could be merged even when
> others could not.

I'd be cautious with this feature, since our heavy CI tests involve
GWT compilation, so Zuul trying to run more tests (on different patch
combinations) = more time spent.

> 
> > If so, it would be the patch owner's responsibility to submit (merge)
> > only after heavy CI (check-merged) pass?
> 
> The question hiding here is wither a maintainer could submit _without_
> passing the "heavy" CI. I'm guessing some maintainers will demand
> that, but I'm hoping in the long run this will not be needed and you
> will know that once submitted, a patch will be eventually merged
> unless it has real issues in it.

We cannot rule out issues that might happen in future. There will be
flaky/broken tests or CI infra issues, we need to decide how to deal
with those, I think.

> 
> > So we could actually write Gerrit plugin using MergeValidationListener
> > that would operate in the following way:
> > ...snip...
> 
> Just because something could be written doesn't mean it should - we
> are already stretched too thinly over too much code in the CI system,
> we do not want to maintain any more of it, certainly not in a core
> component like Gerrit.

It was just an idea =) Zuul sounds more like a proper solution.

> 
> Then again, this is a prioritization question - it not up to me to decide.
> 
> [1]: https://ovirt-jira.atlassian.net/browse/OVIRT-734
> [2]: https://ovirt-jira.atlassian.net/browse/OVIRT-882
> 
> --
> Barak Korren
> bkorren at redhat.com
> RHCE, RHCi, RHV-DevOps Team
> https://ifireball.wordpress.com/
>