PR testing and waiting for stable builds


The recent ugly issues with uglifier causing the Foreman’s develop branch CI test to be unstable showed some double standard we have in our CI infrastructure. While the plugin PRs are stuck on running CI for incoming PRs until their master builds are stable, the Foreman’s core tests are happily continuing to run.

While digging with my investigation further, I’ve noticed we actually don’t have settings for the Foreman PRs in the foreman-infra repository.

So the questions I have are:

  1. why the foreman PRs behave differently than the rest and where is the rest of the configuration?
  2. for the plugin and proxy PRs, what do you think about removing the check on stable builds?
  • Run tests always (change the current behaviour)
  • Wait for stable build of master (keep the current behaviour)

0 voters

@iNecas you give the pros/cons of each? I don’t see why it makes sense to run a build (on limited community resources) we know is going to fail.

The first question has to do with a technology shift in our CI that we are working to bring to every job. We kickoff PR jobs in two ways:

  1. pull_request_scanner
  2. Github Pull Request Builder

The former is our classic PR runner and had facilities to stop all PR testing if some base job that is specified is failing. This is the behavior we all are used to. The latter does not have this functionality. Any job that has been switched to GHPRB will exhibit this new behavior. Foreman core jobs were the first ones ever switched over. Since then we’ve switched a few others but not plugins yet. As far as I know, GHPRB has no functionality to mimic the blocking itself, but the job can be configured natively to block if another job is failing.

GHPRB requires less infrastructure and can be natively configured via JJB. It also has immediate kickoff and ability to set context.

For plugins and proxy, this is on our list to convert. So the option is either to wait till we convert in the next week or two or the configuration can be modified for the existing work.

I think this also raises the question: Do the post merge tests add any value at this point?

The main downside is:

  1. if the Foreman devel is red for any reason, the PRs on plugin are blocked. For some changes, that we are quite sure will not break the Foreman core side, and we are mainly interested that the plugin tests are green, we need to go though the complicated process of triggering the job manually

  2. if there is an intermittent issue on master job, we need to retrigger to unblock the PRs, which is just putting additional load on Jenkins

  3. if there is actually breakage on the master, one needs to trigger the jenkins on the PR fixing that manually

From my experience, most of the reasons for the red builds are 1. and 2, while only 3. would make sense to block on.

1 Like

My experience is that this causes 75% or more of the PR test failures that I see on my PRs. So much so that I often am tempted to simply do a [test ____] instead of actually looking at the result because odds are that it’s just some unrelated intermittent failure.

While I understand that making sure master job is green can be very annoying, I also see a positive outcome of that. It forces us to fix broken tests caused by rubocop upgrades and Foreman core changes. The fact that we need to run tests manually is a good driver for us to fix it asap. Good thing is, there’s still the option.

If we could find a way, that Foreman core master test would be taken into the condition whether to run plugin PR test, I think that would be ideal. If core is broken, run plugin PR tests regardless of plugin master. If plugin master is broken but core is green, don’t run plugin PR tests.

I think that just the tests on the PR are red due to things outside of the PR is good enough motivator. The problem with the current approach is that, when looking at last week incident, everyone knew something is broken, and people were actively working on fixing it, and it still was blocking numerous plugins development for a long time. I don’t agree the benefits overweight this.

While this might be ideal, I’m not sure it’s worth the effort compared to just not blocking it, and start tuning it, if it turns out that plugin tests keep getting red and everyone just ignoring it.