Re: [Gluster-infra] Zuul?

[Jeff Darcy]

> > Our regression test suite allows running tests in a subdirectory, so
> > changes to the tests should not really be needed. In addition to the
> > simple smoke test, this might do:
> >
> >   # ./run-tests.sh tests/basic/
> 
> Perhaps, we could do build + smoke + tests/basic pre-merge so we catch *some*
> of the edge cases.

Looks good to me.  Personally, I think tests/basic is a decent approximation
of what smoke should be.  The non-build part of smoke rarely gives us useful
information.
___
Gluster-infra mailing list
Gluster-infra@gluster.org
http://www.gluster.org/mailman/listinfo/gluster-infra

Re: [Gluster-infra] Zuul?

[Niels de Vos]

On Mon, Sep 05, 2016 at 11:32:19PM -0400, Vijay Bellur wrote:
> On 09/04/2016 01:43 PM, Niels de Vos wrote:
> > On Fri, Sep 02, 2016 at 12:12:00PM -0400, Jeff Darcy wrote:
> > > > We already only merge after NetBSD-regression and CentOS-regression have
> > > > voted
> > > > back. All I'm changing is that you don't need to do the merge manually 
> > > > or do
> > > > Verified +1 for regression to run.. Zuul will run the tests after you 
> > > > get
> > > > Code-Review +2 and merge it for you with patches ordered correctly.
> > > 
> > > The problem is that some reviewers (including myself) might not even look 
> > > at
> > > a patch until it already has CentOS+1 and NetBSD+1.  Reviewing code, 
> > > having
> > > it fail regressions, reviewing a substantially new version, having *that*
> > > fail regressions, etc. tends to be very frustrating for both authors and
> > > reviewers.  Fighting with the regression tests *prior* to review can still
> > > be very frustrating for authors, but at least it doesn't frustrate 
> > > reviewers
> > > as much and doesn't contribute to author/reviewer animosity (apparently a
> > > real problem in this group) as much.
> > 
> > This is the main problem I see with this proposal too. There are quite
> > regular patches posted that break things in related regression tests.
> > Those corner cases can be very difficult to spot in code review, but
> > automated testing catches them. It is one of the reasons I prefer to do
> > code reviews for changes that are known to be (mostly) correct. Other
> > times changes (they should!) add test-cases, and these fail with the
> > initial versions...
> 
> Would it be possible to have a workflow where verified +1 vote from the
> developer indicates that the regression tests have passed in their local
> setup?

I *really* hope that is the case already!!

> If we can establish that protocol, then reviewers will have more
> confidence to look into such patches. Additionally authors will also have
> more motivation to run tests locally and fix obvious problems in our
> regression tests.

I think that local testing is always a must. It is not like it is
difficult to run the tests in a VM. I admit that I do not always run the
whole suite, but at minimal the tests for the component that is affected
by the change I'm going to post. I wait with Verified=+1 until I
actually did run the tests locally.

> I am in favor of switching to zuul and would welcome not burdening the
> regression infrastructure by running unqualified patches.

Agreed :)


signature.asc
Description: PGP signature
___
Gluster-infra mailing list
Gluster-infra@gluster.org
http://www.gluster.org/mailman/listinfo/gluster-infra

Re: [Gluster-infra] Zuul?

[Nigel Babu]

On Sun, Sep 04, 2016 at 07:43:04PM +0200, Niels de Vos wrote:
> On Fri, Sep 02, 2016 at 12:12:00PM -0400, Jeff Darcy wrote:
> > > We already only merge after NetBSD-regression and CentOS-regression have
> > > voted
> > > back. All I'm changing is that you don't need to do the merge manually or 
> > > do
> > > Verified +1 for regression to run.. Zuul will run the tests after you get
> > > Code-Review +2 and merge it for you with patches ordered correctly.
> >
> > The problem is that some reviewers (including myself) might not even look at
> > a patch until it already has CentOS+1 and NetBSD+1.  Reviewing code, having
> > it fail regressions, reviewing a substantially new version, having *that*
> > fail regressions, etc. tends to be very frustrating for both authors and
> > reviewers.  Fighting with the regression tests *prior* to review can still
> > be very frustrating for authors, but at least it doesn't frustrate reviewers
> > as much and doesn't contribute to author/reviewer animosity (apparently a
> > real problem in this group) as much.
>
> This is the main problem I see with this proposal too. There are quite
> regular patches posted that break things in related regression tests.
> Those corner cases can be very difficult to spot in code review, but
> automated testing catches them. It is one of the reasons I prefer to do
> code reviews for changes that are known to be (mostly) correct. Other
> times changes (they should!) add test-cases, and these fail with the
> initial versions...

This is a problem I hadn't realized we had. This makes it mostly a no-go to do
regressions after code review.

>
> > That said, it would be nice to have *something* as a gate between +2 and
> > merge - certainly a build, and at least a few basic tests (more than smoke
> > does IMO).  If Zuul can help us avoid broken builds due to improper merge
> > order, which seem to be the most common kind of broken builds, I'm all for
> > it.
>
> Our regression test suite allows running tests in a subdirectory, so
> changes to the tests should not really be needed. In addition to the
> simple smoke test, this might do:
>
>   # ./run-tests.sh tests/basic/

Perhaps, we could do build + smoke + tests/basic pre-merge so we catch *some*
of the edge cases.

>
> Niels



--
nigelb
___
Gluster-infra mailing list
Gluster-infra@gluster.org
http://www.gluster.org/mailman/listinfo/gluster-infra

Re: [Gluster-infra] Zuul?

[Niels de Vos]

On Fri, Sep 02, 2016 at 12:12:00PM -0400, Jeff Darcy wrote:
> > We already only merge after NetBSD-regression and CentOS-regression have
> > voted
> > back. All I'm changing is that you don't need to do the merge manually or do
> > Verified +1 for regression to run.. Zuul will run the tests after you get
> > Code-Review +2 and merge it for you with patches ordered correctly.
> 
> The problem is that some reviewers (including myself) might not even look at
> a patch until it already has CentOS+1 and NetBSD+1.  Reviewing code, having
> it fail regressions, reviewing a substantially new version, having *that*
> fail regressions, etc. tends to be very frustrating for both authors and
> reviewers.  Fighting with the regression tests *prior* to review can still
> be very frustrating for authors, but at least it doesn't frustrate reviewers
> as much and doesn't contribute to author/reviewer animosity (apparently a
> real problem in this group) as much.

This is the main problem I see with this proposal too. There are quite
regular patches posted that break things in related regression tests.
Those corner cases can be very difficult to spot in code review, but
automated testing catches them. It is one of the reasons I prefer to do
code reviews for changes that are known to be (mostly) correct. Other
times changes (they should!) add test-cases, and these fail with the
initial versions...

> That said, it would be nice to have *something* as a gate between +2 and
> merge - certainly a build, and at least a few basic tests (more than smoke
> does IMO).  If Zuul can help us avoid broken builds due to improper merge
> order, which seem to be the most common kind of broken builds, I'm all for
> it.

Our regression test suite allows running tests in a subdirectory, so
changes to the tests should not really be needed. In addition to the
simple smoke test, this might do:

  # ./run-tests.sh tests/basic/

Niels


signature.asc
Description: PGP signature
___
Gluster-infra mailing list
Gluster-infra@gluster.org
http://www.gluster.org/mailman/listinfo/gluster-infra

Re: [Gluster-infra] Zuul?

[Jeff Darcy]

> We already only merge after NetBSD-regression and CentOS-regression have
> voted
> back. All I'm changing is that you don't need to do the merge manually or do
> Verified +1 for regression to run.. Zuul will run the tests after you get
> Code-Review +2 and merge it for you with patches ordered correctly.

The problem is that some reviewers (including myself) might not even look at
a patch until it already has CentOS+1 and NetBSD+1.  Reviewing code, having
it fail regressions, reviewing a substantially new version, having *that*
fail regressions, etc. tends to be very frustrating for both authors and
reviewers.  Fighting with the regression tests *prior* to review can still
be very frustrating for authors, but at least it doesn't frustrate reviewers
as much and doesn't contribute to author/reviewer animosity (apparently a
real problem in this group) as much.

That said, it would be nice to have *something* as a gate between +2 and
merge - certainly a build, and at least a few basic tests (more than smoke
does IMO).  If Zuul can help us avoid broken builds due to improper merge
order, which seem to be the most common kind of broken builds, I'm all for
it.
___
Gluster-infra mailing list
Gluster-infra@gluster.org
http://www.gluster.org/mailman/listinfo/gluster-infra

Re: [Gluster-infra] Zuul?

[Nigel Babu]

On Fri, Sep 02, 2016 at 08:39:14AM -0400, Kaleb S. KEITHLEY wrote:
> On 09/02/2016 06:36 AM, Nigel Babu wrote:
> >
> > Here's the flow as I see it:
> > 1. Propose a review request.
> > 2. Smoke tests run as soon as a patchset is created or updated.
> > 3. Reviewer gives +2.
> > 4. Zuul will run regressions in the order than patches received Code Review 
> > +2.
> >If they pass regression, Zuul will merge them into the branch requested. 
> > The
> >documentation[1] has a good visualization that will help.
> > 5. If the regression fails, we can still do a retry. Zuul will retry the 
> > job on
> >top of the existing patch queue rather than in isolation.
> >
>
> A full _regression_?  Which takes hours? And is prone to spurious errors?
>
> Eek!
>
> Wouldn't a smoke, or just a compile (or rpmbuild) suffice for this?
> Don't we already have periodic (hourly) regressions of the head of
> master?  Maybe we should have the same for the release branches?
>
> --
>

We already only merge after NetBSD-regression and CentOS-regression have voted
back. All I'm changing is that you don't need to do the merge manually or do
Verified +1 for regression to run.. Zuul will run the tests after you get
Code-Review +2 and merge it for you with patches ordered correctly.

--
nigelb
___
Gluster-infra mailing list
Gluster-infra@gluster.org
http://www.gluster.org/mailman/listinfo/gluster-infra

Re: [Gluster-infra] Zuul?

[Sankarshan Mukhopadhyay]

On Fri, Sep 2, 2016 at 4:06 PM, Nigel Babu  wrote:
> 4. Zuul will run regressions in the order than patches received Code Review 
> +2.
>If they pass regression, Zuul will merge them into the branch requested. 
> The
>documentation[1] has a good visualization that will help.
> 5. If the regression fails, we can still do a retry. Zuul will retry the job 
> on
>top of the existing patch queue rather than in isolation.

Do we need to enhance/modify the existing regression tests as a
pre-requisite to adopting Zuul? It seems to me that this is a critical
topic which needs to be discussed prior to Zuul.




-- 
sankarshan mukhopadhyay

___
Gluster-infra mailing list
Gluster-infra@gluster.org
http://www.gluster.org/mailman/listinfo/gluster-infra

Re: [Gluster-infra] Zuul?

[Nigel Babu]

On Fri, Sep 02, 2016 at 03:40:39PM +0530, Sankarshan Mukhopadhyay wrote:
> On Fri, Sep 2, 2016 at 3:27 PM, Kaushal M  wrote:
> > I'd brought up Zuul a long while back. The opinion then was that,
> > while a gatekeeper is nice, we didn't want to maintain anymore infra
> > over what we had at the time. We tried to make Jenkins itself do the
> > work, which hasn't succeeded as well as we hoped.
> >
> > With you being dedicated to maintain the infra, this will be a nice
> > time to revisit/investigate Zuul again.
>
> I'd propose that concerns of maintenance/administration be separated
> from the value accrued by this move. This approach worked out well
> during the JJB task.
>
> So, a question for Nigel - when you propose Zuul - what is the flow
> and benefits that you see being available to the project? Have you
> previously worked with Zuul or, can cite situations where adoption of
> Zuul has helped?
>
>
> > On Fri, Sep 2, 2016 at 3:01 PM, Nigel Babu  wrote:
> >> Hello,
> >>
> >> We've had master breaking twice in this week because we of when we run
> >> regressions and how we merge. I think it's time we officially thought of 
> >> moving
> >> regressions as a gate controlld by Zuul. And Zuul will do the merge onto
> >> the correct branch.
> >>
> >> This is me throwing the idea about to hear any negative thoughts, before I 
> >> do
> >> further investigation. What does everyone think about this?
> >>
> >> Note: I've purposefully not CC'd gluster-devel here because I'd rather go 
> >> to
> >> the full developer team with a proper plan.
> >>
> >> --
> >> nigelb

Here's the flow as I see it:
1. Propose a review request.
2. Smoke tests run as soon as a patchset is created or updated.
3. Reviewer gives +2.
4. Zuul will run regressions in the order than patches received Code Review +2.
   If they pass regression, Zuul will merge them into the branch requested. The
   documentation[1] has a good visualization that will help.
5. If the regression fails, we can still do a retry. Zuul will retry the job on
   top of the existing patch queue rather than in isolation.

I've not worked with Zuul before, but I've talked to colleagues and friends who
work on Openstack. They've switched off Jenkins and completely moved to
Zuul[2]. The primary reason for my original conversation was to reduce friction
in how we do reviews and merge.

I see three key benefits

* Reduced friction for merging code.
* The tip of all branches will always be green.
* We'll only run regressions once the patch is final. Less wasted resources if
  the patch needs changing after code review.

[1]: http://docs.openstack.org/infra/zuul/gating.html#testing-in-parallel

--
nigelb
___
Gluster-infra mailing list
Gluster-infra@gluster.org
http://www.gluster.org/mailman/listinfo/gluster-infra

Re: [Gluster-infra] Zuul?

[Sankarshan Mukhopadhyay]

On Fri, Sep 2, 2016 at 3:27 PM, Kaushal M  wrote:
> I'd brought up Zuul a long while back. The opinion then was that,
> while a gatekeeper is nice, we didn't want to maintain anymore infra
> over what we had at the time. We tried to make Jenkins itself do the
> work, which hasn't succeeded as well as we hoped.
>
> With you being dedicated to maintain the infra, this will be a nice
> time to revisit/investigate Zuul again.

I'd propose that concerns of maintenance/administration be separated
from the value accrued by this move. This approach worked out well
during the JJB task.

So, a question for Nigel - when you propose Zuul - what is the flow
and benefits that you see being available to the project? Have you
previously worked with Zuul or, can cite situations where adoption of
Zuul has helped?


> On Fri, Sep 2, 2016 at 3:01 PM, Nigel Babu  wrote:
>> Hello,
>>
>> We've had master breaking twice in this week because we of when we run
>> regressions and how we merge. I think it's time we officially thought of 
>> moving
>> regressions as a gate controlld by Zuul. And Zuul will do the merge onto
>> the correct branch.
>>
>> This is me throwing the idea about to hear any negative thoughts, before I do
>> further investigation. What does everyone think about this?
>>
>> Note: I've purposefully not CC'd gluster-devel here because I'd rather go to
>> the full developer team with a proper plan.
>>
>> --
>> nigelb



-- 
sankarshan mukhopadhyay

___
Gluster-infra mailing list
Gluster-infra@gluster.org
http://www.gluster.org/mailman/listinfo/gluster-infra

Re: [Gluster-infra] Zuul?

[Kaushal M]

I'd brought up Zuul a long while back. The opinion then was that,
while a gatekeeper is nice, we didn't want to maintain anymore infra
over what we had at the time. We tried to make Jenkins itself do the
work, which hasn't succeeded as well as we hoped.

With you being dedicated to maintain the infra, this will be a nice
time to revisit/investigate Zuul again.






On Fri, Sep 2, 2016 at 3:01 PM, Nigel Babu  wrote:
> Hello,
>
> We've had master breaking twice in this week because we of when we run
> regressions and how we merge. I think it's time we officially thought of 
> moving
> regressions as a gate controlld by Zuul. And Zuul will do the merge onto
> the correct branch.
>
> This is me throwing the idea about to hear any negative thoughts, before I do
> further investigation. What does everyone think about this?
>
> Note: I've purposefully not CC'd gluster-devel here because I'd rather go to
> the full developer team with a proper plan.
>
> --
> nigelb
> ___
> Gluster-infra mailing list
> Gluster-infra@gluster.org
> http://www.gluster.org/mailman/listinfo/gluster-infra
___
Gluster-infra mailing list
Gluster-infra@gluster.org
http://www.gluster.org/mailman/listinfo/gluster-infra