FreeIPA nightly tests as PRs

[Petr Vobornik]

Hi all,

Felipe made nightly testing working as PRs in freeipa main Git Hub repo.

e.g.:
https://github.com/freeipa/freeipa/pull/1624
https://github.com/freeipa/freeipa/pull/1623

This is good. We can see the results publicly in comparison to
"upstream Jenkins". I also believe that it will be more stable. But
time will tell it. A disadvantage is more noise in mail notifications,
but that is not that drastic.

Let's discuss some aspects. First one is definitions of the tests
(.freeipa-pr-ci.yaml) and how to extend them.

We currently use 3 definitions:
* gating, the definition in repo
* nightly master tests
* nighly rawhide tests

Gating definition is in repo. But both nightly are not.

What is the way how other team members can extend this definition?
Where are they kept?

Could we also put them into a repo?

What about the following solution:
1. create a directory with tests definitions, e.g. /ipatests/definitions
2. put these 3 files there
  - gating
  - nightly_master
  - nightly_rawhide
3.  make .freeipa-pr-ci.yaml a symlink to /ipatest/definitions/gating
4. for nightly testing do PR which would change only the symlink

Benefits:
- anybody can extend nightly test suite
- the nighlty test suite can be extended right away in a PR which adds the tests
- nightly tests won't fail on merge conflict when somebody extends
current gating spec

Possible alternative is to make it more complex and use something like
includes instead of symlinks, but that would need update of PR-CI and
I don't see a real benefit in comparison to the symlink solution.

-- 
Petr Vobornik

[Alexander Bokovoy]

On pe, 23 helmi 2018, Petr Vobornik via FreeIPA-devel wrote:
> Hi all,
>
> Felipe made nightly testing working as PRs in freeipa main Git Hub repo.
>
> e.g.:
> https://github.com/freeipa/freeipa/pull/1624
> https://github.com/freeipa/freeipa/pull/1623
>
> This is good. We can see the results publicly in comparison to
> "upstream Jenkins". I also believe that it will be more stable. But
> time will tell it. A disadvantage is more noise in mail notifications,
> but that is not that drastic.
>
> Let's discuss some aspects. First one is definitions of the tests
> (.freeipa-pr-ci.yaml) and how to extend them.
>
> We currently use 3 definitions:
> * gating, the definition in repo
> * nightly master tests
> * nighly rawhide tests
>
> Gating definition is in repo. But both nightly are not.
>
> What is the way how other team members can extend this definition?
> Where are they kept?
>
> Could we also put them into a repo? 
Yes, we can put them in the repo.

> What about the following solution:
> 1. create a directory with tests definitions, e.g. /ipatests/definitions
> 2. put these 3 files there
>   - gating
>   - nightly_master
>   - nightly_rawhide
> 3.  make .freeipa-pr-ci.yaml a symlink to /ipatest/definitions/gating
> 4. for nightly testing do PR which would change only the symlink
>
> Benefits:
> - anybody can extend nightly test suite
> - the nighlty test suite can be extended right away in a PR which adds the tests
> - nightly tests won't fail on merge conflict when somebody extends
> current gating spec
>
> Possible alternative is to make it more complex and use something like
> includes instead of symlinks, but that would need update of PR-CI and
> I don't see a real benefit in comparison to the symlink solution. 
I think
symlink approach should be fine (relative symlink, that is).

-- 
/ Alexander Bokovoy

[Rob Crittenden]

Alexander Bokovoy via FreeIPA-devel wrote:
>  On pe, 23 helmi 2018, Petr Vobornik via FreeIPA-devel wrote:
> > Hi all,
> >
> > Felipe made nightly testing working as PRs in freeipa main Git Hub repo.
> >
> > e.g.:
> > https://github.com/freeipa/freeipa/pull/1624
> > https://github.com/freeipa/freeipa/pull/1623
> >
> > This is good. We can see the results publicly in comparison to
> > "upstream Jenkins". I also believe that it will be more stable. But
> > time will tell it. A disadvantage is more noise in mail notifications,
> > but that is not that drastic.
> >
> > Let's discuss some aspects. First one is definitions of the tests
> > (.freeipa-pr-ci.yaml) and how to extend them.
> >
> > We currently use 3 definitions:
> > * gating, the definition in repo
> > * nightly master tests
> > * nighly rawhide tests
> >
> > Gating definition is in repo. But both nightly are not.
> >
> > What is the way how other team members can extend this definition?
> > Where are they kept?
> >
> > Could we also put them into a repo?
>  Yes, we can put them in the repo.
>  
> >
> > What about the following solution:
> > 1. create a directory with tests definitions, e.g. /ipatests/definitions
> > 2. put these 3 files there
> >  - gating
> >  - nightly_master
> >  - nightly_rawhide
> > 3.  make .freeipa-pr-ci.yaml a symlink to /ipatest/definitions/gating
> > 4. for nightly testing do PR which would change only the symlink
> >
> > Benefits:
> > - anybody can extend nightly test suite
> > - the nighlty test suite can be extended right away in a PR which adds
> > the tests
> > - nightly tests won't fail on merge conflict when somebody extends
> > current gating spec
> >
> > Possible alternative is to make it more complex and use something like
> > includes instead of symlinks, but that would need update of PR-CI and
> > I don't see a real benefit in comparison to the symlink solution.
>  I think symlink approach should be fine (relative symlink, that is).
>   
How would the symlinks work with concurrency?

What does a nightly/rawhide PR look like? Is that a one-off or something
that would be scheduled?

rob

[Alexander Bokovoy]

On pe, 23 helmi 2018, Rob Crittenden via FreeIPA-devel wrote:
> Alexander Bokovoy via FreeIPA-devel wrote:
> > On pe, 23 helmi 2018, Petr Vobornik via FreeIPA-devel wrote:
> >> Hi all,
> >>
> >> Felipe made nightly testing working as PRs in freeipa main Git Hub repo.
> >>
> >> e.g.:
> >> https://github.com/freeipa/freeipa/pull/1624
> >> https://github.com/freeipa/freeipa/pull/1623
> >>
> >> This is good. We can see the results publicly in comparison to
> >> "upstream Jenkins". I also believe that it will be more stable. But
> >> time will tell it. A disadvantage is more noise in mail notifications,
> >> but that is not that drastic.
> >>
> >> Let's discuss some aspects. First one is definitions of the tests
> >> (.freeipa-pr-ci.yaml) and how to extend them.
> >>
> >> We currently use 3 definitions:
> >> * gating, the definition in repo
> >> * nightly master tests
> >> * nighly rawhide tests
> >>
> >> Gating definition is in repo. But both nightly are not.
> >>
> >> What is the way how other team members can extend this definition?
> >> Where are they kept?
> >>
> >> Could we also put them into a repo?
> > Yes, we can put them in the repo.
> >
> >>
> >> What about the following solution:
> >> 1. create a directory with tests definitions, e.g. /ipatests/definitions
> >> 2. put these 3 files there
> >>  - gating
> >>  - nightly_master
> >>  - nightly_rawhide
> >> 3.  make .freeipa-pr-ci.yaml a symlink to /ipatest/definitions/gating
> >> 4. for nightly testing do PR which would change only the symlink
> >>
> >> Benefits:
> >> - anybody can extend nightly test suite
> >> - the nighlty test suite can be extended right away in a PR which adds
> >> the tests
> >> - nightly tests won't fail on merge conflict when somebody extends
> >> current gating spec
> >>
> >> Possible alternative is to make it more complex and use something like
> >> includes instead of symlinks, but that would need update of PR-CI and
> >> I don't see a real benefit in comparison to the symlink solution.
> > I think symlink approach should be fine (relative symlink, that is).
> >
>
> How would the symlinks work with concurrency? 
What concurrency? A PR CI run is:
 - git checkout of the pull request
 - run things in the checkout
There are no concurrent runners with different test definitions for the
same test run checkout.

> What does a nightly/rawhide PR look like? Is that a one-off or
> something
> that would be scheduled? 
Here are the changes from the current run:
https://github.com/freeipa/freeipa/pull/1624/files

Even in this state a simple sequence of
  - rebase against master
  - push the rebase to PR1624

performed nightly would cause this PR to re-run.

So, yes, someone needs to schedule this periodic rebase but that's all.

-- 
/ Alexander Bokovoy

[Robbie Harwood]

Petr Vobornik via FreeIPA-devel <freeipa-devel(a)lists.fedorahosted.org&gt;
writes:

>  Felipe made nightly testing working as PRs in freeipa main Git Hub
>  repo. 
Is there really not a better way to do this than spamming freeipa-devel
with two more PRs every day?

Travis has cronjob support; wouldn't this be a better fit there?  Why
does it need to be a PR?

Like I suspect many users, I will be muting these.

Thanks,
--Robbie

[Standa Laznicka]

On 02/23/2018 05:26 PM, Robbie Harwood via FreeIPA-devel wrote:
>  Petr Vobornik via FreeIPA-devel
> <freeipa-devel(a)lists.fedorahosted.org&gt;
>  writes:
>
> > Felipe made nightly testing working as PRs in freeipa main Git Hub
> > repo.
>  Is there really not a better way to do this than spamming freeipa-devel
>  with two more PRs every day? 
+1, it messes up the PR queue, too, either make it use the same PR or
use another repo. The current state is unbearable.

>  Travis has cronjob support; wouldn't this be a better fit there?  Why
>  does it need to be a PR? 
I second the opinion that github PR is just a bad place to place nightly
CI run results to and it already shows.

>  Like I suspect many users, I will be muting these.
>
>  Thanks,
>  --Robbie
>
>
>  _______________________________________________
>  FreeIPA-devel mailing list -- freeipa-devel(a)lists.fedorahosted.org
>  To unsubscribe send an email to freeipa-devel-leave(a)lists.fedorahosted.org 

-- 
Standa Láznička
A Red Hat person
PGP: 8B00 620A 713B 714E B4CB 4767 C98C 4149 36B1 A7F3

[Petr Vobornik]

Hi thanks for the feedback, comments inline

On Mon, Feb 26, 2018 at 8:59 AM, Standa Laznicka <slaznick(a)redhat.com&gt; wrote:
>  On 02/23/2018 05:26 PM, Robbie Harwood via FreeIPA-devel wrote:
>
>  Petr Vobornik via FreeIPA-devel <freeipa-devel(a)lists.fedorahosted.org&gt;
>  writes:
>
>  Felipe made nightly testing working as PRs in freeipa main Git Hub
>  repo.
>
>  Is there really not a better way to do this than spamming freeipa-devel
>  with two more PRs every day? 
I guess it can be optimized a bit so that notification mails are a bit
more useful. I agree that notification mails about opening and closing
PR don't bring any added value. Maybe it can be suppressed in
https://github.com/freeipa/freeipa-tools/tree/master/github-email-notific...

On the other hand, what is desired is to show results of nightly
testing to project maintainers and contributors. Because tests are
useful only if their results are visible and it drives actions.
Therefore I find beneficial to send a mail with test failures. This is
not working yet. A possible technical solution might be to create a
task depending on all other tasks which would get the results and
would create a comment with them thus creating a notification mail.

>  +1, it messes up the PR queue, too, either make it use the same PR or use
>  another repo. The current state is unbearable. 
I don't see a benefit in moving it to other repo. It would lose the
visibility and therefore would become half-useless.

Reusing PRs could be a way to limit some noise. But if I'm correct old
prs are shown back in the queue.  IMO the goal here should be to show
nightly testing on a first PR page. One way is to have fewer PRs ;).
Other optimization could be e.g. to have weekly nightly prs - meaning
one PR would be updated 5-7 times.

IMO it is also useless to run nightly testing over weekend as no one
is currently pushing any patches during the time.  So I'd test on
Friday evening and then on a Monday evening.

Could you describe in more details what you mean by "messes up the PR
queue" or "unbearable? These are quite hard words but their meaning is
not specific. Could be interpreted in many ways. It's good to express
feeling but it is hard to grasp. What is the effect it creates from
your perspective?

>  Travis has cronjob support; wouldn't this be a better fit there?  Why
>  does it need to be a PR? 
PR is a way how to show results and coordinate test runners.  The
testing infra used for it doesn't currently work without PR.  How it
works now: https://github.com/freeipa/freeipa-pr-ci/tree/master/doc I
have no idea how travis cron jobs work. But if it uses Travis infra
then it is a no-go as Travis cannot handle our load.

>  I second the opinion that github PR is just a bad place to place nightly CI
>  run results to and it already shows. 
Could you explain what you mean by "it already shows"?

>  Like I suspect many users, I will be muting these.
>
>  Thanks,
>  --Robbie 

-- 
Petr Vobornik

[Standa Laznicka]

On 02/26/2018 10:32 AM, Petr Vobornik wrote:
>  Hi thanks for the feedback, comments inline
>
>  On Mon, Feb 26, 2018 at 8:59 AM, Standa Laznicka <slaznick(a)redhat.com&gt; wrote:
> > On 02/23/2018 05:26 PM, Robbie Harwood via FreeIPA-devel wrote:
> >
> > Petr Vobornik via FreeIPA-devel <freeipa-devel(a)lists.fedorahosted.org&gt;
> > writes:
> >
> > Felipe made nightly testing working as PRs in freeipa main Git Hub
> > repo.
> >
> > Is there really not a better way to do this than spamming freeipa-devel
> > with two more PRs every day?
>  I guess it can be optimized a bit so that notification mails are a bit
>  more useful. I agree that notification mails about opening and closing
>  PR don't bring any added value. Maybe it can be suppressed in
>  https://github.com/freeipa/freeipa-tools/tree/master/github-email-notific...
>
>  On the other hand, what is desired is to show results of nightly
>  testing to project maintainers and contributors. Because tests are
>  useful only if their results are visible and it drives actions.
>  Therefore I find beneficial to send a mail with test failures. This is
>  not working yet. A possible technical solution might be to create a
>  task depending on all other tasks which would get the results and
>  would create a comment with them thus creating a notification mail.
>
> >
> > +1, it messes up the PR queue, too, either make it use the same PR or use
> > another repo. The current state is unbearable.
>  I don't see a benefit in moving it to other repo. It would lose the
>  visibility and therefore would become half-useless. 
I doubt the number of people
who actually looked at the logs of failing
tests exceeds 2. From a user perspective, the results are just hard to
view and understand because the UI of a github PR is not supposed to be
used to view results of a great number of complicated integration tests.
>  Reusing PRs could be a way to limit some noise. But if I'm correct old
>  prs are shown back in the queue.  IMO the goal here should be to show
>  nightly testing on a first PR page. One way is to have fewer PRs ;).
>  Other optimization could be e.g. to have weekly nightly prs - meaning
>  one PR would be updated 5-7 times. 
Sounds definitely better than what we have now,
although it removes the
possibility too view history.
>  IMO it is also useless to run nightly testing over weekend as no one
>  is currently pushing any patches during the time.  So I'd test on
>  Friday evening and then on a Monday evening.
>
>  Could you describe in more details what you mean by "messes up the PR
>  queue" or "unbearable? 
Of course - unbearable = people don't want to
deal with their mailboxes
being spammed; messes up the PR queue = creates unnecessary
pull requests (keeping in mind that the value of a PR is in new code).
>  These are quite hard words but their meaning is
>  not specific.Could be interpreted in many ways. It's good to express
>  feeling but it is hard to grasp. What is the effect it creates from
>  your perspective? 
^-
> >
> > Travis has cronjob support; wouldn't this be a better fit there?  Why
> > does it need to be a PR?
>  PR is a way how to show results and coordinate test runners.  The
>  testing infra used for it doesn't currently work without PR.  How it
>  works now: https://github.com/freeipa/freeipa-pr-ci/tree/master/doc I
>  have no idea how travis cron jobs work. But if it uses Travis infra
>  then it is a no-go as Travis cannot handle our load.
>
> >
> > I second the opinion that github PR is just a bad place to place nightly CI
> > run results to and it already shows.
>  Could you explain what you mean by "it already shows"? 
^- and also -v
> >
> > Like I suspect many users, I will be muting these.
> >
> > Thanks,
> > --Robbie
>  
-- 
Standa Láznička
A Red Hat person
PGP: 8B00 620A 713B 714E B4CB 4767 C98C 4149 36B1 A7F3

[Felipe Barreto]

On 02/26/2018 06:32 AM, Petr Vobornik wrote:
>  Hi thanks for the feedback, comments inline
>  
>  On Mon, Feb 26, 2018 at 8:59 AM, Standa Laznicka <slaznick(a)redhat.com&gt; wrote:
> > On 02/23/2018 05:26 PM, Robbie Harwood via FreeIPA-devel wrote:
> >
> > Petr Vobornik via FreeIPA-devel <freeipa-devel(a)lists.fedorahosted.org&gt;
> > writes:
> >
> > Felipe made nightly testing working as PRs in freeipa main Git Hub
> > repo.
> >
> > Is there really not a better way to do this than spamming freeipa-devel
> > with two more PRs every day?
>  
>  I guess it can be optimized a bit so that notification mails are a bit
>  more useful. I agree that notification mails about opening and closing
>  PR don't bring any added value. Maybe it can be suppressed in
>  https://github.com/freeipa/freeipa-tools/tree/master/github-email-notific...
>  
>  On the other hand, what is desired is to show results of nightly
>  testing to project maintainers and contributors. Because tests are
>  useful only if their results are visible and it drives actions.
>  Therefore I find beneficial to send a mail with test failures. This is
>  not working yet. A possible technical solution might be to create a
>  task depending on all other tasks which would get the results and
>  would create a comment with them thus creating a notification mail.
>  
> >
> >
> > +1, it messes up the PR queue, too, either make it use the same PR or use
> > another repo. The current state is unbearable.
>  
>  I don't see a benefit in moving it to other repo. It would lose the
>  visibility and therefore would become half-useless. 
Actually, if we want to use the PR CI resources, we can't move it to
another repo. The runners points to freeipa/freeipa repo.

PR CI was designed to watch the PRs list, so it would not be simple to
do not use this way of running tests.

IMHO the easiest way is:
- Run nightly PRs two or three times per week (Mon and Friday or Mon, 
Wed, Fry)
- Keep the PRs on freeipa/freeipa repo
- people create some filter in their email client if they don't want to
receive the notifications

As Petr already mentioned, it's important to keep them on GitHub because
it's easy to have a historic, it keep them public available and it
already uses all of our infrastructure. If we "hide" the results, we
would have the same problem that Jenkins has: it's not easy to see the
results.

> 
> Reusing PRs could be a way to limit some noise. But if I'm correct old
> prs are shown back in the queue.  IMO the goal here should be to show
> nightly testing on a first PR page. One way is to have fewer PRs ;).
> Other optimization could be e.g. to have weekly nightly prs - meaning
> one PR would be updated 5-7 times.
> 
> IMO it is also useless to run nightly testing over weekend as no one
> is currently pushing any patches during the time.  So I'd test on
> Friday evening and then on a Monday evening.
> 
> Could you describe in more details what you mean by "messes up the PR
> queue" or "unbearable? These are quite hard words but their meaning is
> not specific. Could be interpreted in many ways. It's good to express
> feeling but it is hard to grasp. What is the effect it creates from
> your perspective?
> 
>>
>>
>> Travis has cronjob support; wouldn't this be a better fit there?  Why
>> does it need to be a PR?
> 
> PR is a way how to show results and coordinate test runners.  The
> testing infra used for it doesn't currently work without PR.  How it
> works now: https://github.com/freeipa/freeipa-pr-ci/tree/master/doc I
> have no idea how travis cron jobs work. But if it uses Travis infra
> then it is a no-go as Travis cannot handle our load.
> 
>>
>>
>> I second the opinion that github PR is just a bad place to place nightly CI
>> run results to and it already shows.
> 
> Could you explain what you mean by "it already shows"?
> 
>>
>>
>> Like I suspect many users, I will be muting these.
>>
>> Thanks,
>> --Robbie
> 
>

[Felipe Barreto]

On 02/23/2018 09:02 AM, Petr Vobornik wrote:
>  Hi all,
>  
>  Felipe made nightly testing working as PRs in freeipa main Git Hub repo.
>  
>  e.g.:
>  https://github.com/freeipa/freeipa/pull/1624
>  https://github.com/freeipa/freeipa/pull/1623
>  
>  This is good. We can see the results publicly in comparison to
>  "upstream Jenkins". I also believe that it will be more stable. But
>  time will tell it. A disadvantage is more noise in mail notifications,
>  but that is not that drastic.
>  
>  Let's discuss some aspects. First one is definitions of the tests
>  (.freeipa-pr-ci.yaml) and how to extend them.
>  
>  We currently use 3 definitions:
>  * gating, the definition in repo
>  * nightly master tests
>  * nighly rawhide tests
>  
>  Gating definition is in repo. But both nightly are not.
>  
>  What is the way how other team members can extend this definition?
>  Where are they kept?
>  
>  Could we also put them into a repo?
>  
>  What about the following solution:
>  1. create a directory with tests definitions, e.g. /ipatests/definitions
>  2. put these 3 files there
>     - gating
>     - nightly_master
>     - nightly_rawhide
>  3.  make .freeipa-pr-ci.yaml a symlink to /ipatest/definitions/gating
>  4. for nightly testing do PR which would change only the symlink
>   
Sounds a good solution for me. We could go that way. This would
require a small change in the github/openclose_pr script, but that's
easy.

> Benefits:
> - anybody can extend nightly test suite
> - the nighlty test suite can be extended right away in a PR which adds the tests
> - nightly tests won't fail on merge conflict when somebody extends
> current gating spec
> 
> Possible alternative is to make it more complex and use something like
> includes instead of symlinks, but that would need update of PR-CI and
> I don't see a real benefit in comparison to the symlink solution.
>