----- Original Message ----- > From: "Sandro Bonazzola" <sbona...@redhat.com> > To: "Eyal Edri" <ee...@redhat.com> > Cc: infra@ovirt.org, de...@ovirt.org > Sent: Thursday, June 4, 2015 9:50:56 AM > Subject: Re: [ovirt-devel] gerrit+ci improvement proposal > > Il 04/06/2015 08:46, Eyal Edri ha scritto: > > > > > > ----- Original Message ----- > >> From: "Sandro Bonazzola" <sbona...@redhat.com> > >> To: "Eyal Edri" <ee...@redhat.com>, "Max Kovgan" <mkov...@redhat.com> > >> Cc: de...@ovirt.org, infra@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" <mkov...@redhat.com> > >>>> To: de...@ovirt.org > >>>> Cc: infra@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 cycles and > >>>> encourage developers to write tests. > >>>> > >>>> # Problem > >>>> > >>>> Many patches are neither ready for review nor for CI upon submission, > >>>> which > >>>> is OK. > >>>> But running all the jobs on those patches with limited resources results > >>>> in: > >>>> overloaded resources, slow response time, unhappy developers. > >>>> > >>>> # Proposed Solution > >>>> > >>>> To run less jobs we know we don’t need to, thus making more resources > >>>> for > >>>> the > >>>> jobs we need to run. > >>>> We have been experimenting to make our CI stabler and quicker to respond > >>>> 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 - “to fail > >>>> early”, > >>>> yet full blown static code analysis and long tests to run “when ready”. > >>>> > >>>> # 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) should > >>>> be > >>>> able to set/override (except Jenkins). > >>>> > >>>> ## Workflow flag > >>>> > >>>> Will express patch “workflow” state. Values: > >>>> * 0 Work In Progress > >>>> * +1 Ready For Review > >>>> * +2 Ready For Merge > >>>> Permissions for setting: Owner can set +1, Project Maintainers can set > >>>> +2 > >>>> > >>>> ## Review + CI Integration: > >>>> > >>>> Merging [“Submit” button to appear] will require: Review+1, CI+1, > >>>> 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 the > >>>> patch > >>>> to be reviewed, and heavily auto-tested. > >>>> Maintainer now needs to set "Workflow+2" and wait for "Submit" button to > >>>> appear after CI has completed running gating tests. > >>>> > >>>> > >>>> Next step will be to automate merge the change after Workflow+2 has been > >>>> 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’ve tried detecting what to run from jenkins > >>>> relying only on gerrit comments so that upon Verified+1, we’d run the > >>>> job. > >>>> We could not use “Review+1”, because it makes no sense at all, so we > >>>> left > >>>> the job to set Verified+1. > >>>> Meaning - re-trigger itself immediately more than 1 times. > >>>> > >>>> Jenkins and its visitors very unhappy, and we had to stop those jobs, > >>>> 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 enforced: > >>>> 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 text > >>>> too. > >>>> > >>> > >>> +1 from me, releasing CI from running non critical and un-essential jobs > >>> will not only reduce load from ci, > >>> and shorted response time for developers, it will allow us to add much > >>> more > >>> powerful tests such as functional & system > >>> tests that actually add hosts and run VMs, improving our ability to find > >>> 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 reviewers > >>> - > >>> for example people can add an email rule > >>> to alert them only when they are added to patches that have Workflow+1, > >>> and > >>> not before. > >> > >> For human reviewers I suggest to keep using drafts until the patch is > >> finished. > > > > keep using? how many developers do you know are working with drafts until > > their patch is ready? > > We have ~350 drafts in gerrit so someone is using them :-) > https://gerrit.ovirt.org/#/q/is:draft
good to know :) so i guess even with that amount its still hard on jenkins.... > > > i agree if everyone would use drafts load on jenkins was already much > > lower, > > unfortunately its not the case. > > > >> Once it's finished and humans reviewed the logic of the patch, Workflow+1 > >> should be triggered allowing automation to check the correctness of the > >> patch. > >> IMHO there's no reason for wasting CI time on patches that will be correct > >> from an automation point of view but nacked by reviewers. Especially if > >> the > >> patches are part of a big patchset. > >> > >> > >>> > >>> And one final note, for Workflow+2 -> this is a preparation for a gating > >>> system, like Zuul used by openstack, that in the future > >>> we might use as automatic merger pending passing a verification step. > >>> this > >>> will prevent errors that happen sometimes > >>> post merge due to conflicts or other issues, and will be another level of > >>> validation before final merge. > >>> But as max said, its all part of the plan and we'll test it of course > >>> 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 > >>>> de...@ovirt.org > >>>> http://lists.ovirt.org/mailman/listinfo/devel > >>> _______________________________________________ > >>> Devel mailing list > >>> de...@ovirt.org > >>> http://lists.ovirt.org/mailman/listinfo/devel > >>> > >> > >> > >> -- > >> Sandro Bonazzola > >> Better technology. Faster innovation. Powered by community collaboration. > >> See how it works at redhat.com > >> _______________________________________________ > >> Infra mailing list > >> Infra@ovirt.org > >> http://lists.ovirt.org/mailman/listinfo/infra > >> > >> > >> > > > -- > Sandro Bonazzola > Better technology. Faster innovation. Powered by community collaboration. > See how it works at redhat.com > _______________________________________________ > Infra mailing list > Infra@ovirt.org > http://lists.ovirt.org/mailman/listinfo/infra > > > _______________________________________________ Infra mailing list Infra@ovirt.org http://lists.ovirt.org/mailman/listinfo/infra