Re: [ovirt-devel] Gerrit parallel patch handling and CI (Or, why did my code fail post-merge)

[Barak Korren]

On 1 December 2016 at 19:19, Vojtech Szocs  wrote:
>
> Thanks for sharing those links.
>
> I didn't know you're already working on adopting Zuul, my bad =)

It ok, I guess oVirt Jira is not anyone's favorite reading material ;)

> So the maintainer can simply express his/her intent to merge the given
> patch, and CI infra takes care of the rest (run heavy tests and submit
> changes if successful).

Yep that is where we want to be.

> I'd be cautious with this feature, since our heavy CI tests involve
> GWT compilation, so Zuul trying to run more tests (on different patch
> combinations) = more time spent.

Its not something that Zuul allows you to heavily customize - and I am
hoping we will manage to get more efficient with the builds. Besides -
looking at a common scenario now - where the maintainer merges a set
of patches before leaving the office for the night, having the CI
system crunch all night and bring results in the morning is not such a
bad thing. Way better then finding out which patch broke the
experimental flow 3 days later.

> We cannot rule out issues that might happen in future. There will be
> flaky/broken tests or CI infra issues, we need to decide how to deal
> with those, I think.

Hopefully we get to the point where they are rare.

> It was just an idea =) Zuul sounds more like a proper solution.

Just to make things clear - Zuul does not do any magic with Gerrit's
"submit" button - one needs to use a flag instead.


-- 
Barak Korren
bkor...@redhat.com
RHCE, RHCi, RHV-DevOps Team
https://ifireball.wordpress.com/
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Gerrit parallel patch handling and CI (Or, why did my code fail post-merge)

[Vojtech Szocs]

- Original Message -
> From: "Barak Korren" <bkor...@redhat.com>
> To: "Vojtech Szocs" <vsz...@redhat.com>
> Cc: "devel" <devel@ovirt.org>
> Sent: Thursday, December 1, 2016 9:10:19 AM
> Subject: Re: [ovirt-devel] Gerrit parallel patch handling and CI (Or, why did 
> my code fail post-merge)
> 
> On 30 November 2016 at 19:12, Vojtech Szocs <vsz...@redhat.com> wrote:
> >
> > So before we adopt auto-submit-after-ci-pass (e.g. using Zuul, as Eyal
> > mentioned in that thread), we should be able to manually trigger heavy
> > CI (check-merged) job from Gerrit web interface, is that correct?
> 
> We could make it a two-step operation where you indicate you want to
> submit, wait for the CI output and then click submit - but why not
> just let the system submit for you?
> 
> One road I don't think we should go down is to have 3 CI stages:
> "light" check_patch, "heavy" check_patch and "pre-merge", this looks
> like a design smell to me.
> 
> WRT Zuul:
> 
> Unless you are using a strict policy like ff-only (which so far seems
> no to scale, because it is essentially a GIL on merging), Zuul is
> essential in order to test the right code in CI (even just for basic
> check_patch testing). So we are working to bring it in (See OVIRT-734
> [1] and OVIRT-882 [2]).

Thanks for sharing those links.

I didn't know you're already working on adopting Zuul, my bad =)

So the maintainer can simply express his/her intent to merge the given
patch, and CI infra takes care of the rest (run heavy tests and submit
changes if successful).

> 
> WRT merge gating - Zuul tries to make the process "fire and forget" as
> far as maintainers are concerned - if failures are found, it actually
> tries out different combinations of patches to see if, from a set of
> patches that were asked to be merged, some could be merged even when
> others could not.

I'd be cautious with this feature, since our heavy CI tests involve
GWT compilation, so Zuul trying to run more tests (on different patch
combinations) = more time spent.

> 
> > If so, it would be the patch owner's responsibility to submit (merge)
> > only after heavy CI (check-merged) pass?
> 
> The question hiding here is wither a maintainer could submit _without_
> passing the "heavy" CI. I'm guessing some maintainers will demand
> that, but I'm hoping in the long run this will not be needed and you
> will know that once submitted, a patch will be eventually merged
> unless it has real issues in it.

We cannot rule out issues that might happen in future. There will be
flaky/broken tests or CI infra issues, we need to decide how to deal
with those, I think.

> 
> > So we could actually write Gerrit plugin using MergeValidationListener
> > that would operate in the following way:
> > ...snip...
> 
> Just because something could be written doesn't mean it should - we
> are already stretched too thinly over too much code in the CI system,
> we do not want to maintain any more of it, certainly not in a core
> component like Gerrit.

It was just an idea =) Zuul sounds more like a proper solution.

> 
> Then again, this is a prioritization question - it not up to me to decide.
> 
> [1]: https://ovirt-jira.atlassian.net/browse/OVIRT-734
> [2]: https://ovirt-jira.atlassian.net/browse/OVIRT-882
> 
> --
> Barak Korren
> bkor...@redhat.com
> RHCE, RHCi, RHV-DevOps Team
> https://ifireball.wordpress.com/
> 
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Gerrit parallel patch handling and CI (Or, why did my code fail post-merge)

[Barak Korren]

On 1 December 2016 at 10:44, Eyal Edri  wrote:

...snip...

>> We could make it a two-step operation where you indicate you want to
>> submit, wait for the CI output and then click submit - but why not
>> just let the system submit for you?

...snip...

> Another option is not to do it from Gerrit, but via a 'developer' job that
> will be able
> to run "heavy jobs" on demand, basically we should be able to utilize the
> same
> flow Zuul will run, only instead of basic sanity, run tier1/2 of testing and
> not publish RPMs at the end.
>
> This is more complicated than it sounds ( at least to what we knew about
> developer jobs in the past),
> So we'll learn more about this as we move forward with the gating project.

This is just like the "3 CI stages" I described above, just without
placing the button in Gerrit - I still don't think we should have this
(Better to aim for CI to be fast and reliable enough).


-- 
Barak Korren
bkor...@redhat.com
RHCE, RHCi, RHV-DevOps Team
https://ifireball.wordpress.com/
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Gerrit parallel patch handling and CI (Or, why did my code fail post-merge)

[Eyal Edri]

On Thu, Dec 1, 2016 at 10:10 AM, Barak Korren  wrote:

> On 30 November 2016 at 19:12, Vojtech Szocs  wrote:
> >
> > So before we adopt auto-submit-after-ci-pass (e.g. using Zuul, as Eyal
> > mentioned in that thread), we should be able to manually trigger heavy
> > CI (check-merged) job from Gerrit web interface, is that correct?
>
> We could make it a two-step operation where you indicate you want to
> submit, wait for the CI output and then click submit - but why not
> just let the system submit for you?
>
> One road I don't think we should go down is to have 3 CI stages:
> "light" check_patch, "heavy" check_patch and "pre-merge", this looks
> like a design smell to me.
>
> WRT Zuul:
>
> Unless you are using a strict policy like ff-only (which so far seems
> no to scale, because it is essentially a GIL on merging), Zuul is
> essential in order to test the right code in CI (even just for basic
> check_patch testing). So we are working to bring it in (See OVIRT-734
> [1] and OVIRT-882 [2]).
>
> WRT merge gating - Zuul tries to make the process "fire and forget" as
> far as maintainers are concerned - if failures are found, it actually
> tries out different combinations of patches to see if, from a set of
> patches that were asked to be merged, some could be merged even when
> others could not.
>
> > If so, it would be the patch owner's responsibility to submit (merge)
> > only after heavy CI (check-merged) pass?
>
> The question hiding here is wither a maintainer could submit _without_
> passing the "heavy" CI. I'm guessing some maintainers will demand
> that, but I'm hoping in the long run this will not be needed and you
> will know that once submitted, a patch will be eventually merged
> unless it has real issues in it.
>

Another option is not to do it from Gerrit, but via a 'developer' job that
will be able
to run "heavy jobs" on demand, basically we should be able to utilize the
same
flow Zuul will run, only instead of basic sanity, run tier1/2 of testing
and not publish RPMs at the end.

This is more complicated than it sounds ( at least to what we knew about
developer jobs in the past),
So we'll learn more about this as we move forward with the gating project.


>
> > So we could actually write Gerrit plugin using MergeValidationListener
> > that would operate in the following way:
> > ...snip...
>
> Just because something could be written doesn't mean it should - we
> are already stretched too thinly over too much code in the CI system,
> we do not want to maintain any more of it, certainly not in a core
> component like Gerrit.
>
> Then again, this is a prioritization question - it not up to me to decide.
>
> [1]: https://ovirt-jira.atlassian.net/browse/OVIRT-734
> [2]: https://ovirt-jira.atlassian.net/browse/OVIRT-882
>
> --
> Barak Korren
> bkor...@redhat.com
> RHCE, RHCi, RHV-DevOps Team
> https://ifireball.wordpress.com/
> ___
> Devel mailing list
> Devel@ovirt.org
> http://lists.ovirt.org/mailman/listinfo/devel
>



-- 
Eyal Edri
Associate Manager
RHV DevOps
EMEA ENG Virtualization R
Red Hat Israel

phone: +972-9-7692018
irc: eedri (on #tlv #rhev-dev #rhev-integ)
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Gerrit parallel patch handling and CI (Or, why did my code fail post-merge)

[Barak Korren]

On 30 November 2016 at 19:12, Vojtech Szocs  wrote:
>
> So before we adopt auto-submit-after-ci-pass (e.g. using Zuul, as Eyal
> mentioned in that thread), we should be able to manually trigger heavy
> CI (check-merged) job from Gerrit web interface, is that correct?

We could make it a two-step operation where you indicate you want to
submit, wait for the CI output and then click submit - but why not
just let the system submit for you?

One road I don't think we should go down is to have 3 CI stages:
"light" check_patch, "heavy" check_patch and "pre-merge", this looks
like a design smell to me.

WRT Zuul:

Unless you are using a strict policy like ff-only (which so far seems
no to scale, because it is essentially a GIL on merging), Zuul is
essential in order to test the right code in CI (even just for basic
check_patch testing). So we are working to bring it in (See OVIRT-734
[1] and OVIRT-882 [2]).

WRT merge gating - Zuul tries to make the process "fire and forget" as
far as maintainers are concerned - if failures are found, it actually
tries out different combinations of patches to see if, from a set of
patches that were asked to be merged, some could be merged even when
others could not.

> If so, it would be the patch owner's responsibility to submit (merge)
> only after heavy CI (check-merged) pass?

The question hiding here is wither a maintainer could submit _without_
passing the "heavy" CI. I'm guessing some maintainers will demand
that, but I'm hoping in the long run this will not be needed and you
will know that once submitted, a patch will be eventually merged
unless it has real issues in it.

> So we could actually write Gerrit plugin using MergeValidationListener
> that would operate in the following way:
> ...snip...

Just because something could be written doesn't mean it should - we
are already stretched too thinly over too much code in the CI system,
we do not want to maintain any more of it, certainly not in a core
component like Gerrit.

Then again, this is a prioritization question - it not up to me to decide.

[1]: https://ovirt-jira.atlassian.net/browse/OVIRT-734
[2]: https://ovirt-jira.atlassian.net/browse/OVIRT-882

-- 
Barak Korren
bkor...@redhat.com
RHCE, RHCi, RHV-DevOps Team
https://ifireball.wordpress.com/
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Gerrit parallel patch handling and CI (Or, why did my code fail post-merge)

[Vojtech Szocs]

- Original Message -
> From: "Barak Korren" <bkor...@redhat.com>
> To: "Vojtech Szocs" <vsz...@redhat.com>
> Cc: "devel" <devel@ovirt.org>
> Sent: Wednesday, November 30, 2016 9:20:27 AM
> Subject: Re: [ovirt-devel] Gerrit parallel patch handling and CI (Or, why did 
> my code fail post-merge)
> 
> On 29 November 2016 at 19:34, Vojtech Szocs <vsz...@redhat.com> wrote:
> >
> > Question: after the patch is submitted in Gerrit (it's fully acked
> > and maintainer hits the "Submit" button), does Gerrit allow us to
> > run CI (e.g. `check-merged` script) *before* doing the actual merge?
> >
> > [In other words, does Gerrit support gating merge based on CI script?]
> 
> No.
> See separate thread about merge-gateing:
> http://lists.ovirt.org/pipermail/devel/2016-November/014192.html

Thanks, I've missed that thread.

So before we adopt auto-submit-after-ci-pass (e.g. using Zuul, as Eyal
mentioned in that thread), we should be able to manually trigger heavy
CI (check-merged) job from Gerrit web interface, is that correct?

If so, it would be the patch owner's responsibility to submit (merge)
only after heavy CI (check-merged) pass?

As for Gerrit's "Submit" button hook, there's "pre-merge validation":

  
https://gerrit-review.googlesource.com/Documentation/config-validation.html#pre-merge-validation
  
https://github.com/gerrit-review/gerrit/blob/master/gerrit-server/src/main/java/com/google/gerrit/server/git/validators/MergeValidationListener.java

This validation API (MergeValidationListener) doesn't seem async as it
relies on exception (MergeValidationException) to block the operation.

So we could actually write Gerrit plugin using MergeValidationListener
that would operate in the following way:

1, on first "Submit" button click, tell Jenkins CI to run check-merged
   and block merge (e.g. providing reason like "wait till CI finishes")
2, Jenkins CI is expected to set some flag on patch once it finishes
3, on next "Submit" button click, block unless flag has "ok" value

> 
> > That said, +1 on "Fast Forward Only" submit type.
> 
> So far only VDSM stepped forward to try it out - and it proved to be
> too cumbersome for VDSM maintainers - so switched back to "Merge if
> Necessary".
> 
> --
> Barak Korren
> bkor...@redhat.com
> RHCE, RHCi, RHV-DevOps Team
> https://ifireball.wordpress.com/
> 
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Gerrit parallel patch handling and CI (Or, why did my code fail post-merge)

[Barak Korren]

On 29 November 2016 at 19:34, Vojtech Szocs  wrote:
>
> Question: after the patch is submitted in Gerrit (it's fully acked
> and maintainer hits the "Submit" button), does Gerrit allow us to
> run CI (e.g. `check-merged` script) *before* doing the actual merge?
>
> [In other words, does Gerrit support gating merge based on CI script?]

No.
See separate thread about merge-gateing:
http://lists.ovirt.org/pipermail/devel/2016-November/014192.html

> That said, +1 on "Fast Forward Only" submit type.

So far only VDSM stepped forward to try it out - and it proved to be
too cumbersome for VDSM maintainers - so switched back to "Merge if
Necessary".

-- 
Barak Korren
bkor...@redhat.com
RHCE, RHCi, RHV-DevOps Team
https://ifireball.wordpress.com/
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Gerrit parallel patch handling and CI (Or, why did my code fail post-merge)

[Barak Korren]

>
> Let us now try the more lenient "rebase if needed", please.

Moved.


-- 
Barak Korren
bkor...@redhat.com
RHCE, RHCi, RHV-DevOps Team
https://ifireball.wordpress.com/
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Gerrit parallel patch handling and CI (Or, why did my code fail post-merge)

[Dan Kenigsberg]

On Mon, Nov 21, 2016 at 03:55:46PM +0200, Barak Korren wrote:
> >
> > I enjoyed the freedom of cherry-pick, but after 2 broken nightly builds
> > in the span of 10 days, I give up. Let's try ff-only.
> >
> All right, changed.

Thanks. But it ruined my work experience. Manual rebase for every patch,
sometimes under a race condition from Nir causes delays and at least
once made me miss an important merge until the morning after.

Let us now try the more lenient "rebase if needed", please.

Reagards,
Dan.
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Gerrit parallel patch handling and CI (Or, why did my code fail post-merge)

[Barak Korren]

>
> I enjoyed the freedom of cherry-pick, but after 2 broken nightly builds
> in the span of 10 days, I give up. Let's try ff-only.
>
All right, changed.


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

Re: [ovirt-devel] Gerrit parallel patch handling and CI (Or, why did my code fail post-merge)

[Dan Kenigsberg]

On Sun, Nov 20, 2016 at 06:09:59PM +0200, Nir Soffer wrote:
> On Sun, Nov 20, 2016 at 5:12 PM, Barak Korren  wrote:
> >> With the current setting (in vdsm), submitting a series of patches is
> >> a huge pain. Sometimes refreshing the page and submitting the next
> >> patch in the series works, but sometimes you have to rebase again
> >> the next patches in the series, and in the worst cases, you have to
> >> do several rebases in the same series. This when the entire series
> >> was already rebased properly before the submit.
> >
> > Actually vdsm is configured to "Cherry Pick" ATM, I'm not sure what
> > were the reasons for this, but this should probably be changed to
> > ff-only ASAP b/c as it is, it allows patches to be submitted
> > completely out-of-order.
> >
> >> In vdsm we were bitten by this many times, and both Dan and me agree
> >> now that fast-forward is the only way.
> >>
> >> I don't think we need to agree on all projects for this, the whole point
> >> of having multiple project is that we don't to agree on every little
> >> detail, the project maintainer can do whatever they want.
> >
> > Ok, so can we get an agreement between the vdsm maintainers to change
> > to "ff-only"?
> 
> +1
> 
> Dan, can you confirm?

I enjoyed the freedom of cherry-pick, but after 2 broken nightly builds
in the span of 10 days, I give up. Let's try ff-only.

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

Re: [ovirt-devel] Gerrit parallel patch handling and CI (Or, why did my code fail post-merge)

[Martin Perina]

On Sun, Nov 20, 2016 at 9:18 PM, Sandro Bonazzola 
wrote:

> Il 20/Nov/2016 15:08, "Barak Korren"  ha scritto:
> >
> > Hi there,
> >
> > I would like to address a concernt that had been raised to us by
> > multiple developers, and reach an agreement on how (and if)  to remedy
> > it.
> >
> > Lets assume the following situation:
> > We have a Git repo in Gerrit with top commit C0 in master.
> > On time t0 developers Alice and Bob push patches P1 and P2 respectively
> > to master so that we end up with the following situation in git:
> > C0 <= P1 (this is Alice`s patch)
> > C0 <= P2 (this is Bob`s patch)
> >
> > On time t1 CI runs for both patches checking the code as it looks for
> > each patch. Lets assume CI is successful for both.
> >
> > On time t2 Alice submits her patch and Gerrit merges it, resulting in
> > the following situation in master:
> > C0 <= P1
> >
> > On time t2 Bob submits his patch. Gerrit, seeing master has changed,
> > re-bases the patch and merges it, the resulting situation (If the
> > rebase is successful) is:
> > C0 <= P1 <= P2
> >
> > This means that the resulting code was never tested in CI. This, in
> > turn, causes various failures to show up post-merge despite having
> > pre-merge CI run successfully.
> >
> > This situation is a result of the way our repos are currently
> > configured. Most repos ATM are configured with the "Rebase If
> > Necessary" submit type. This means that Gerrit tries to automatically
> > rebase patches as mentioned in t2 above.
> >
> > We could, instead, configure the repos to use the "Fast Forward Only"
> > submit type. In that case, when Bob submits on t2, Gerrit refuses to
> > merge and asks Bob to rebase (While offering a convenient button to do
> > it). When he does, a new patch set gets pushed, and subsequently
> > checked by CI.
> >
> > I recommend we switch all projects to use the "Fast Forward Only" submit
> type.
> >
> > Thoughts? Concerns?
>

​AFAIR this was enabled for ovirt-engine project in the past and it was
pretty impossible to merge any patch with CI+1 when some important dates
were near (like feature freeze), because all maintainer tried to merge
patches and waited for CI to finish. Personally I'd say that current status
is OK, because it's a responsibility of a maintainer to check CI results of
a patch that he/she merged (and if error is raised then investigate the
issue and post a fix asap if needed).

So "Fast Forward Only" could successfully works for smaller projects, but I
don't think it will work for big projects like engine or vdsm.

+1 for me
>
> >
> > --
> > 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
>
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Gerrit parallel patch handling and CI (Or, why did my code fail post-merge)

[Sandro Bonazzola]

Il 20/Nov/2016 15:08, "Barak Korren"  ha scritto:
>
> Hi there,
>
> I would like to address a concernt that had been raised to us by
> multiple developers, and reach an agreement on how (and if)  to remedy
> it.
>
> Lets assume the following situation:
> We have a Git repo in Gerrit with top commit C0 in master.
> On time t0 developers Alice and Bob push patches P1 and P2 respectively
> to master so that we end up with the following situation in git:
> C0 <= P1 (this is Alice`s patch)
> C0 <= P2 (this is Bob`s patch)
>
> On time t1 CI runs for both patches checking the code as it looks for
> each patch. Lets assume CI is successful for both.
>
> On time t2 Alice submits her patch and Gerrit merges it, resulting in
> the following situation in master:
> C0 <= P1
>
> On time t2 Bob submits his patch. Gerrit, seeing master has changed,
> re-bases the patch and merges it, the resulting situation (If the
> rebase is successful) is:
> C0 <= P1 <= P2
>
> This means that the resulting code was never tested in CI. This, in
> turn, causes various failures to show up post-merge despite having
> pre-merge CI run successfully.
>
> This situation is a result of the way our repos are currently
> configured. Most repos ATM are configured with the "Rebase If
> Necessary" submit type. This means that Gerrit tries to automatically
> rebase patches as mentioned in t2 above.
>
> We could, instead, configure the repos to use the "Fast Forward Only"
> submit type. In that case, when Bob submits on t2, Gerrit refuses to
> merge and asks Bob to rebase (While offering a convenient button to do
> it). When he does, a new patch set gets pushed, and subsequently
> checked by CI.
>
> I recommend we switch all projects to use the "Fast Forward Only" submit
type.
>
> Thoughts? Concerns?

+1 for me

>
> --
> 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] Gerrit parallel patch handling and CI (Or, why did my code fail post-merge)

[Nir Soffer]

On Sun, Nov 20, 2016 at 5:12 PM, Barak Korren  wrote:
>> With the current setting (in vdsm), submitting a series of patches is
>> a huge pain. Sometimes refreshing the page and submitting the next
>> patch in the series works, but sometimes you have to rebase again
>> the next patches in the series, and in the worst cases, you have to
>> do several rebases in the same series. This when the entire series
>> was already rebased properly before the submit.
>
> Actually vdsm is configured to "Cherry Pick" ATM, I'm not sure what
> were the reasons for this, but this should probably be changed to
> ff-only ASAP b/c as it is, it allows patches to be submitted
> completely out-of-order.
>
>> In vdsm we were bitten by this many times, and both Dan and me agree
>> now that fast-forward is the only way.
>>
>> I don't think we need to agree on all projects for this, the whole point
>> of having multiple project is that we don't to agree on every little
>> detail, the project maintainer can do whatever they want.
>
> Ok, so can we get an agreement between the vdsm maintainers to change
> to "ff-only"?

+1

Dan, can you confirm?
___
Devel mailing list
Devel@ovirt.org
http://lists.ovirt.org/mailman/listinfo/devel

Re: [ovirt-devel] Gerrit parallel patch handling and CI (Or, why did my code fail post-merge)

[Barak Korren]

> With the current setting (in vdsm), submitting a series of patches is
> a huge pain. Sometimes refreshing the page and submitting the next
> patch in the series works, but sometimes you have to rebase again
> the next patches in the series, and in the worst cases, you have to
> do several rebases in the same series. This when the entire series
> was already rebased properly before the submit.

Actually vdsm is configured to "Cherry Pick" ATM, I'm not sure what
were the reasons for this, but this should probably be changed to
ff-only ASAP b/c as it is, it allows patches to be submitted
completely out-of-order.

> In vdsm we were bitten by this many times, and both Dan and me agree
> now that fast-forward is the only way.
>
> I don't think we need to agree on all projects for this, the whole point
> of having multiple project is that we don't to agree on every little
> detail, the project maintainer can do whatever they want.

Ok, so can we get an agreement between the vdsm maintainers to change
to "ff-only"?

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

Re: [ovirt-devel] Gerrit parallel patch handling and CI (Or, why did my code fail post-merge)

[Nir Soffer]

On Sun, Nov 20, 2016 at 4:07 PM, Barak Korren  wrote:
> Hi there,
>
> I would like to address a concernt that had been raised to us by
> multiple developers, and reach an agreement on how (and if)  to remedy
> it.
>
> Lets assume the following situation:
> We have a Git repo in Gerrit with top commit C0 in master.
> On time t0 developers Alice and Bob push patches P1 and P2 respectively
> to master so that we end up with the following situation in git:
> C0 <= P1 (this is Alice`s patch)
> C0 <= P2 (this is Bob`s patch)
>
> On time t1 CI runs for both patches checking the code as it looks for
> each patch. Lets assume CI is successful for both.
>
> On time t2 Alice submits her patch and Gerrit merges it, resulting in
> the following situation in master:
> C0 <= P1
>
> On time t2 Bob submits his patch. Gerrit, seeing master has changed,
> re-bases the patch and merges it, the resulting situation (If the
> rebase is successful) is:
> C0 <= P1 <= P2
>
> This means that the resulting code was never tested in CI.

This makes the CI useless.

To know if a patch actually passed the tests, you have to manually
rebase each patch and wait for the CI - this takes up to 20 minutes
on vdsm CI.

> This, in
> turn, causes various failures to show up post-merge despite having
> pre-merge CI run successfully.
>
> This situation is a result of the way our repos are currently
> configured. Most repos ATM are configured with the "Rebase If
> Necessary" submit type. This means that Gerrit tries to automatically
> rebase patches as mentioned in t2 above.
>
> We could, instead, configure the repos to use the "Fast Forward Only"
> submit type. In that case, when Bob submits on t2, Gerrit refuses to
> merge and asks Bob to rebase (While offering a convenient button to do
> it). When he does, a new patch set gets pushed, and subsequently
> checked by CI.
>
> I recommend we switch all projects to use the "Fast Forward Only" submit type.
>
> Thoughts? Concerns?

We have fast-forward in ioprocess and ovirt-imageio, and we are
happy with this setting.

Another advantage of fast-forward only merges is being able to submit
multiple patches with *one click*. If you submit the top patch in a series,
all patches are submitted.

With the current setting (in vdsm), submitting a series of patches is
a huge pain. Sometimes refreshing the page and submitting the next
patch in the series works, but sometimes you have to rebase again
the next patches in the series, and in the worst cases, you have to
do several rebases in the same series. This when the entire series
was already rebased properly before the submit.

In vdsm we were bitten by this many times, and both Dan and me agree
now that fast-forward is the only way.

I don't think we need to agree on all projects for this, the whole point
of having multiple project is that we don't to agree on every little
detail, the project maintainer can do whatever they want.

Thanks for raising this issue.

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