Re: [ovirt-devel] gerrit+ci improvement proposal

=20 =20 ----- Original Message ----- > From: "Sandro Bonazzola" <sbonazzo(a)redhat.com&gt; > To: "Eyal Edri" <eedri(a)redhat.com&gt; > Cc: infra(a)ovirt.org, devel(a)ovirt.org > Sent: Thursday, June 4, 2015 9:50:56 AM > Subject: Re: [ovirt-devel] gerrit+ci improvement proposal >=20 > Il 04/06/2015 08:46, Eyal Edri ha scritto: > >=20 > >=20 > > ----- Original Message ----- > >> From: "Sandro Bonazzola" <sbonazzo(a)redhat.com&gt; > >> To: "Eyal Edri" <eedri(a)redhat.com&gt;, "Max Kovgan" <mkovgan(a)redhat.com&gt; > >> Cc: devel(a)ovirt.org, infra(a)ovirt.org > >> Sent: Thursday, June 4, 2015 9:11:10 AM > >> Subject: Re: [ovirt-devel] gerrit+ci improvement proposal > >> > >> Il 03/06/2015 21:46, Eyal Edri ha scritto: > >>> > >>> > >>> ----- Original Message ----- > >>>> From: "Max Kovgan" <mkovgan(a)redhat.com&gt; > >>>> To: devel(a)ovirt.org > >>>> Cc: infra(a)ovirt.org > >>>> Sent: Wednesday, June 3, 2015 8:22:54 PM > >>>> Subject: [ovirt-devel] gerrit+ci improvement proposal > >>>> > >>>> Hi everyone! > >>>> We really want to have reliable and snappy CI: to allow short cycl=

> >>>> encourage developers to write tests. > >>>> > >>>> # Problem > >>>> > >>>> Many patches are neither ready for review nor for CI upon submissi=

> >>>> which > >>>> is OK. > >>>> But running all the jobs on those patches with limited resources r=

> >>>> in: > >>>> overloaded resources, slow response time, unhappy developers. > >>>> > >>>> # Proposed Solution > >>>> > >>>> To run less jobs we know we don=E2=80=99t need to, thus making mor=

> >>>> for > >>>> the > >>>> jobs we need to run. > >>>> We have been experimenting to make our CI stabler and quicker to r=

> >>>> by > >>>> using gerrit flags. This has improved in both directions very well > >>>> internally. > >>>> Now it seems a good time to let all the oVirt projects to use this. > >>>> This solution indirectly promotes reviews and quick tests - =E2=80=

> >>>> early=E2=80=9D, > >>>> yet full blown static code analysis and long tests to run =E2=80=

> >>>> > >>>> # How it works > >>>> > >>>> 2 new gerrit independent flags are added to gerrit. > >>>> > >>>> ## CI flag > >>>> > >>>> Will express patch CI status. Values: > >>>> * +1 CI passed > >>>> * 0 CI did not run yet > >>>> * -1 CI failed > >>>> Permissions for setting: project maintainers (for special cases) s=

> >>>> be > >>>> able to set/override (except Jenkins). > >>>> > >>>> ## Workflow flag > >>>> > >>>> Will express patch =E2=80=9Cworkflow=E2=80=9D state. Values: > >>>> * 0 Work In Progress > >>>> * +1 Ready For Review > >>>> * +2 Ready For Merge > >>>> Permissions for setting: Owner can set +1, Project Maintainers can=

> >>>> +2 > >>>> > >>>> ## Review + CI Integration: > >>>> > >>>> Merging [=E2=80=9CSubmit=E2=80=9D button to appear] will require: =

> >>>> Workflow+2 > >>>> Patch lifecycle now is: > >>>> --------------------------------------------------------------- > >>>> patch state |owner |reviewer |maintainer |CI tests |pass > >>>> --------------------------------------------------------------- > >>>> added/updated |- |- |- |quick |CI+1 > >>>> review |Workflow+1|Review+1 |- |heavy |CI+1 > >>>> merge ready |- |- |Workflow+2 |gating |CI+1 > >>>> merge |- |- |merge |merge |CI+1 > >>>> > >>>> Changes from current workflow: > >>>> Owner only adds reviewers, now owner needs to set "Workflow+1" for=

> >>>> patch > >>>> to be reviewed, and heavily auto-tested. > >>>> Maintainer now needs to set "Workflow+2" and wait for "Submit" but=

> >>>> appear after CI has completed running gating tests. > >>>> > >>>> > >>>> Next step will be to automate merge the change after Workflow+2 ha=

> >>>> set > >>>> by the Maintainer and gating tests passed. > >>>> > >>>> > >>>> ## Why now? > >>>> > >>>> It is elimination of waste. The sooner - the better. > >>>> The solution has been used for a while and it works. > >>>> Resolving the problem without gerrit involved will lead to adding > >>>> unreliable > >>>> code into jobs, and will still be prone to problems: > >>>> Just recently, 3d ago we=E2=80=99ve tried detecting what to run =

> >>>> relying only on gerrit comments so that upon Verified+1, we=E2=

> >>>> job. > >>>> We could not use =E2=80=9CReview+1=E2=80=9D, because it makes no=

> >>>> left > >>>> the job to set Verified+1. > >>>> Meaning - re-trigger itself immediately more than 1 times. > >>>> =20 > >>>> Jenkins and its visitors very unhappy, and we had to stop those =

> >>>> clean > >>>> up the queue, and spam developers. > >>>> > >>>> ## OK OK OK. Now what? > >>>> > >>>> Now we want your comments and opinions before pushing this further: > >>>> Please participate in this thread, so we can start trying it out. > >>>> Ask, Suggest better ideas, all this is welcome. > >>>> > >>>> > >>>> Best Regards! > >>>> > >>>> > >>>> N.B. > >>>> Of course, this is not written in stone, in case we find a better > >>>> approach > >>>> on > >>>> solving those issues, we will change to it. > >>>> And we will keep improving so don't be afraid that it will be enfo=

> >>>> if > >>>> this does not work out we will discard it. > >>>> > >>>> P.S. > >>>> Kudos to dcaro, most of the work was done by him, and most of this=

> >>>> too. > >>>> > >>> > >>> +1 from me, releasing CI from running non critical and un-essential=

> >>> will not only reduce load from ci, > >>> and shorted response time for developers, it will allow us to add m=

> >>> more > >>> powerful tests such as functional & system > >>> tests that actually add hosts and run VMs, improving our ability to=

> >>> regression much more effectively. > >>> > >>> Another benefit to consider is saving reviewers time. I.e not only > >>> jenkins > >>> benefits from Worklow+1, but also human reviewers. > >>> Instead of looking at a patch that is too early to be reviewed, the > >>> author > >>> can set the Workflow+1 when the code is ready to review > >>> (even if he didn't verified it yet), thus saving time to other revi=

> >>> - > >>> for example people can add an email rule > >>> to alert them only when they are added to patches that have Workflo=

> >>> and > >>> not before. > >> > >> For human reviewers I suggest to keep using drafts until the patch is > >> finished. > >=20 > > keep using? how many developers do you know are working with drafts u=

> > their patch is ready? >=20 > We have ~350 drafts in gerrit so someone is using them :-) > https://gerrit.ovirt.org/#/q/is:draft =20 good to know :) so i guess even with that amount its still hard on jenkins....=20

=20 >=20 > > i agree if everyone would use drafts load on jenkins was already much > > lower, > > unfortunately its not the case. > >=20 > >> Once it's finished and humans reviewed the logic of the patch, Workf=

> >> should be triggered allowing automation to check the correctness of =

> >> patch. > >> IMHO there's no reason for wasting CI time on patches that will be c=

> >> from an automation point of view but nacked by reviewers. Especially=

> >> the > >> patches are part of a big patchset. > >> > >> > >>> > >>> And one final note, for Workflow+2 -> this is a preparation for a g=

> >>> system, like Zuul used by openstack, that in the future > >>> we might use as automatic merger pending passing a verification ste=

> >>> this > >>> will prevent errors that happen sometimes > >>> post merge due to conflicts or other issues, and will be another le=

> >>> validation before final merge. > >>> But as max said, its all part of the plan and we'll test it of cour=

> >>> before implementing to see its value. > >>> > >>> > >>>> > >>>> > >>>> > >>>> Max Kovgan > >>>> > >>>> Senior Software Engineer > >>>> Red Hat - EMEA ENG Virtualization R&D > >>>> Tel.: +972 9769 2060 > >>>> Email: mkovgan [at] redhat [dot] com > >>>> Web: http://www.redhat.com > >>>> RHT Global #: 82-72060 > >>>> > >>>> _______________________________________________ > >>>> Devel mailing list > >>>> Devel(a)ovirt.org > >>>> http://lists.ovirt.org/mailman/listinfo/devel > >>> _______________________________________________ > >>> Devel mailing list > >>> Devel(a)ovirt.org > >>> http://lists.ovirt.org/mailman/listinfo/devel > >>> > >> > >> > >> -- > >> Sandro Bonazzola > >> Better technology. Faster innovation. Powered by community collabora=

> >> See how it works at redhat.com > >> _______________________________________________ > >> Infra mailing list > >> Infra(a)ovirt.org > >> http://lists.ovirt.org/mailman/listinfo/infra > >> > >> > >> >=20 >=20 > -- > Sandro Bonazzola > Better technology. Faster innovation. Powered by community collaboratio=

> See how it works at redhat.com > _______________________________________________ > Infra mailing list > Infra(a)ovirt.org > http://lists.ovirt.org/mailman/listinfo/infra >=20 >=20 >=20 _______________________________________________ Infra mailing list Infra(a)ovirt.org http://lists.ovirt.org/mailman/listinfo/infra

-----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQEcBAEBAgAGBQJVb/3PAAoJEEBxx+HSYmnDnxIH/jHHtyMnvplDSSWZSoNC5r8v x7l7LUzc/21Ai1q7DRW9fM1jYtZM+cJQcsrb8YJC5RF6GMV7zL/AF6yXAhbLFQUg vgRC4QgeEk7usZ9Tss70JJKgr1044SAXqxwpcTFrDkBKNmqQpk1eQ5oMHLRe0I9u MxVPlLY+w4m208BAiMRHpITs0B/339a4f4mEqxUl8MMuYRxh5qwe1PQs0mFGt4c8 de7NJUi0AluTMq4soDJUPUQDmknLabUwYL2Dg9EG+x2I66wZI9Rxi44PjUmuXmLf Ig0uHapD6z4DS6HZGAUt+RAKXW4jTgig1vElYgtMUK237FRuDbPvqOzE0fCLfHU= =uymq -----END PGP SIGNATURE-----