[ovirt-devel] gerrit+ci improvement proposal

[Alexander Wels]

On Wednesday, June 03, 2015 01:22:54 PM Max Kovgan wrote:
> 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 looks like a good improvement to me.

> 
> 
> 
> 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 at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel