Re: [Gluster-infra] Zuul?
> > 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!! It doesn't seem that way. I'm not even trying to address the reasons or whether they're good; that's just an empirical observation. I'll also add that it's not that hard to have regressions pass in developer's own environment and fail in the "official" one if there's any difference at all - e.g. OS or compiler version, installed packages - between them. > I think that local testing is always a must. It is not like it is > difficult to run the tests in a VM. The run-tests-in-vagrant stuff makes this *super* easy. > 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. If it's a subset of tests - e.g. tests/basic - I'd say we might as well run them on the official infrastructure and go back to giving V+1 automatically. At least then we're sure of what was run. Otherwise, if it's a full set of tests, we have to overcome developer reluctance. ___ Gluster-infra mailing list Gluster-infra@gluster.org http://www.gluster.org/mailman/listinfo/gluster-infra
Re: [Gluster-infra] Zuul?
> > 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
[Gluster-infra] [Bug 1373454] SSL certs are missing
https://bugzilla.redhat.com/show_bug.cgi?id=1373454 --- Comment #3 from Nigel Babu --- Thanks to Kaushal, we noticed that it was running an older commit from July. Here's what we had as the config: scm: - git: branches: - $GERRIT_BRANCH refspec: $GERRIT_REFSPEC url: git://review.gluster.org/glusterfs.git wipe-workspace: false The problem part of this is `refspec`. I gave it an initial refspec (refs/heads/master) manually when I updated the job with JJB. My assumption was that it would just pull in the latest `refs/heads/master` each time the job ran. Turns out that's not how it works. I'm not sure if this is a Gerrit Trigger bug or a JJB bug, but I will investigate. I've changed the config to look like this now, which works: scm: - git: branches: - origin/master url: git://review.gluster.org/glusterfs.git wipe-workspace: false This should clear up the problems and I'll be monitoring this job for the next few days to confirm everything works. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=Sq0vf1sYDf&a=cc_unsubscribe ___ Gluster-infra mailing list Gluster-infra@gluster.org http://www.gluster.org/mailman/listinfo/gluster-infra
[Gluster-infra] [Bug 1373454] SSL certs are missing
https://bugzilla.redhat.com/show_bug.cgi?id=1373454 Nigel Babu changed: What|Removed |Added Status|NEW |CLOSED CC||nig...@redhat.com Resolution|--- |CURRENTRELEASE Last Closed||2016-09-06 07:00:28 --- Comment #2 from Nigel Babu --- This lead down a rabbit hole. Turns out we've been running this job against the same revision for ages. There was a bug in the job configuration which meant we were continuously running this job against the same revision over and over again. Entirely my fault. It was a very subtle bug in Gerrit + JJB combo. I've fixed this now so it always runs from HEAD of origin/master. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=5xOBEOo5zc&a=cc_unsubscribe ___ Gluster-infra mailing list Gluster-infra@gluster.org http://www.gluster.org/mailman/listinfo/gluster-infra
[Gluster-infra] [Bug 1373454] SSL certs are missing
https://bugzilla.redhat.com/show_bug.cgi?id=1373454 --- Comment #1 from Atin Mukherjee --- Refer to http://www.gluster.org/pipermail/maintainers/2016-September/001356.html for more details. -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=MI0uyhK8UG&a=cc_unsubscribe ___ Gluster-infra mailing list Gluster-infra@gluster.org http://www.gluster.org/mailman/listinfo/gluster-infra
[Gluster-infra] [Bug 1373454] New: SSL certs are missing
https://bugzilla.redhat.com/show_bug.cgi?id=1373454 Bug ID: 1373454 Summary: SSL certs are missing Product: GlusterFS Version: mainline Component: project-infrastructure Assignee: b...@gluster.org Reporter: amukh...@redhat.com CC: b...@gluster.org, gluster-infra@gluster.org Description of problem: SSL Certs are missing in slave28.cloud.gluster.org which leads to tests/bugs/cli/bug-1320388.t failing frequently. Version-Release number of selected component (if applicable): How reproducible: Steps to Reproduce: 1. 2. 3. Actual results: Expected results: Additional info: -- You are receiving this mail because: You are on the CC list for the bug. Unsubscribe from this bug https://bugzilla.redhat.com/token.cgi?t=wzZNL7band&a=cc_unsubscribe ___ Gluster-infra mailing list Gluster-infra@gluster.org http://www.gluster.org/mailman/listinfo/gluster-infra
Re: [Gluster-infra] Zuul?
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] [Gluster-devel] Gerrit Access Control
On Tue, Sep 06, 2016 at 12:32:42PM +0530, Nigel Babu wrote: > On Thu, Sep 01, 2016 at 12:43:06PM +0530, Nigel Babu wrote: > > > > Just need a clarification. Does a "commit in the last 90 days" means > > > > merging a patch sent by someone else by maintainer or maintainer > > > > sending a > > > > patch to be merged? > > > > > > > Your email needs to either be in Reviewed-By or Author in git log. So you > > either need to send patches or review patches. Ideally, I'm looking for > > activity on Gerrit and this is the easiest way to figure that out. Yes, I'm > > checking across all active branches. > > > > As an additional bonus, this will also give us a list of people who should > > be > > on the maintainers team, but aren't. > > > > > Interesting question. I was wondering about something similar as well. > > > What about commits/permissions for the different repositories we host on > > > Gerrit? Does each repository has its own maintainers, or is it one group > > > of maintainers that has merge permissions for all repos? > > > > > > > Each repo on Gerrit seems to mostly have it's own permissions. That's > > a sensible way to go about it. Some of them are unused a clean up is coming > > along, but that's later. > > I've answered everyone's concerns on this thread. If nobody is opposed to the > idea, shall I go ahead with this? If you mean "removing+emailing maintainers that are not active in Gerrit anymore", I guess that should be fine. However, before you do this, make sure the requirements to be counted as 'active' are included in our contributors guide. You can then easily add the link to the page in your emails. 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?
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] [Gluster-devel] Gerrit Access Control
On Thu, Sep 01, 2016 at 12:43:06PM +0530, Nigel Babu wrote: > > > Just need a clarification. Does a "commit in the last 90 days" means > > > merging a patch sent by someone else by maintainer or maintainer sending a > > > patch to be merged? > > > > Your email needs to either be in Reviewed-By or Author in git log. So you > either need to send patches or review patches. Ideally, I'm looking for > activity on Gerrit and this is the easiest way to figure that out. Yes, I'm > checking across all active branches. > > As an additional bonus, this will also give us a list of people who should be > on the maintainers team, but aren't. > > > Interesting question. I was wondering about something similar as well. > > What about commits/permissions for the different repositories we host on > > Gerrit? Does each repository has its own maintainers, or is it one group > > of maintainers that has merge permissions for all repos? > > > > Each repo on Gerrit seems to mostly have it's own permissions. That's > a sensible way to go about it. Some of them are unused a clean up is coming > along, but that's later. I've answered everyone's concerns on this thread. If nobody is opposed to the idea, shall I go ahead with this? -- nigelb ___ Gluster-infra mailing list Gluster-infra@gluster.org http://www.gluster.org/mailman/listinfo/gluster-infra
