[JIRA] (JENKINS-56255) occasionally builds the branch for a PR also

[brian.murrell@intel.com (JIRA)]

Brian J Murrell created an issue

Jenkins / JENKINS-56255 occasionally builds the branch for a PR also Issue Type: Bug Assignee: Unassigned Components: github-branch-source-plugin Created: 2019-02-23 17:03 Environment: Jenkins ver. 2.150.3 GitHub Branch Source Plugin 2.4.2 Priority: Major Reporter: Brian J Murrell I have a GitHub Organisation configured and it is operating fairly satisfactorily.  It finds PRs when they are opened and builds them, and so on, quite successfully. But every now and then it will build not only the PR but also the branch that the PR is on, adding a continuous-integration/jenkins/branch commit status to the PR. This is wasteful in the least a can be confusing to the PR submitters about why they have this strange new commit status occasionally. I suppose this could happen legitimately if there is latency between pushing to a new branch on GitHub and opening the PR for it.  Is there any delay on building new branches to account for this sort of behaviour?  If not, perhaps there should be? Add Comment

This message was sent by Atlassian Jira (v7.11.2#711002-sha1:fdc329d)

[brian.murrell@intel.com (JIRA)]

Brian J Murrell commented on JENKINS-56255

Re: occasionally builds the branch for a PR also The other possibility for this issue is to have a whiltelist filter of branches to build and ignore the rest assuming they will eventually become a PR.

Add Comment

[brian.murrell@intel.com (JIRA)]

Brian J Murrell commented on JENKINS-56255

Re: occasionally builds the branch for a PR also

Any response here?  Having the branch build for PRs that are opened within a very small number of seconds of pushing the branch is quite annoying at best and frequently confusing for users who don't understand the workings of the github-branch-source-plugin.

Add Comment

[brian.murrell@intel.com (JIRA)]

Brian J Murrell commented on JENKINS-56255

Re: occasionally builds the branch for a PR also

This is starting to happen more frequently and is wreaking havoc in our CI system.  Users are constantly confused about failed continuous-integration/jenkins/branch commit statuses in their GitHub PRs and some of these branch builds are overwriting statuses of the PR build.  I wonder why there hasn't been any response to my queries about this bug.  Have I done something wrong in the way I filed this issue in so much as it's being ignored?

Add Comment

[brian.murrell@intel.com (JIRA)]

Brian J Murrell updated an issue

Jenkins / JENKINS-56255

occasionally builds the branch for a PR also

Change By: Brian J Murrell Priority: Major Critical

Add Comment

[bitwiseman@gmail.com (JIRA)]

Liam Newman commented on JENKINS-56255

Re: occasionally builds the branch for a PR also Brian J Murrell No, you haven't done anything wrong. Just not enough resources. I understand your frustration. Is there any pattern to when this happens? My guess is this is a race condition related to scanning, but we'll see. What is the config for your multibranch pipeline?

Add Comment

[brian.murrell@intel.com (JIRA)]

Brian J Murrell commented on JENKINS-56255

Re: occasionally builds the branch for a PR also

Liam Newman

Is there any pattern to when this happens?

I have not been able to determine any pattern, no.

My guess is this is a race condition related to scanning, but we'll see.

Indeed, that is my guess also.  Can you describe the algorithm that the plugin is using to decide when a branch is not a branch but is a PR?

What is the config for your multibranch pipeline?

What is the best way to convey that to you?

Add Comment

[bitwiseman@gmail.com (JIRA)]

Liam Newman commented on JENKINS-56255

Re: occasionally builds the branch for a PR also

Brian J Murrell If you want to take a screenshot or attach a copy of your config.xml, either of those would be good.

Add Comment

[brian.murrell@intel.com (JIRA)]

Brian J Murrell updated an issue

Jenkins / JENKINS-56255

occasionally builds the branch for a PR also

Change By: Brian J Murrell

Attachment: image-2019-06-28-07-49-51-334.png

Add Comment

[brian.murrell@intel.com (JIRA)]

Brian J Murrell commented on JENKINS-56255

Re: occasionally builds the branch for a PR also Hopefully this is what you are looking for:

Add Comment

[brian.murrell@intel.com (JIRA)]

Brian J Murrell commented on JENKINS-56255

Re: occasionally builds the branch for a PR also

Was my previous comment what you were looking for? We are seeing this increasingly frequently and it's getting very confusing for our developers.

Add Comment

[bitwiseman@gmail.com (JIRA)]

Liam Newman commented on JENKINS-56255

Re: occasionally builds the branch for a PR also

Brian J Murrell Yes, thanks. So, you're developer are pushing branches to the main repo and not to their own fork. Also, yes, I agree with your original suggestion, this is not a bug but a race/latency condition that is part of the design. I see what you're saying - when "Exclude branches that are also filed as PRs" having a delay of some sort would be useful. However, how long should that delay be? It may take someone 5 minutes or 50 minutes to finish drafting a PR description. I'm think this issue should be closed as "By Design". There are at least two reasonable workarounds. 1. Use forks and discover PRs from forks. This will make it so new branches are rarely if ever created in the main repo. 2. If you do not want to use forks, you can use "Filter by name (with wildcards)" or "Filter by name (with regex)" discovery strategies to make it so only specific branches are built and the rest are ignored.

Add Comment

[brian.murrell@intel.com (JIRA)]

Brian J Murrell commented on JENKINS-56255

Re: occasionally builds the branch for a PR also

you're developer are pushing branches to the main repo and not to their own fork. That's correct. They are all organisation members so philosophically, the main repo is really their (shared) repo. But pragmatically (and primarily), it is done this way due to the inability to prevent Jenkins from processing PRs from untrusted sources. Jenkins can be told to refuse to process the Jenkinsfile from an untrusted source, but cannot be told to completely ignore PRs from untrusted source. But Jenkins can be told to ignore PRs from forks. So the proxy for "untrusted source" is any fork.

having a delay of some sort would be useful

So you are suggesting that there is in fact no delay? At all? I cannot see how that is ever going to work. The branch will always show up before the PR no matter how quickly the developer can get to turning a branch into a PR. I guess this is trying to rely on Jenkins being slower to notice the new branch than noticing the PR? Surely you can see how this is fatally flawed.

how long should that delay be?

Yes, this is a good question. Probably begs to be configurable. But even if not, some number of minutes seems reasonable. At least that way I can say to my users "you need to make sure you create your PR within ... of pushing your branch or you will get this ugly behaviour". Right now I cannot tell them anything other than that this is a nasty Jenkins bug and they have to just live with it.

I'm think this issue should be closed as "By Design".

Or left open as "Needs redesign"?

Use forks and discover PRs from forks

This simply cannot be done. There are a lot of people that agree with me on this. We are simply not allowed to just let any random stranger run whatever (perhaps nefarious) code they can push to a PR for to run on our Jenkins. The lack of this ability is griped about quite frequently.

use "Filter by name (with wildcards)" or "Filter by name (with regex)" discovery strategies

That might be workable. Funny enough, it's more or less the same solution as I coded up earlier this morning when I commented on this issue earlier.

Add Comment

[bitwiseman@gmail.com (JIRA)]

Liam Newman commented on JENKINS-56255

Re: occasionally builds the branch for a PR also

That's correct. They are all organisation members so philosophically, the main repo is really their (shared) repo. But pragmatically (and primarily), it is done this way due to the inability to prevent Jenkins from processing PRs from untrusted sources. Jenkins can be told to refuse to process the Jenkinsfile from an untrusted source, but cannot be told to completely ignore PRs from untrusted source. But Jenkins can be told to ignore PRs from forks. So the proxy for "untrusted source" is any fork. What about "Ignore change requests flagged as originating from an untrusted source"? It is in your image above.

So you are suggesting that there is in fact no delay? At all? I cannot see how that is ever going to work. The branch will always show up before the PR no matter how quickly the developer can get to turning a branch into a PR. I guess this is trying to rely on Jenkins being slower to notice the new branch than noticing the PR? Surely you can see how this is fatally flawed.

I agree, but my point was I think the problem is inherent in the feature. How would you redesign this to make it not flawed? Adding a delay would be mean that all branches wait a certain amount of time before they are built, which is generally not desirable.

The lack of this ability is griped about quite frequently.

Let's get those gripers together and discuss how to address the problem.

That might be workable. Funny enough, it's more or less the same solution as I coded up earlier this morning when I commented on this issue earlier.

Cool, well done. Either way.

Add Comment

[brian.murrell@intel.com (JIRA)]

Brian J Murrell commented on JENKINS-56255

Re: occasionally builds the branch for a PR also

What about "Ignore change requests flagged as originating from an untrusted source"? It is in your image above. My understanding of that option is that it doesn't actually prevent Jenkins from building from the untrusted source but rather just instructs Jenkins to ignore the Jenkinsfile such a change-request and use the Jenkinsfile from the project's master.

I agree, but my point was I think the problem is inherent in the feature. How would you redesign this to make it not flawed? Adding a delay would be mean that all branches wait a certain amount of time before they are built, which is generally not desirable.

How frequent is creating new branches that are not backed by a PR in the real world? Because this delay should only really be required by new branches given that subsequent pushes to the branch already have the PR in place to tell Jenkins not to build the branch. And if it were configurable from 0 to whatever delay a Jenkins admin desires, then it's backward compatible with existing (flawed) behaviour of anyone wants to retain that. But it does allow fixing this behaviour for anyone who desires that. Let's get those gripers together and discuss how to address the problem.  The gripers are griping about a completely different issue to this issue so it's really quite orthogonal. They are griping about an issue that occurs when trying to employ your suggested "work-around". And the explanation of why your work-around cannot work in some situations is only use-case why that work-around doesn't work. It could just plainly be that an organisation doesn't want PRs from forks for other organisational reasons. Ultimately any workarounds are just pussy-footing around the real issue of the branch/PR race that is inherent in any design which doesn't allow for a (configurable if you wish) delay to give the PR time to be submitted. So please, let's not get off into the weeds about why any particular work-around is not suitable and just fix the root issue?

Add Comment

[bitwiseman@gmail.com (JIRA)]

Liam Newman updated an issue

Jenkins / JENKINS-56255

occasionally builds the branch for a PR also

Change By: Liam Newman Priority: Critical Major

Add Comment

[bitwiseman@gmail.com (JIRA)]

Liam Newman commented on JENKINS-56255

Re: occasionally builds the branch for a PR also Brian J Murrell It sounds like you're getting frustrated with my responses. I want to be clear that I am not trying to block you, I am trying to help. I want to make sure you (and anyone else encountering this issue) have a reasonable work-around. It sound like we've achieved that - either by script added to the pipeline or by setting added to the job configuration. The more involved work-around is to move to the more mainstream pattern of doing PRs from forks (and using "Ignore change requests flagged as originating from an untrusted source" if you need to prevent building of PRs from any rando forks). I'm not blocking anyone from implementing a fix for this. I'd like that fix to make sense and be useful, so I'm asking questions about what it should look like. I apologize if that came off as too skeptical or pessimistic. Let me try again. The fix you proposed in seems like a reasonable improvement, which also has some limitations. I'd like to make sure that when someone goes to implement this they understand those limitations as part of the proposed behavior. I'm one of several regular contributors to this plugin but to my knowledge most us are already at full capacity currently. Perhaps you or someone from your organization could implement this?

Add Comment

[brian.murrell@intel.com (JIRA)]

Brian J Murrell commented on JENKINS-56255

Re: occasionally builds the branch for a PR also

Liam Newman I tested your work-around,  and have verified that the Ignore change requests flagged as originating from an untrusted source option (recently added I think) feature does as it seems and as such could be a valid work-around. But that doesn't make our current workflow any less valid[1] and it might be that I cannot convince all of my users to switch to submitting from forks now that they have gotten used to the branch-in-organisation workflow. [1] One redeeming feature of the workflow of using branches inside the main repo is that everyone can easily be aware of all of all WIP simply by inspecting the branches of repo.

Add Comment

[brian.murrell@intel.com (JIRA)]

Brian J Murrell edited a comment on JENKINS-56255

Re: occasionally builds the branch for a PR also

{quote}

What about "Ignore change requests flagged as originating from an untrusted source"? It is in your image above.

{quote} - My understanding of that option is that it doesn't actually prevent Jenkins from building from the untrusted source but rather just instructs Jenkins to ignore the {{Jenkinsfile}} such a change-request and use the {{Jenkinsfile}} from the project's master. -This was confusing the new feature you are suggesting using with some previous functionality around PRs from untrusted sources. {quote}

I agree, but my point was I think the problem is inherent in the feature. How would you redesign this to make it not flawed? Adding a delay would be mean that all branches wait a certain amount of time before they are built, which is generally not desirable.

{quote}

How frequent is creating new branches that are not backed by a PR in the real world?  Because this delay should only really be required by new branches given that subsequent pushes to the branch already have the PR in place to tell Jenkins not to build the branch.  And if it were configurable from 0 to whatever delay a Jenkins admin desires, then it's backward compatible with existing (flawed) behaviour of anyone wants to retain that.  But it does allow fixing this behaviour for anyone who desires that.

{quote}

Let's get those gripers together and discuss how to address the problem. 

{quote} - The gripers are griping about a completely different issue to this issue so it's really quite orthogonal.  They are griping about an issue that occurs when trying to employ your suggested "work-around".  And the explanation of why your work-around cannot work in some situations is only use-case why that work-around doesn't work. -   It could just plainly be that an organisation doesn't want PRs from forks for other organisational reasons.

Ultimately any workarounds are just pussy-footing around the real issue of the branch/PR race that is inherent in any design which doesn't allow for a (configurable if you wish) delay to give the PR time to be submitted.  So please, let's not get off into the weeds about why any particular work-around is not suitable and just fix the root issue?

Add Comment

[brian.murrell@intel.com (JIRA)]

Brian J Murrell edited a comment on JENKINS-56255

Re: occasionally builds the branch for a PR also

{quote}What about "Ignore change requests flagged as originating from an untrusted source"? It is in your image above. {quote}

-My understanding of that option is that it doesn't actually prevent Jenkins from building from the untrusted source but rather just instructs Jenkins to ignore the {{Jenkinsfile}} such a change-request and use the {{Jenkinsfile}} from the project's master.- This The above was my confusing the new feature you are suggesting using with some previous functionality around PRs from untrusted sources.

{quote}I agree, but my point was I think the problem is inherent in the feature. How would you redesign this to make it not flawed? Adding a delay would be mean that all branches wait a certain amount of time before they are built, which is generally not desirable. {quote} How frequent is creating new branches that are not backed by a PR in the real world? Because this delay should only really be required by new branches given that subsequent pushes to the branch already have the PR in place to tell Jenkins not to build the branch. And if it were configurable from 0 to whatever delay a Jenkins admin desires, then it's backward compatible with existing (flawed) behaviour of anyone wants to retain that. But it does allow fixing this behaviour for anyone who desires that. {quote}Let's get those gripers together and discuss how to address the problem.  {quote}

-The gripers are griping about a completely different issue to this issue so it's really quite orthogonal. They are griping about an issue that occurs when trying to employ your suggested "work-around". And the explanation of why your work-around cannot work in some situations is only use-case why that work-around doesn't work.- It could just plainly be that an organisation doesn't want PRs from forks for other organisational reasons.

Ultimately any workarounds are just pussy-footing around the real issue of the branch/PR race that is inherent in any design which doesn't allow for a (configurable if you wish) delay to give the PR time to be submitted. So please, let's not get off into the weeds about why any particular work-around is not suitable and just fix the root issue?

Add Comment

[bitwiseman@gmail.com (JIRA)]

Liam Newman edited a comment on JENKINS-56255

Re: occasionally builds the branch for a PR also

[~brianjmurrell]

It sounds like you're getting frustrated with my responses.   I want to be clear that I am not trying to block you, I am trying to help.

I want to make sure you (and anyone else encountering this issue) have a reasonable work-around.  It sound like we've achieved that using branch filtering - either by script added to the pipeline or by branch filter setting added to the job configuration.  The more involved work-around is to move to the more mainstream pattern of doing PRs from forks (and using "Ignore change requests flagged as originating from an untrusted source" if you need to prevent building of PRs from any rando forks).

I'm not blocking anyone from implementing a fix for this.  I'd like that fix to make sense and be useful, so I'm asking questions about what it should look like.  I apologize if that came off as too skeptical or pessimistic. Let me try again. The fix you proposed in seems like a reasonable improvement, which also has some limitations.  I'd like to make sure that when someone goes to implement this they understand those limitations as part of the proposed behavior. I'm one of several regular contributors to this plugin but to my knowledge most us are already at full capacity currently. Perhaps you or someone from your organization could implement this?

Add Comment

[bitwiseman@gmail.com (JIRA)]

Liam Newman commented on JENKINS-56255

Re: occasionally builds the branch for a PR also

Brian J Murrell Hey, thanks for testing that alternative. And yes, I totally agree branch-in-organization is a totally a valid workflow, in which case the branch filtering work-around is the way to go.

Add Comment

[brian.murrell@intel.com (JIRA)]

Brian J Murrell commented on JENKINS-56255

Re: occasionally builds the branch for a PR also

But having to do branch filtering does not embrace the principle of least surprise. I am sure most people will find it surprising that the branch/PR race design is not more robust and that they have to employ branch filtering to augment it. On the other hand, I think a configuration item, right next to (or below) the selector for whether you want to build branches that are not PRs – for how long to delay a first-time branch build to decide if it's going to become a PR is really straightforward.  I think most people would understand that delay is how Jenkins decides if branches are going to become PRs.  Not having anything at all leaves people to not even consider the design problem and assuming that Jenkins will then just figure it out.  And then it doesn't.

Add Comment

[brian.murrell@intel.com (JIRA)]

Brian J Murrell commented on JENKINS-56255

Re: occasionally builds the branch for a PR also

OK.  So I am back. There is something more flawed than just a race between new branch and PR discovery going on here. This doesn't just happen on initial branch/PR creation (i.e Build #1) but happens on subsequent pushes to the PR/branch.  If this were just a race, subsequent pushes would not fall victim to this, correct? Additionally, working from forks (as much as my users really want to do it) is actually, unworkable due to these bugs. But working from forks or not is actually quite orthogonal, as I have described above, this does not seem to merely be a race condition on new branches/forks.  This doesn't seem to be "by design".  This seems like an actual bug.

Add Comment

This message was sent by Atlassian Jira (v7.13.12#713012-sha1:6e07c38)

[roman@pickl.eu (JIRA)]

Roman Pickl commented on JENKINS-56255

Re: occasionally builds the branch for a PR also

I have the same issue. I have the feeling it is even worse. the started branch builds (with multiple stages/agents) seem to fail as jenkins already cleans up after detecting a PR:

Add Comment

[roman@pickl.eu (JIRA)]

Roman Pickl commented on JENKINS-56255

Re: occasionally builds the branch for a PR also

Brian J Murrell does the workaround in https://issues.jenkins-ci.org/browse/JENKINS-58442 "fix" this for you? I'm interested, because I've implemented it as well and it looks ok so far.

Add Comment

[brian.murrell@intel.com (JIRA)]

Brian J Murrell commented on JENKINS-56255

Re: occasionally builds the branch for a PR also

Roman Pickl No unfortunately not, per my previous comment.

Add Comment