Re: [foreman-dev] [Infra] Proposal to Move Jenkins Job Configurations

[Ivan Necas]

so 2. 9. 2017 v 1:27 odesílatel Eric D Helms  napsal:

> Howdy,
>
> Some quick background, for those that don't know a majority of our Jenkins
> job configuration are stored in git. I find our Jenkins job configuration
> to be important for developers to be able to understand and contribute
> changes. However, today, these configurations are stored deep inside the
> foreman-infra [1] repository. I am proposing one of two ideas to bring
> these to the forefront:
>
>  1) Move them to the top of foreman-infra under a "jenkins" or "jobs"
> folder
>  2) Move them to their own repository (e.g. foreman-ci)
>

:+1: for both. I'm leaning towards 2)

-- Ivan


>
> [1]
> https://github.com/theforeman/foreman-infra/tree/master/puppet/modules/jenkins_job_builder/files/theforeman.org
>
> --
> Eric D. Helms
> Red Hat Engineering
>
> --
> You received this message because you are subscribed to the Google Groups
> "foreman-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to foreman-dev+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups 
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[foreman-dev] [infra] Packaging reorganization

[Eric D Helms]

Howdy,

As a lead-in to being working towards migrating Katello's packages to the
foreman-packaging repository, I'd like to propose a slight re-organization
of the repository. As well, to seek any other ideas or problems any might
see with the proposal.

Currently, the packaging repository is a flat structure with all packages
being represented by a directory containing sources and a spec file. When
looking at it, I find it hard to think about them in an organized manner
given we separate by repository into foreman and foreman-plugins (and
eventually katello repositories). Thus, my proposal is to let the packaging
repository reflect the repository organization by moving things into the
following directories:

foreman-packaging
   - foreman
   - plugins
   - katello
   - katello-client


Thoughts?

-- 
Eric D. Helms
Red Hat Engineering

-- 
You received this message because you are subscribed to the Google Groups 
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[foreman-dev] [Infra] Proposal to Move Jenkins Job Configurations

[Eric D Helms]

Howdy,

Some quick background, for those that don't know a majority of our Jenkins
job configuration are stored in git. I find our Jenkins job configuration
to be important for developers to be able to understand and contribute
changes. However, today, these configurations are stored deep inside the
foreman-infra [1] repository. I am proposing one of two ideas to bring
these to the forefront:

 1) Move them to the top of foreman-infra under a "jenkins" or "jobs" folder
 2) Move them to their own repository (e.g. foreman-ci)


[1]
https://github.com/theforeman/foreman-infra/tree/master/puppet/modules/jenkins_job_builder/files/theforeman.org

-- 
Eric D. Helms
Red Hat Engineering

-- 
You received this message because you are subscribed to the Google Groups 
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [foreman-dev] Enabling RHEL7 extra repository on koji

[Eric D Helms]

Works for me. Would only the foreman-nightly-rhel7 tag need this as an
external repository?

On Fri, Sep 1, 2017 at 8:40 AM, Lukas Zapletal  wrote:

> Hello,
>
> we have a long-standing bug in SELinux (#18284) - conflict between
> docker and foreman policies. In order to properly handle this, we need
> to have container-selinux present in the buildroot. The thing is this
> is RHEL7 extras repository, we only have base and optional.
>
> I would like to propose enabling this in the next maintenance window.
>
> --
> Later,
>   Lukas @lzap Zapletal
>
> --
> You received this message because you are subscribed to the Google Groups
> "foreman-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to foreman-dev+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>



-- 
Eric D. Helms
Red Hat Engineering

-- 
You received this message because you are subscribed to the Google Groups 
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [foreman-dev] [Infra] Jenkins Job Naming Conventions

[Ewoud Kohl van Wijngaarden]

Joining in on the party: +1 for #2

On Fri, Sep 01, 2017 at 09:29:17AM -0400, John Mitsch wrote:

Another vote for #2 as you can scan quickly and group by project.

John Mitsch
Red Hat Engineering
(860)-967-7285
irc: jomitsch

On Fri, Sep 1, 2017 at 8:56 AM, Justin Sherrill  wrote:


My vote would be #2, as it groups things by entity a bit nicer.  I don't
normally care to look at the release job across all projects, but would
want to look at all jobs across a particular project.

On 09/01/2017 08:43 AM, Eric D Helms wrote:

Howdy,

This is a bit of bike shedding perhaps, but as I start to re-write some of
our jobs into Jenkins pipeline in my spare time I'd like to establish and
follow some conventions along the way since this is a bit of a fresh start.
Two example PRs for those curious are at [1] [2].

The topic for this question is around job naming conventions. I see it as
two options:

 1) functionality - entity
   -- test-katello
   -- test-foreman
   -- release-katello
   -- test-foreman-pull-request
   -- test-foreman-tasks
 2) entity - functionality
   -- katello-test
   -- katello-release
   -- foreman-test
   -- foreman-release
   -- foreman-pull-request-test
   -- foreman-tasks-test


[1] https://github.com/theforeman/foreman-infra/pull/321
[2] https://github.com/theforeman/foreman-infra/pull/323


--
Eric D. Helms
Red Hat Engineering
--
You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


--
You received this message because you are subscribed to the Google Groups
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an
email to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.



--
You received this message because you are subscribed to the Google Groups 
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.


--
You received this message because you are subscribed to the Google Groups 
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [foreman-dev] [Infra] Jenkins Job Naming Conventions

[John Mitsch]

Another vote for #2 as you can scan quickly and group by project.

John Mitsch
Red Hat Engineering
(860)-967-7285
irc: jomitsch

On Fri, Sep 1, 2017 at 8:56 AM, Justin Sherrill  wrote:

> My vote would be #2, as it groups things by entity a bit nicer.  I don't
> normally care to look at the release job across all projects, but would
> want to look at all jobs across a particular project.
>
> On 09/01/2017 08:43 AM, Eric D Helms wrote:
>
> Howdy,
>
> This is a bit of bike shedding perhaps, but as I start to re-write some of
> our jobs into Jenkins pipeline in my spare time I'd like to establish and
> follow some conventions along the way since this is a bit of a fresh start.
> Two example PRs for those curious are at [1] [2].
>
> The topic for this question is around job naming conventions. I see it as
> two options:
>
>  1) functionality - entity
>-- test-katello
>-- test-foreman
>-- release-katello
>-- test-foreman-pull-request
>-- test-foreman-tasks
>  2) entity - functionality
>-- katello-test
>-- katello-release
>-- foreman-test
>-- foreman-release
>-- foreman-pull-request-test
>-- foreman-tasks-test
>
>
> [1] https://github.com/theforeman/foreman-infra/pull/321
> [2] https://github.com/theforeman/foreman-infra/pull/323
>
>
> --
> Eric D. Helms
> Red Hat Engineering
> --
> You received this message because you are subscribed to the Google Groups
> "foreman-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to foreman-dev+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "foreman-dev" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to foreman-dev+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

-- 
You received this message because you are subscribed to the Google Groups 
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [foreman-dev] [Infra] Jenkins Job Naming Conventions

[Justin Sherrill]

My vote would be #2, as it groups things by entity a bit nicer. I don't 
normally care to look at the release job across all projects, but would 
want to look at all jobs across a particular project.



On 09/01/2017 08:43 AM, Eric D Helms wrote:

Howdy,

This is a bit of bike shedding perhaps, but as I start to re-write 
some of our jobs into Jenkins pipeline in my spare time I'd like to 
establish and follow some conventions along the way since this is a 
bit of a fresh start. Two example PRs for those curious are at [1] [2].


The topic for this question is around job naming conventions. I see it 
as two options:


 1) functionality - entity
   -- test-katello
   -- test-foreman
   -- release-katello
   -- test-foreman-pull-request
   -- test-foreman-tasks
 2) entity - functionality
   -- katello-test
   -- katello-release
   -- foreman-test
   -- foreman-release
   -- foreman-pull-request-test
   -- foreman-tasks-test


[1] https://github.com/theforeman/foreman-infra/pull/321
[2] https://github.com/theforeman/foreman-infra/pull/323


--
Eric D. Helms
Red Hat Engineering
--
You received this message because you are subscribed to the Google 
Groups "foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send 
an email to foreman-dev+unsubscr...@googlegroups.com 
.

For more options, visit https://groups.google.com/d/optout.


--
You received this message because you are subscribed to the Google Groups 
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[foreman-dev] [Infra] Jenkins Job Naming Conventions

[Eric D Helms]

Howdy,

This is a bit of bike shedding perhaps, but as I start to re-write some of
our jobs into Jenkins pipeline in my spare time I'd like to establish and
follow some conventions along the way since this is a bit of a fresh start.
Two example PRs for those curious are at [1] [2].

The topic for this question is around job naming conventions. I see it as
two options:

 1) functionality - entity
   -- test-katello
   -- test-foreman
   -- release-katello
   -- test-foreman-pull-request
   -- test-foreman-tasks
 2) entity - functionality
   -- katello-test
   -- katello-release
   -- foreman-test
   -- foreman-release
   -- foreman-pull-request-test
   -- foreman-tasks-test


[1] https://github.com/theforeman/foreman-infra/pull/321
[2] https://github.com/theforeman/foreman-infra/pull/323


-- 
Eric D. Helms
Red Hat Engineering

-- 
You received this message because you are subscribed to the Google Groups 
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[foreman-dev] Enabling RHEL7 extra repository on koji

[Lukas Zapletal]

Hello,

we have a long-standing bug in SELinux (#18284) - conflict between
docker and foreman policies. In order to properly handle this, we need
to have container-selinux present in the buildroot. The thing is this
is RHEL7 extras repository, we only have base and optional.

I would like to propose enabling this in the next maintenance window.

-- 
Later,
  Lukas @lzap Zapletal

-- 
You received this message because you are subscribed to the Google Groups 
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [foreman-dev] Changing PR processor introduction text

[Greg Sutcliffe]

On Thu, 2017-08-31 at 13:55 +0200, Lukas Zapletal wrote:
> That's actually very good idea, because processor only comments when
> there is an issue and these are generic recommendation.
> 
> Greg, do you want to write down some nice proposal for such a
> template?

Yes I can do that. I'll add it to my list, thanks for the idea.

Side note, why are you CC'ing me? I'm on this list anyway :D

Greg

-- 
You received this message because you are subscribed to the Google Groups 
"foreman-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to foreman-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [foreman-dev] Test failures and merging PRs

[Ivan Necas]

On Fri, Sep 1, 2017 at 8:56 AM, Marek Hulán  wrote:
> Thanks Timo for your input. Please see my comment below in the text.
>
> On čtvrtek 31. srpna 2017 23:08:34 CEST Timo Goebel wrote:
>> Am 28.08.17 um 17:12 schrieb Marek Hulán:
>> > 1) codeclimate is red
>> >
>> > This can be ignored, we never agreed on using this as a hard metric for
>> > the
>> > PR. The motivation to introduce it was mainly to save some time to
>> > reviewer. We don't have to run it manually to get indications whether
>> > there's something introducing a big complexity [1]. From my experience it
>> > sometimes leads to worse code, since author splits the logic into more
>> > methods to lower e.g. cyclomatic complexity but it should be judged
>> > separately in every case.
>> +1
>> I like it as a suggestion, but sometimes it's just off and better be
>> ignored.
>>
>> > 2) foreman is red
>> >
>> > This can happen because of intermittent tests failures. If the PR is
>> > clearly not causing new ones and commiter is aware of this error, the PR
>> > is merged with message like "test unrelated" comments. If we are not
>> > sure, we retrigger the run,
>> >
>> > If Foreman develop branch is broken, we need to merge the PR to fix it so
>> > this is another exception. Usually we trigger the jenkins job manually
>> > first to see that the PR fixes the issue.
>>
>> +1
>> Yes, don't merge a PR with failing Foreman core tests.
>>
>> > 3) katello is red
>> >
>> > If katello becomes red only for this PR, we analyze what causes it.
>> > Usually it means that we change some internal things that have impact on
>> > Katello. In such case, we're doing our best to send a fixing PR to
>> > Katello or we ping someone with better knowledge in this area. We don't
>> > merge the PR until it's resolved, then usually we merge both parts at the
>> > same time.
>>
>> I think, this is totally unfair to all the "smaller" plugin maintainers
>> and that's why I vote for removing the test completely or just keep it
>> to test our public APIs.
>> I believe, we should do the following:
>> If the Foreman PR breaks some public API, e.g. facets, and the Katello
>> tests show that, my suggestion is to fix the foreman PR to not break the
>> public API and add proper depreciations if possible.
>> If we change something inside Foreman - in the past we changed the host
>> multiple actions from GET to POST or introduced strong parameters for
>> example - the contributor or maintainer should send a mail to
>> foreman-dev expaining what needs to be changed in plugins. I think it's
>> also a good idea to fix the example plugin or the How to create a plugin
>> wiki page if applicable.
>> However I think, it's the plugin maintainer's responsibility to make
>> sure his plugin works with Foreman. Everything else doesn't scale. In
>> the past a lot of "my" plugins broke because of changes to Foreman core.
>> Nobody cared to send a PR so far. But that's fine. I don't expect
>> anybody to. It's my job to test the plugin and fix it if it breaks.
>> I think, we should not block Foreman PRs because an additional parameter
>> was added to some internal method, just because Katello happens to
>> overwrite that method. It just doesn't make any sense to me why we
>> should do that for Katello but not for all the other plugins out there.
>> This is not against Katello, it's just the only plugin tested with
>> Foreman core right now.
>> Currently, we're developing a plugin to show system logfiles in Foreman.
>> That requires a complete ELK-stack for development. I would not expect
>> every developer to have that at hand.
>> If we leave Katello in the matrix, I think it would be totally fair to
>> also add our new plugin to Foreman's test matrix as well. But I wouldn't
>> want to block Foreman development just because some test in that plugin
>> breaks.
>> I know, Katello is important for RedHat and it's one of the larger
>> plugins. But that doesn't justify a special role in my opinion.
>
> I understand your feeling. A "justification" for me is that Katello is the
> largest plugin we have and therefore is much more prone to be broken by
> changes in Foreman. The more code you touch from plugin the higher the chance
> is that new core PR breaks something. Also I think for core it's a good way to
> find out what impacts our changes have. By testing Katello we get early notice
> about something that can impact all plugins. The PR author can consider
> whether there's less breaking way of doing the change.
>
> Having said that I still can understand that other plugin maintainers feel
> it's unfair. But instead of dropping Katello from matrix, I think the opposite
> approach would make more sense. I'd like to see many more plugins tested. I
> think plugin test sets are usually much smaller than core one, so it shouldn't
> take too much computation time.

+1 - we should probably skip running the foreman tests again with the plugin
in this matrix, as this is usually the longest time in the test run for pl

Re: [foreman-dev] Test failures and merging PRs

[Ivan Necas]

On Tue, Aug 29, 2017 at 11:25 AM, Daniel Lobato Garcia
 wrote:
> On 08/28, Marek Hulán wrote:
>> Hello devs,
>>
>> since there was a discussion on foreman-dev IRC channel recently about 
>> merging
>> PRs in Foreman core even if there's some build failed, I talked to few people
>> and decided to describe here what I think is current way of how it works. I'm
>> also attaching one suggestion at the end that came up after the discussion.
>>
>> Please, add questions, comments or simple +1 so we all know whether we're on
>> the same page.
>>
>> Core PR runs 7 checks - foreman, katello, codeclimate, hound, prprocessor,
>> upgrade, continuous-integration/travis-ci/pr. In ideal case they are all 
>> green
>> and after review, the PR is merged. There are several cases where we can 
>> merge
>> even if the PR is red.
>>
>> 1) codeclimate is red
>>
>> This can be ignored, we never agreed on using this as a hard metric for the
>> PR. The motivation to introduce it was mainly to save some time to reviewer.
>> We don't have to run it manually to get indications whether there's something
>> introducing a big complexity [1]. From my experience it sometimes leads to
>> worse code, since author splits the logic into more methods to lower e.g.
>> cyclomatic complexity but it should be judged separately in every case.
>
> It should be taken care of whenever possible. Most of the times it's
> certainly right and notices typical problems like stupidly long classes,
> duplication, etc..
>
> https://codeclimate.com/github/rails/rails itself enforces it unless a
> maintainer vets the PR. (ad hominem fallacy, I know)
>
>>
>> 2) foreman is red
>>
>> This can happen because of intermittent tests failures. If the PR is clearly
>> not causing new ones and commiter is aware of this error, the PR is merged
>> with message like "test unrelated" comments. If we are not sure, we retrigger
>> the run,
>>
>> If Foreman develop branch is broken, we need to merge the PR to fix it so 
>> this
>> is another exception. Usually we trigger the jenkins job manually first to 
>> see
>> that the PR fixes the issue.
>
> Makes sense to me, generally. The only con I've seen since I started
> contributing is that few people care to fix intermittent tests, which
> caused the job to be red for weeks at times

The problem with intermittent issues, or broken builds that are not related
to the PR itself, is that it's not clear whether somebody is already
working on it or not.

For intermittent issues, would it make sense to track every such an
issue in redmine (we perhaps already do)
and add vote for it every time it occurs (not sure if one person can
vote multiple times for some issue).
The goal would be to take this issues as one input of planning and
making sure we put it on the iteration.

For the master failures, having a foreman-dev thread as soon as it
appears and either
inform that one is working on fixing that, or ask for somebody else to
look into it, could work.
Also irc status about that would be useful to know right away what's going on.

>
>>
>> 3) katello is red
>>
>> If katello becomes red only for this PR, we analyze what causes it. Usually 
>> it
>> means that we change some internal things that have impact on Katello. In 
>> such
>> case, we're doing our best to send a fixing PR to Katello or we ping someone
>> with better knowledge in this area. We don't merge the PR until it's 
>> resolved,
>> then usually we merge both parts at the same time.
>
> This sounds good. Ideally the contributor sends the patch to Katello, please.

+1

>>
>> If it turns out there are more PRs that are failing with same errors, we 
>> merge
>> PRs if we're sure they don't introduce new Katello failures. At this time,
>> we're not blocking merges until Katello is green again. (*) here the
>> suggestion applies
>
> I don't think this is fair. If the Katello job is red, it's our
> responsibility to bring it back to green. When the causes for the job to
> be red are unknown, merging more stuff in Foreman will certainly NOT
> make it easier to understand them. In fact it may just aggravate the
> problem. So I would say *no* - at least on PRs I'm reviewing, I'm not
> merging if Katello is red.

If it's clear the PR is not the reason for the failures, there should
not be a reason
for blocking the PR from merging. For the investigation, one needs to
find the first
occurrence of that error in the history, which should be enough to
find exact time
when it started happening.

>
>>
>> 4) upgrade is red
>>
>> this is very similar to katello job, if there's some issue in upgrade, we
>> should not merge the PR. I remember though, there was a time when we knew the
>> job is broken which fall under "known to be broken" category.
>
> Same as 3.

This sound more like broken master case for me.

>
>>
>> 5) There's no 5, all the rest must be green. Sometimes hound service does not
>> respond and remains in "running" state, then it's retriggered by the 
>> reviewer.
>> prprocessor and t