[ovirt-devel] gerrit+ci improvement proposal

[Nir Soffer]

----- Original Message -----
> From: "Max Kovgan" <mkovgan at redhat.com>
> To: devel at ovirt.org
> Cc: infra at 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

-1

You must have CI error state. Most of the errors I have seen, the CI fail to run,
and the failure is not related to the tested patch.

> 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

-1

Cannot require CI+1, the CI is not reliable enough yet.

Even if the CI will be reliable, a failed test which is not related to the submitted
patch should not block unrelated changes.

> 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.

-1

This means more work for developers.

Instead, make workflow default to +1. If the want to disable the CI, she can
set it to 0.

Developers need more power and less process overhead.

Nir