[ovirt-devel] gerrit+ci improvement proposal

[Eyal Edri]

----- Original Message -----
> From: "Nir Soffer" <nsoffer at redhat.com>
> To: "Max Kovgan" <mkovgan at redhat.com>
> Cc: infra at ovirt.org, devel at ovirt.org
> Sent: Thursday, June 4, 2015 10:26:22 AM
> Subject: Re: [ovirt-devel] gerrit+ci improvement proposal
> 
> ----- 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.

its not possible to populate the exact error into the gerrit comment,
that's why there is a link to the job.
the fact that you've seen false positive errors is unfourtunate, but it doesn't represent 
the majority of patches, and each false positive error should be communicated to infra team
so we can resolve it, it has nothing to do with this suggestion,
on the contrary - once jenkins will run much less un-needed runs on patches,
the probability for false positives will reduce dramatically.

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

you're repeating the same calim from above, same answer.

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

this will serve nothing, and jobs will keep running on each patchset as now,
increasing the time you need to wait for each job to run and adding risk
for failures.

the only alternative to this is to start using Drafts for in progress patches,
but only 1/4 of the patches are drafts atm, so we still are running more jobs on ci than we should,
thus preventing us from running more complex and useful jobs.

> 
> Developers need more power and less process overhead.
> 
> Nir
> _______________________________________________
> Infra mailing list
> Infra at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/infra
> 
> 
>