Re: [ovirt-devel] Merge gating in Gerrit

[Martin Sivak]

Well it is only partially off topic, because it is impossible to merge
a db script close to release. A file rename means new CI job and that
can take ages (especially for the engine and the multiple branches we
have). The same what Martin is describing for the engine with FF only
setup.

Gating does not necessarily have to mean blocking only, it can contain
an ordering component too.

Martin



On Mon, Nov 21, 2016 at 3:04 PM, Barak Korren  wrote:
> On 21 November 2016 at 13:51, Martin Sivak  wrote:
>> Will it also auto-rename the database scripts? Please please!
>>
> Well, automated systems are not supposed to make code changes without
> humans being aware (see other thread about "ff-only" where "rebase if
> necessary" is a counter-example).
>
> What we could do is have the gate fail when colliding scripts show up
> (we will also have to use "ff-only" then to ensure changes are
> ordered). Actually this test is simple enough for "regular" CI to do,
> so no need to wait for the gate...
>
> Anyway, this is not directly related to merge-gating, so a little off-topic 
> IMO.
>
> --
> Barak Korren
> bkor...@redhat.com
> RHEV-CI Team
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Merge gating in Gerrit

[Barak Korren]

On 21 November 2016 at 13:51, Martin Sivak  wrote:
> Will it also auto-rename the database scripts? Please please!
>
Well, automated systems are not supposed to make code changes without
humans being aware (see other thread about "ff-only" where "rebase if
necessary" is a counter-example).

What we could do is have the gate fail when colliding scripts show up
(we will also have to use "ff-only" then to ensure changes are
ordered). Actually this test is simple enough for "regular" CI to do,
so no need to wait for the gate...

Anyway, this is not directly related to merge-gating, so a little off-topic IMO.

-- 
Barak Korren
bkor...@redhat.com
RHEV-CI Team
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Merge gating in Gerrit

[Eyal Edri]

Off topic I think..,  but wasn't there a Gerrit plugin Roy wrote for it?

On Nov 21, 2016 13:51, "Martin Sivak"  wrote:

> Will it also auto-rename the database scripts? Please please!
>
> Martin
>
> On Mon, Nov 21, 2016 at 12:40 PM, Eyal Edri  wrote:
> > This isn't gating.
> > Just trigger to run more heavy lifting CI jobs,  the idea is to replace
> the
> > manual submit with automatic system like Zuul.
> >
> >
> > On Nov 21, 2016 1:32 PM, "Tal Nisan"  wrote:
> >>
> >> Why not use +1 on verified? That way the patch owner can wait till the
> >> code review process is over, mark it as verified, wait for CI and then
> >> submit.
> >> It doesn't really give much added value to the code reviewers whether
> it's
> >> marked as verified or not
> >>
> >> On Sun, Nov 20, 2016 at 10:26 PM, Sandro Bonazzola  >
> >> wrote:
> >>>
> >>> Il 20/Nov/2016 17:25, "Nir Soffer"  ha scritto:
> >>> >
> >>> > On Sun, Nov 20, 2016 at 5:39 PM, Yedidyah Bar David  >
> >>> > wrote:
> >>> > > On Sun, Nov 20, 2016 at 5:06 PM, Barak Korren 
> >>> > > wrote:
> >>> > >> Hi all,
> >>> > >>
> >>> > >> Perhaps the main purpose of CI, is to prevent braking code from
> >>> > >> getting merged into the stable/master branches. Unfortunately our
> CI
> >>> > >> is not there yet, and one of the reasons for that is that we do
> >>> > >> large
> >>> > >> amount of our CI tests only _after_ the code is merged.
> >>> > >>
> >>> > >> The reason for that is that when balancing through, but time
> >>> > >> consuming, tests (e.g. enging build with all permutations) v.s.
> >>> > >> faster
> >>> > >> but more basic ones (e.g. "findbugs" and single permutation
> build),
> >>> > >> we
> >>> > >> typically choose the faster tests to be run per-patch-set and
> leave
> >>> > >> the through testing to only be run post-merge.
> >>> > >>
> >>> > >> We'd like to change that and have the through tests also run
> before
> >>> > >> merge. Ideally we would like to just hook stuff to the "submit"
> >>> > >> button, but Gerrit doesn't allow one to do that easily. So instead
> >>> > >> we'll need to adopt some kind of flag to indicate we want to
> submit
> >>> > >> and have Jenkins
> >>> > >> "click" the submit button on our behalf if tests pass.
> >>> > >>
> >>> > >> I see two options here:
> >>> > >> 1. Use Code-Review+2 as the indicator to run "heavy" CI and merge.
> >>> >
> >>> > This is problematic. For example in vdsm we have 5 maintainers with
> >>> > +2, and 4 maintainers with commit right, but only 2 are commenting
> >>> > regularly.
> >>> >
> >>> > >> 2. Add an "approve" flag that maintainers can set to +1 (This is
> >>> > >>what OpenStack is doing).
> >>> >
> >>> > This seems better.
> >>> >
> >>> > But there is another requirement - maintainer should be able to
> commit
> >>> > even if jenkins fails. Sometimes the CI is broken, or there are
> flakey
> >>> > tests
> >>> > breaking the build, and some jobs are failing regularly
> (check-merged)
> >>> > and I don't want to wait for it.
> >>>
> >>> Either disable the jobs or fix them. Having jobs consitently failing
> and
> >>> just ignore them is just a waste of resources.
> >>>
> >>> >
> >>> > Today we can override the CI vote and commit, if we keep it as is I
> >>> > don't
> >>> > see any problem with this change.
> >>> >
> >>> > Nir
> >>> > ___
> >>> > Devel mailing list
> >>> > Devel@ovirt.org
> >>> > http://lists.ovirt.org/mailman/listinfo/devel
> >>>
> >>>
> >>> ___
> >>> Devel mailing list
> >>> Devel@ovirt.org
> >>> http://lists.ovirt.org/mailman/listinfo/devel
> >>
> >>
> >>
> >> ___
> >> Devel mailing list
> >> Devel@ovirt.org
> >> http://lists.ovirt.org/mailman/listinfo/devel
> >
> >
> > ___
> > Devel mailing list
> > Devel@ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/devel
>
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Merge gating in Gerrit

[Martin Sivak]

Will it also auto-rename the database scripts? Please please!

Martin

On Mon, Nov 21, 2016 at 12:40 PM, Eyal Edri  wrote:
> This isn't gating.
> Just trigger to run more heavy lifting CI jobs,  the idea is to replace the
> manual submit with automatic system like Zuul.
>
>
> On Nov 21, 2016 1:32 PM, "Tal Nisan"  wrote:
>>
>> Why not use +1 on verified? That way the patch owner can wait till the
>> code review process is over, mark it as verified, wait for CI and then
>> submit.
>> It doesn't really give much added value to the code reviewers whether it's
>> marked as verified or not
>>
>> On Sun, Nov 20, 2016 at 10:26 PM, Sandro Bonazzola 
>> wrote:
>>>
>>> Il 20/Nov/2016 17:25, "Nir Soffer"  ha scritto:
>>> >
>>> > On Sun, Nov 20, 2016 at 5:39 PM, Yedidyah Bar David 
>>> > wrote:
>>> > > On Sun, Nov 20, 2016 at 5:06 PM, Barak Korren 
>>> > > wrote:
>>> > >> Hi all,
>>> > >>
>>> > >> Perhaps the main purpose of CI, is to prevent braking code from
>>> > >> getting merged into the stable/master branches. Unfortunately our CI
>>> > >> is not there yet, and one of the reasons for that is that we do
>>> > >> large
>>> > >> amount of our CI tests only _after_ the code is merged.
>>> > >>
>>> > >> The reason for that is that when balancing through, but time
>>> > >> consuming, tests (e.g. enging build with all permutations) v.s.
>>> > >> faster
>>> > >> but more basic ones (e.g. "findbugs" and single permutation build),
>>> > >> we
>>> > >> typically choose the faster tests to be run per-patch-set and leave
>>> > >> the through testing to only be run post-merge.
>>> > >>
>>> > >> We'd like to change that and have the through tests also run before
>>> > >> merge. Ideally we would like to just hook stuff to the "submit"
>>> > >> button, but Gerrit doesn't allow one to do that easily. So instead
>>> > >> we'll need to adopt some kind of flag to indicate we want to submit
>>> > >> and have Jenkins
>>> > >> "click" the submit button on our behalf if tests pass.
>>> > >>
>>> > >> I see two options here:
>>> > >> 1. Use Code-Review+2 as the indicator to run "heavy" CI and merge.
>>> >
>>> > This is problematic. For example in vdsm we have 5 maintainers with
>>> > +2, and 4 maintainers with commit right, but only 2 are commenting
>>> > regularly.
>>> >
>>> > >> 2. Add an "approve" flag that maintainers can set to +1 (This is
>>> > >>what OpenStack is doing).
>>> >
>>> > This seems better.
>>> >
>>> > But there is another requirement - maintainer should be able to commit
>>> > even if jenkins fails. Sometimes the CI is broken, or there are flakey
>>> > tests
>>> > breaking the build, and some jobs are failing regularly (check-merged)
>>> > and I don't want to wait for it.
>>>
>>> Either disable the jobs or fix them. Having jobs consitently failing and
>>> just ignore them is just a waste of resources.
>>>
>>> >
>>> > Today we can override the CI vote and commit, if we keep it as is I
>>> > don't
>>> > see any problem with this change.
>>> >
>>> > Nir
>>> > ___
>>> > Devel mailing list
>>> > Devel@ovirt.org
>>> > http://lists.ovirt.org/mailman/listinfo/devel
>>>
>>>
>>> ___
>>> Devel mailing list
>>> Devel@ovirt.org
>>> http://lists.ovirt.org/mailman/listinfo/devel
>>
>>
>>
>> ___
>> Devel mailing list
>> Devel@ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/devel
>
>
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Merge gating in Gerrit

[Eyal Edri]

This isn't gating.
Just trigger to run more heavy lifting CI jobs,  the idea is to replace the
manual submit with automatic system like Zuul.

On Nov 21, 2016 1:32 PM, "Tal Nisan"  wrote:

> Why not use +1 on verified? That way the patch owner can wait till the
> code review process is over, mark it as verified, wait for CI and then
> submit.
> It doesn't really give much added value to the code reviewers whether it's
> marked as verified or not
>
> On Sun, Nov 20, 2016 at 10:26 PM, Sandro Bonazzola 
> wrote:
>
>> Il 20/Nov/2016 17:25, "Nir Soffer"  ha scritto:
>> >
>> > On Sun, Nov 20, 2016 at 5:39 PM, Yedidyah Bar David 
>> wrote:
>> > > On Sun, Nov 20, 2016 at 5:06 PM, Barak Korren 
>> wrote:
>> > >> Hi all,
>> > >>
>> > >> Perhaps the main purpose of CI, is to prevent braking code from
>> > >> getting merged into the stable/master branches. Unfortunately our CI
>> > >> is not there yet, and one of the reasons for that is that we do large
>> > >> amount of our CI tests only _after_ the code is merged.
>> > >>
>> > >> The reason for that is that when balancing through, but time
>> > >> consuming, tests (e.g. enging build with all permutations) v.s.
>> faster
>> > >> but more basic ones (e.g. "findbugs" and single permutation build),
>> we
>> > >> typically choose the faster tests to be run per-patch-set and leave
>> > >> the through testing to only be run post-merge.
>> > >>
>> > >> We'd like to change that and have the through tests also run before
>> > >> merge. Ideally we would like to just hook stuff to the "submit"
>> > >> button, but Gerrit doesn't allow one to do that easily. So instead
>> > >> we'll need to adopt some kind of flag to indicate we want to submit
>> > >> and have Jenkins
>> > >> "click" the submit button on our behalf if tests pass.
>> > >>
>> > >> I see two options here:
>> > >> 1. Use Code-Review+2 as the indicator to run "heavy" CI and merge.
>> >
>> > This is problematic. For example in vdsm we have 5 maintainers with
>> > +2, and 4 maintainers with commit right, but only 2 are commenting
>> > regularly.
>> >
>> > >> 2. Add an "approve" flag that maintainers can set to +1 (This is
>> > >>what OpenStack is doing).
>> >
>> > This seems better.
>> >
>> > But there is another requirement - maintainer should be able to commit
>> > even if jenkins fails. Sometimes the CI is broken, or there are flakey
>> tests
>> > breaking the build, and some jobs are failing regularly (check-merged)
>> > and I don't want to wait for it.
>>
>> Either disable the jobs or fix them. Having jobs consitently failing and
>> just ignore them is just a waste of resources.
>>
>> >
>> > Today we can override the CI vote and commit, if we keep it as is I
>> don't
>> > see any problem with this change.
>> >
>> > Nir
>> > ___
>> > Devel mailing list
>> > Devel@ovirt.org
>> > http://lists.ovirt.org/mailman/listinfo/devel
>>
>> ___
>> Devel mailing list
>> Devel@ovirt.org
>> http://lists.ovirt.org/mailman/listinfo/devel
>>
>
>
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
>
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Merge gating in Gerrit

[Tal Nisan]

Why not use +1 on verified? That way the patch owner can wait till the code
review process is over, mark it as verified, wait for CI and then submit.
It doesn't really give much added value to the code reviewers whether it's
marked as verified or not

On Sun, Nov 20, 2016 at 10:26 PM, Sandro Bonazzola 
wrote:

> Il 20/Nov/2016 17:25, "Nir Soffer"  ha scritto:
> >
> > On Sun, Nov 20, 2016 at 5:39 PM, Yedidyah Bar David 
> wrote:
> > > On Sun, Nov 20, 2016 at 5:06 PM, Barak Korren 
> wrote:
> > >> Hi all,
> > >>
> > >> Perhaps the main purpose of CI, is to prevent braking code from
> > >> getting merged into the stable/master branches. Unfortunately our CI
> > >> is not there yet, and one of the reasons for that is that we do large
> > >> amount of our CI tests only _after_ the code is merged.
> > >>
> > >> The reason for that is that when balancing through, but time
> > >> consuming, tests (e.g. enging build with all permutations) v.s. faster
> > >> but more basic ones (e.g. "findbugs" and single permutation build), we
> > >> typically choose the faster tests to be run per-patch-set and leave
> > >> the through testing to only be run post-merge.
> > >>
> > >> We'd like to change that and have the through tests also run before
> > >> merge. Ideally we would like to just hook stuff to the "submit"
> > >> button, but Gerrit doesn't allow one to do that easily. So instead
> > >> we'll need to adopt some kind of flag to indicate we want to submit
> > >> and have Jenkins
> > >> "click" the submit button on our behalf if tests pass.
> > >>
> > >> I see two options here:
> > >> 1. Use Code-Review+2 as the indicator to run "heavy" CI and merge.
> >
> > This is problematic. For example in vdsm we have 5 maintainers with
> > +2, and 4 maintainers with commit right, but only 2 are commenting
> > regularly.
> >
> > >> 2. Add an "approve" flag that maintainers can set to +1 (This is
> > >>what OpenStack is doing).
> >
> > This seems better.
> >
> > But there is another requirement - maintainer should be able to commit
> > even if jenkins fails. Sometimes the CI is broken, or there are flakey
> tests
> > breaking the build, and some jobs are failing regularly (check-merged)
> > and I don't want to wait for it.
>
> Either disable the jobs or fix them. Having jobs consitently failing and
> just ignore them is just a waste of resources.
>
> >
> > Today we can override the CI vote and commit, if we keep it as is I don't
> > see any problem with this change.
> >
> > Nir
> > ___
> > Devel mailing list
> > Devel@ovirt.org
> > http://lists.ovirt.org/mailman/listinfo/devel
>
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
>
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Merge gating in Gerrit

[Sandro Bonazzola]

Il 20/Nov/2016 17:25, "Nir Soffer"  ha scritto:
>
> On Sun, Nov 20, 2016 at 5:39 PM, Yedidyah Bar David 
wrote:
> > On Sun, Nov 20, 2016 at 5:06 PM, Barak Korren 
wrote:
> >> Hi all,
> >>
> >> Perhaps the main purpose of CI, is to prevent braking code from
> >> getting merged into the stable/master branches. Unfortunately our CI
> >> is not there yet, and one of the reasons for that is that we do large
> >> amount of our CI tests only _after_ the code is merged.
> >>
> >> The reason for that is that when balancing through, but time
> >> consuming, tests (e.g. enging build with all permutations) v.s. faster
> >> but more basic ones (e.g. "findbugs" and single permutation build), we
> >> typically choose the faster tests to be run per-patch-set and leave
> >> the through testing to only be run post-merge.
> >>
> >> We'd like to change that and have the through tests also run before
> >> merge. Ideally we would like to just hook stuff to the "submit"
> >> button, but Gerrit doesn't allow one to do that easily. So instead
> >> we'll need to adopt some kind of flag to indicate we want to submit
> >> and have Jenkins
> >> "click" the submit button on our behalf if tests pass.
> >>
> >> I see two options here:
> >> 1. Use Code-Review+2 as the indicator to run "heavy" CI and merge.
>
> This is problematic. For example in vdsm we have 5 maintainers with
> +2, and 4 maintainers with commit right, but only 2 are commenting
> regularly.
>
> >> 2. Add an "approve" flag that maintainers can set to +1 (This is
> >>what OpenStack is doing).
>
> This seems better.
>
> But there is another requirement - maintainer should be able to commit
> even if jenkins fails. Sometimes the CI is broken, or there are flakey
tests
> breaking the build, and some jobs are failing regularly (check-merged)
> and I don't want to wait for it.

Either disable the jobs or fix them. Having jobs consitently failing and
just ignore them is just a waste of resources.

>
> Today we can override the CI vote and commit, if we keep it as is I don't
> see any problem with this change.
>
> Nir
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Merge gating in Gerrit

[Sandro Bonazzola]

Il 20/Nov/2016 16:06, "Barak Korren"  ha scritto:
>
> Hi all,
>
> Perhaps the main purpose of CI, is to prevent braking code from
> getting merged into the stable/master branches. Unfortunately our CI
> is not there yet, and one of the reasons for that is that we do large
> amount of our CI tests only _after_ the code is merged.
>
> The reason for that is that when balancing through, but time
> consuming, tests (e.g. enging build with all permutations) v.s. faster
> but more basic ones (e.g. "findbugs" and single permutation build), we
> typically choose the faster tests to be run per-patch-set and leave
> the through testing to only be run post-merge.
>
> We'd like to change that and have the through tests also run before
> merge.

Hopefully not the same tests ☺

Ideally we would like to just hook stuff to the "submit"
> button, but Gerrit doesn't allow one to do that easily. So instead
> we'll need to adopt some kind of flag to indicate we want to submit
> and have Jenkins
> "click" the submit button on our behalf if tests pass.
>
> I see two options here:
> 1. Use Code-Review+2 as the indicator to run "heavy" CI and merge.
> 2. Add an "approve" flag that maintainers can set to +1 (This is
>what OpenStack is doing).
>
> What would you prefer?

I would prefer to follow openstack example. Will help developers to have
same flow in both projects.

>
> --
> Barak Korren
> bkor...@redhat.com
> RHEV-CI Team
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Merge gating in Gerrit

[Nir Soffer]

On Sun, Nov 20, 2016 at 5:39 PM, Yedidyah Bar David  wrote:
> On Sun, Nov 20, 2016 at 5:06 PM, Barak Korren  wrote:
>> Hi all,
>>
>> Perhaps the main purpose of CI, is to prevent braking code from
>> getting merged into the stable/master branches. Unfortunately our CI
>> is not there yet, and one of the reasons for that is that we do large
>> amount of our CI tests only _after_ the code is merged.
>>
>> The reason for that is that when balancing through, but time
>> consuming, tests (e.g. enging build with all permutations) v.s. faster
>> but more basic ones (e.g. "findbugs" and single permutation build), we
>> typically choose the faster tests to be run per-patch-set and leave
>> the through testing to only be run post-merge.
>>
>> We'd like to change that and have the through tests also run before
>> merge. Ideally we would like to just hook stuff to the "submit"
>> button, but Gerrit doesn't allow one to do that easily. So instead
>> we'll need to adopt some kind of flag to indicate we want to submit
>> and have Jenkins
>> "click" the submit button on our behalf if tests pass.
>>
>> I see two options here:
>> 1. Use Code-Review+2 as the indicator to run "heavy" CI and merge.

This is problematic. For example in vdsm we have 5 maintainers with
+2, and 4 maintainers with commit right, but only 2 are commenting
regularly.

>> 2. Add an "approve" flag that maintainers can set to +1 (This is
>>what OpenStack is doing).

This seems better.

But there is another requirement - maintainer should be able to commit
even if jenkins fails. Sometimes the CI is broken, or there are flakey tests
breaking the build, and some jobs are failing regularly (check-merged)
and I don't want to wait for it.

Today we can override the CI vote and commit, if we keep it as is I don't
see any problem with this change.

Nir
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Merge gating in Gerrit

[Yedidyah Bar David]

On Sun, Nov 20, 2016 at 5:06 PM, Barak Korren  wrote:
> Hi all,
>
> Perhaps the main purpose of CI, is to prevent braking code from
> getting merged into the stable/master branches. Unfortunately our CI
> is not there yet, and one of the reasons for that is that we do large
> amount of our CI tests only _after_ the code is merged.
>
> The reason for that is that when balancing through, but time
> consuming, tests (e.g. enging build with all permutations) v.s. faster
> but more basic ones (e.g. "findbugs" and single permutation build), we
> typically choose the faster tests to be run per-patch-set and leave
> the through testing to only be run post-merge.
>
> We'd like to change that and have the through tests also run before
> merge. Ideally we would like to just hook stuff to the "submit"
> button, but Gerrit doesn't allow one to do that easily. So instead
> we'll need to adopt some kind of flag to indicate we want to submit
> and have Jenkins
> "click" the submit button on our behalf if tests pass.
>
> I see two options here:
> 1. Use Code-Review+2 as the indicator to run "heavy" CI and merge.
> 2. Add an "approve" flag that maintainers can set to +1 (This is
>what OpenStack is doing).
>
> What would you prefer?

(2.), and call it "Run heavy CI tests", and only do this and not
merge, so that one can ask to run these tests prior to merging.
-- 
Didi
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel