• Icon: Bug Bug
    • Resolution: Unresolved
    • Icon: Major Major
    • None
    • Jenkins ver. 2.150.3
      GitHub Branch Source Plugin 2.4.2

      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?

          [JENKINS-56255] 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.

          Brian J Murrell added a comment - 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.

          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.

          Brian J Murrell added a comment - 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.

          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?

          Brian J Murrell added a comment - 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?

          Liam Newman added a comment -

          brianjmurrell
          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?

          Liam Newman added a comment - brianjmurrell 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?

          bitwiseman

          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?

          Brian J Murrell added a comment - bitwiseman 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?

          Liam Newman added a comment -

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

          Liam Newman added a comment - brianjmurrell If you want to take a screenshot or attach a copy of your config.xml, either of those would be good.

          Hopefully this is what you are looking for:

          Brian J Murrell added a comment - Hopefully this is what you are looking for:

          Was my previous comment what you were looking for?

          We are seeing this increasingly frequently and it's getting very confusing for our developers.

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

          Liam Newman added a comment -

          brianjmurrell
          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.

          Liam Newman added a comment - brianjmurrell 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.

          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.

          Brian J Murrell added a comment - 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.

          Liam Newman added a comment -

          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.

          Liam Newman added a comment - 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.

          Brian J Murrell added a comment - - edited

          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.

          The above was my confusing the new feature you are suggesting using with some previous functionality around PRs from untrusted sources.

          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?

          Brian J Murrell added a comment - - edited 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. The above was my confusing the new feature you are suggesting using with some previous functionality around PRs from untrusted sources. 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?

          Liam Newman added a comment - - edited

          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?

          Liam Newman added a comment - - edited 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?

          bitwiseman 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.

          Brian J Murrell added a comment - bitwiseman 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.

          Liam Newman added a comment -

          brianjmurrell
          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.

          Liam Newman added a comment - brianjmurrell 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.

          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.

          Brian J Murrell added a comment - 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.

          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.

          Brian J Murrell added a comment - 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.

          Roman Pickl added a comment -

          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:

          Roman Pickl added a comment - 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:

          Roman Pickl added a comment -

          brianjmurrell 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.

          Roman Pickl added a comment - brianjmurrell 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.

          rompic No unfortunately not, per my previous comment.

          Brian J Murrell added a comment - rompic No unfortunately not, per my previous comment .

          Wade added a comment -

          > 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.

          I do this all of the time once I've opened a PR and am making revisions against it; if I have a branch `feature` that is a PR then I'll push to `feature-staging`, verify that CI passes, then merge `feature-staging` into `feature`.  This prevents me from accidentally pushing totally broken stuff to the original PR.

           

          That being said, I've also been running into issues because of the current behavior.  The most nefarious issue is when an intermittent failure causes the branch build to fail but the PR build succeeds; this causes the overall PR on GitHub to be marked as failing CI because of the branch build failure.  A workaround is to manually re-trigger the build in the branch and hope it succeeds, but if the branch has been cleaned up there's no way to even re-trigger the branch build so the PR stays marked as failing CI with no way to fix the GitHub status from Jenkins.  There should be some kind of way to (preferably automatically) detect and fix this issue from Jenkins.

          In my opinion, the following two things should be done when a PR build is created and the corresponding branch is supposed to be excluded from the build:

          1. Abort any outstanding build jobs against the branch job
          2. Create a pending build status for the PR and then submit a successful build status against the branch (since there doesn't appear to be a way to clear existing statuses)

          This would allow branch jobs to continue as normal while also doing the needed clean-up when a PR is opened.

          In addition, when scanning a GitHub repo, Jenkins should check for PRs with stale build statuses and fix them as above.

          Thoughts?

          P.S. The workaround to fix a stale branch build status is:

          $ curl -L -X POST -H "Authorization: Bearer ${TOKEN}" https://api.github.com/repos/${OWNER}/${REPO}/statuses/${SHA} -d '{"state":"success","description":"Manual fix","context":"continuous-integration/jenkins/branch"}' 

          Wade added a comment - > 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. I do this all of the time once I've opened a PR and am making revisions against it; if I have a branch `feature` that is a PR then I'll push to `feature-staging`, verify that CI passes, then merge `feature-staging` into `feature`.  This prevents me from accidentally pushing totally broken stuff to the original PR.   That being said, I've also been running into issues because of the current behavior.  The most nefarious issue is when an intermittent failure causes the branch build to fail but the PR build succeeds ; this causes the overall PR on GitHub to be marked as failing CI because of the branch build failure.  A workaround is to manually re-trigger the build in the branch and hope it succeeds, but if the branch has been cleaned up there's no way to even re-trigger the branch build so the PR stays marked as failing CI with no way to fix the GitHub status from Jenkins .  There should be some kind of way to (preferably automatically) detect and fix this issue from Jenkins. In my opinion, the following two things should be done when a PR build is created and the corresponding branch is supposed to be excluded from the build: Abort any outstanding build jobs against the branch job Create a pending build status for the PR and then submit a successful build status against the branch (since there doesn't appear to be a way to clear existing statuses) This would allow branch jobs to continue as normal while also doing the needed clean-up when a PR is opened. In addition, when scanning a GitHub repo, Jenkins should check for PRs with stale build statuses and fix them as above. Thoughts? P.S. The workaround to fix a stale branch build status is: $ curl -L -X POST -H "Authorization: Bearer ${TOKEN}" https: //api.github.com/repos/${OWNER}/${REPO}/statuses/${SHA} -d '{ "state" : "success" , "description" : "Manual fix" , "context" : "continuous-integration/jenkins/branch" }'

            Unassigned Unassigned
            brianjmurrell Brian J Murrell
            Votes:
            2 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated: