[ovirt-devel] gerrit+ci improvement proposal
Greg Sheremeta
gshereme at redhat.com
Wed Jun 3 21:33:29 UTC 2015
+1 to only triggering CI when we're "ready." Via a flag or via
gerrit drafts -- either is fine with me.
----- Original Message -----
> From: "Eyal Edri" <eedri at redhat.com>
> To: "Max Kovgan" <mkovgan at redhat.com>
> Cc: infra at ovirt.org, devel at ovirt.org
> Sent: Wednesday, June 3, 2015 3:46:06 PM
> 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
> > 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.
>
> 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
> > Devel at ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/devel
> _______________________________________________
> Infra mailing list
> Infra at ovirt.org
> http://lists.ovirt.org/mailman/listinfo/infra
>
More information about the Infra
mailing list