[ovirt-devel] Creating a new gerrit flag

David Caro dcaroest at redhat.com
Thu Dec 11 11:22:01 UTC 2014


On 12/11, Nir Soffer wrote:
> 
> 
> ----- Original Message -----
> > From: "David Caro" <dcaroest at redhat.com>
> > To: "Nir Soffer" <nsoffer at redhat.com>
> > Cc: "Eyal Edri" <eedri at redhat.com>, "Oved Ourfali" <ovedo at redhat.com>, "infra" <infra at ovirt.org>, devel at ovirt.org
> > Sent: Thursday, December 11, 2014 12:06:24 PM
> > Subject: Re: [ovirt-devel] Creating a new gerrit flag
> > 
> > On 12/11, Nir Soffer wrote:
> > > ----- Original Message -----
> > > > From: "David Caro" <dcaroest at redhat.com>
> > > > To: "Nir Soffer" <nsoffer at redhat.com>
> > > > Cc: "Eyal Edri" <eedri at redhat.com>, "Oved Ourfali" <ovedo at redhat.com>,
> > > > "infra" <infra at ovirt.org>, devel at ovirt.org
> > > > Sent: Wednesday, December 10, 2014 4:59:36 PM
> > > > Subject: Re: [ovirt-devel] Creating a new gerrit flag
> > > > 
> > > > On 12/10, Nir Soffer wrote:
> > > > > 
> > > > > 
> > > > > ----- Original Message -----
> > > > > > From: "Eyal Edri" <eedri at redhat.com>
> > > > > > To: devel at ovirt.org
> > > > > > Cc: "Oved Ourfali" <ovedo at redhat.com>, "infra" <infra at ovirt.org>
> > > > > > Sent: Wednesday, December 10, 2014 10:40:47 AM
> > > > > > Subject: Re: [ovirt-devel] Creating a new gerrit flag
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > ----- Original Message -----
> > > > > > > From: "Oved Ourfali" <ovedo at redhat.com>
> > > > > > > To: "David Caro" <dcaroest at redhat.com>
> > > > > > > Cc: devel at ovirt.org
> > > > > > > Sent: Wednesday, December 10, 2014 8:30:30 AM
> > > > > > > Subject: Re: [ovirt-devel] Creating a new gerrit flag
> > > > > > > 
> > > > > > > 
> > > > > > > 
> > > > > > > ----- Original Message -----
> > > > > > > > From: "David Caro" <dcaroest at redhat.com>
> > > > > > > > To: "Oved Ourfali" <ovedo at redhat.com>
> > > > > > > > Cc: devel at ovirt.org
> > > > > > > > Sent: Tuesday, December 9, 2014 7:02:44 PM
> > > > > > > > Subject: Re: [ovirt-devel] Creating a new gerrit flag
> > > > > > > > 
> > > > > > > > On 12/09, Oved Ourfali wrote:
> > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > ----- Original Message -----
> > > > > > > > > > From: "David Caro" <dcaroest at redhat.com>
> > > > > > > > > > To: "Oved Ourfali" <ovedo at redhat.com>
> > > > > > > > > > Cc: "Sven Kieske" <s.kieske at mittwald.de>, devel at ovirt.org
> > > > > > > > > > Sent: Tuesday, December 9, 2014 3:40:30 PM
> > > > > > > > > > Subject: Re: [ovirt-devel] Creating a new gerrit flag
> > > > > > > > > > 
> > > > > > > > > > On 12/09, Oved Ourfali wrote:
> > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > ----- Original Message -----
> > > > > > > > > > > > From: "Sven Kieske" <s.kieske at mittwald.de>
> > > > > > > > > > > > To: devel at ovirt.org
> > > > > > > > > > > > Sent: Tuesday, December 9, 2014 3:21:43 PM
> > > > > > > > > > > > Subject: Re: [ovirt-devel] Creating a new gerrit flag
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > 
> > > > > > > > > > > > On 09/12/14 13:47, Oved Ourfali wrote:
> > > > > > > > > > > > > safe up to 95% or so.
> > > > > > > > > > > > 
> > > > > > > > > > > > You just made up that number.
> > > > > > > > > > > > I don't really understand why you would want
> > > > > > > > > > > > to downgrade your code quality by circumventing tests.
> > > > > > > > > > > > 
> > > > > > > > > > > > Maybe someone can elaborate on this a bit?
> > > > > > > > > > > > 
> > > > > > > > > > > 
> > > > > > > > > > > It doesn't downgrade the code quality.
> > > > > > > > > > > It is just a way to ensure developers can both merge
> > > > > > > > > > > changes,
> > > > > > > > > > > and
> > > > > > > > > > > do
> > > > > > > > > > > it
> > > > > > > > > > > as
> > > > > > > > > > > safely as possible without relying on post-submit tools.
> > > > > > > > > > > The number is indeed invented... as I don't have real
> > > > > > > > > > > statistics,
> > > > > > > > > > > but
> > > > > > > > > > > it
> > > > > > > > > > > comes to say that it would be safe most of the time.
> > > > > > > > > > > After the patch is merged, if CI will fail, it is the
> > > > > > > > > > > responsibility
> > > > > > > > > > > of
> > > > > > > > > > > the
> > > > > > > > > > > developer to check that out and fix that.
> > > > > > > > > > 
> > > > > > > > > > This thread was started to avoid getting to that point, as
> > > > > > > > > > getting a
> > > > > > > > > > failed patch inside the code means breaking all the other
> > > > > > > > > > tests
> > > > > > > > > > that
> > > > > > > > > > run on top of it and that blocks all the development, not
> > > > > > > > > > only
> > > > > > > > > > that
> > > > > > > > > > specific patch.
> > > > > > > > > > 
> > > > > > > > > 
> > > > > > > > > The issue that started the discussion was an issue in which
> > > > > > > > > there
> > > > > > > > > was a
> > > > > > > > > Tests "-1" flag, and it was ignored.
> > > > > > > > > My suggestion will enforce that it won't be ignored.
> > > > > > > > > In more rare cases, in which the rebase is the source of the
> > > > > > > > > tests
> > > > > > > > > issue,
> > > > > > > > > then you'll find about it later.
> > > > > > > > 
> > > > > > > > I started the discussion, and I started it because a developer
> > > > > > > > complained about not being able to merge a patch because it was
> > > > > > > > failing the tests due to an already merged patch that was making
> > > > > > > > all
> > > > > > > > the builds to fail. And was trying to get a solution to avoid
> > > > > > > > getting
> > > > > > > > to that point where a patch is merged while breaking the tests.
> > > > > > > > 
> > > > > > > > 
> > > > > > > > So in summary, you are suggestion this:
> > > > > > > > 
> > > > > > > > Creating a new flag 'tested' with values +1, 0 and -1 that only
> > > > > > > > jenkins
> > > > > > > > and managers can set
> > > > > > > > 
> > > > > > > > Block form submitting any patches that have a -1
> > > > > > > > 
> > > > > > > > Carry the value of that flag to following patches only if the
> > > > > > > > flag
> > > > > > > > was
> > > > > > > > -1
> > > > > > > > 
> > > > > > > 
> > > > > > 
> > > > > > +1, we need a way to block bad patches from being merged, even with a
> > > > > > rebase
> > > > > > in gerrit.
> > > > > > going forward we're planning a few changes to the way jenkins jobs
> > > > > > are
> > > > > > run on
> > > > > > ovirt ci, which will help
> > > > > > reduce noise and imrove resources usages.
> > > > > > 
> > > > > > 1. moving into a flow process, where critical jobs like unit
> > > > > > tests/checkstyle
> > > > > > will run first and only then other heavy jobs will run
> > > > > > (integration/rpms/findbugs)
> > > > > 
> > > > > This is already implemented in vdsm for few months - running "make
> > > > > check"
> > > > > will run the fast tests first and will not run the slower tests if a
> > > > > fast
> > > > > test
> > > > > failed.
> > > > 
> > > > Please change to be able to run only fast tests or only slow tests,
> > > > that way we can separate the job into two and give feedback about the
> > > > fast tests before the slows have finished running.
> > > 
> > > These are the available targets (from faster to slower):
> > > 
> > > - gitignore - check that certain files are ignored
> > > 
> > > - pyflakes - check common Python errors (e.g. unused imports)
> > > 
> > > - pep8 - style check
> > > 
> > > - check - run the fast checks above and if successful, the unittests
> > > 
> > >   Environment variables:
> > > 
> > >   NOSE_SLOW_TESTS=1 - enable slow tests (we have only few)
> > >   NOSE_STRESS_TESTS=1 - enable stress tests (probably not useful for the
> > >   CI)
> > > 
> > >   Note that the environment variables are used only for the tests in
> > >   vdsm/tests there are few tests in various sub directories that do
> > >   not use the test infrastructure in vdsm/tests.
> > > 
> > > - check-all - run make check enbaling both slow and stress tests
> > > 
> > > Do you need a separate target for the unittests?
> > 
> > I just want to be able to execute:
> > 
> > > make fast-check
> > 
> > for fast checks (you decide what are and what are not)
> > 
> > > make slow-check
> > 
> > for slow checks, non overlapping, meaning that if you want to runa ll
> > the checks, you'll have to use fast and then slow.
> > 
> > The idea is to generalize the interface we use for the tests between
> > all the ovirt projects so from ci we don't have to go keeping specific
> > scripts for each provect and for each project version. And be able to
> > run the fast checks (on each patchset, merges included) and the slow
> > ones (on each merge only) separated so we can give feedback faster and
> > not start the slow ones. Then after that goes the building of the rpm,
> > and then the functional tests. And last the release if relevant.
> > 
> > On patch:
> >    fast_check
> 
> fast_check is not a good name - how about check-patch?

That name is just a placeholder for the step where you run the fast
checks, however you do it is not specified here.

> 
> > 
> > On merge:
> >    fast_check -> slow_check -> build -> functional_check -> release
> 
> Same: how about check-merge?
> 
> > 
> > That flow is generic for all the projects, so each project will have
> > to implement the same interface to run each step (I don't really care
> > if it's make fast-check or just a bash script that runs whatever you
> > need underneath, the key is not having to specify any options if
> > possible and running just one line, the same everywhere).
> 
> These target do not make any sense out of the CI environment.

And that's part of the development process, as much as creating unit
tests. I think that's the point that is creating all the fuss around
this. For you CI is external to the development.

> 
> I think you should implement these using a wrapper, adapting any
> project the the CI "interface".

I totally disagree, because that 'adapter' is bound to a project and
to a version, making it a perfect fit to have it inside the
project code, changing it at the same time you change the code and
adapting it to the code changes while they are made.

Writing that adapter external to the project itself means that you
have one adapter per project, and per project version, so doing some
number, we have right now >50 projects so that would mean having to
monitor all the 50 projects for each patch, and create a new adapet
each time a change is made that modifies the way it runs the tests or
the dependencies it needs. And bind that to that specific hash (and
future ones).

Compare that to having the 'adapter' inside the project, where you
modify it on the same commit you change the way it builds or the
dependencies, and just using that.

If on the first step you need a team with 10% (random number) of the
developers creating patches to keep up the pace, with the last you
don't need anyone doing that job and you'll get no false failure and
you'll have no 'adaptation' period where the code running does not
have the correct deps or tests.


> 
> We can provide the building blocks using the makefile, so the CI
> can use them to build the required flow.

I don't really care what tools to use, I just want the interface
implemented. If you decide to use a bash script ok for me, if it's a
make targe, ok for ma, if its a perl script, ruby script, python
script or anything else, ok for me too. Just the same everywhere.

> 
> > 
> > That simplifies a LOT the maintenance of the jenkins jobs, and allows
> > to change the way checks are run bound to the version of the product.
> 
> Since each version may have different makefile, the CI wrapper must
> handle this.

Totally disagree. Not implementing this means that the job will fail
each time the code changes (because you can't maintain as many job
versions as code changes in parallel and select each of them depending
on the commit hash or whatever). This is a blocker to automate any
CI and to keep a stable CI.
If this is not done the stability will keep being low (because each
dependency or version change will mean that the jobs fail, just like
they are failing due to ioprocess or as they failed with pthread or
pep8).
And the maintenance will be very high, just like it is right now,
requiring puppet and job changes each time a new dependency is needed
and doing tricks and malabars when versions conflict, just like we do
now, what enforces us to use specialized slaves and not being able to
take advantage of all the resources we have.

> 
> Nir

-- 
David Caro

Red Hat S.L.
Continuous Integration Engineer - EMEA ENG Virtualization R&D

Tel.: +420 532 294 605
Email: dcaro at redhat.com
Web: www.redhat.com
RHT Global #: 82-62605
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <http://lists.ovirt.org/pipermail/devel/attachments/20141211/e9069e47/attachment-0001.sig>


More information about the Devel mailing list