Jenkins won't trigger a build of merge commit on merged Merge Request

[petewilcock]

Issue

Context

• Gitlab plugin version: 1.4.8
• Gitlab version: 9.5.4-ee
• Jenkins version: 2.8.2
• Job type: Pipeline

Logs & Traces

WebHook called with url: /project/Core/Core_build
Oct 05, 2017 10:45:53 AM INFO com.dabsquared.gitlabjenkins.trigger.handler.merge.MergeRequestHookTriggerHandlerImpl isLastCommitNotYetBuild
Last commit in Merge Request has already been built in build #23

Problem description

Here is the scenario:

1. User opens merge request to master in Gitlab. Jenkins CI integration sends webhook on the merge request open event which commences pipeline job.
2. Job checks whether gitlabMergeRequestState == 'opened' and if so, checks out "origin/merge-requests/${env.gitlabMergeRequestIid}" with a refspec defined as "+refs/merge-requests/${env.gitlabMergeRequestIid}/head:refs/remotes/origin/merge-requests/${env.gitlabMergeRequestIid}"
3. Build runs and passes, all good.
4. User merges pull request to master.
5. Webhook for the merge event is sent to Jenkins, but job does not re-trigger (which we want it to do in order to build master) and the logs comment that Last commit in Merge Request has already been built in build #.

The issue seems to stem from the fact that the plugin evaluates the gitlabMergeRequestLastCommit value sent by the webhook, and if it sees it has been built already by another job, refuses to build it.

We have 'Merge Commit' strategy enabled in our repo, so a merge commit sha is actually created. The logic in the pipeline (if the job would actually trigger on merge) looks for the value env.gitlabMergeRequestState == 'merged'at which point it checks out the target branch (in this case master), in order to rebuild master.

So I believe the issue is because the plugin simply evaluates gitlabMergeRequestLastCommit regardless of whether the gitlabMergeRequestState is 'opened' or 'merged' and won't trigger the 'merged' job because in both instances the head of the merge request is the same - despite the fact we want to build the new head of master - but can't get to the job logic to do that!

[petewilcock]

Bump. Any acknowledgement that this is an issue?

The major culprit is the logic here:

gitlab-plugin/src/main/java/com/dabsquared/gitlabjenkins/trigger/handler/merge/MergeRequestHookTriggerHandlerImpl.java

Line 130 in c92e972

private boolean isLastCommitNotYetBuild(Job<?, ?> project, MergeRequestHook hook) {

There are many legitimate scenarios when one may want to rebuild the same commit sha of a previously 'successful' build in Jenkins - having no override feature for this is immensely frustrating considering this doesn't seem to work as intended anyway.

My merges HAVE a commit sha into master, but this trigger examines the head of the PR (which hasn't changed), instead of letting me handle the logic myself within my pipeline - my pipeline that actually looks for the Merge Request State and builds the correct branch!

[petewilcock]

I can't imagine I'm the only one experiencing this issue - in it's current state the plugin is flat out broken for triggering a build to a merged branch.

If I'm doing something wrong, please do tell me, however to be clear, look at this code block.

    private boolean isLastCommitNotYetBuild(Job<?, ?> project, MergeRequestHook hook) {
        MergeRequestObjectAttributes objectAttributes = hook.getObjectAttributes();
        if (objectAttributes != null && objectAttributes.getLastCommit() != null) {
            Run<?, ?> mergeBuild = BuildUtil.getBuildBySHA1IncludingMergeBuilds(project, objectAttributes.getLastCommit().getId());
            if (mergeBuild != null && StringUtils.equals(getTargetBranchFromBuild(mergeBuild), objectAttributes.getTargetBranch())) {
                LOGGER.log(Level.INFO, "Last commit in Merge Request has already been built in build #" + mergeBuild.getNumber());
                return false;
            }
        }
        return true;
    }

This is evaluating whether an incoming web hook for the job in question has already been built, by looking at the last commit to the PR and checking whether the target branch of the hook and the target of the merge request is the same. If it is the same, IT SKIPS THE BUILD.

The obvious, obvious flaw in this logic is that this doesn't care whether the merge request status is 'opened' or 'merged'. So it builds the head of the PR when it gets opened, and then won't even trigger a job for a 'merged' hook because it decides the sha has already been built and won't build it again.

Except for the fact that I'm NOT trying to build that sha again. I'm trying to build the head of the TARGET branch which now has a new commit sha in it. But I can't trigger that job because of this logic that I can't intercept! The plugin only evaluates the head of the PR for an incoming hook.

How the heck are you supposed to build opened merge requests, and then merged requests to the target branch? Again, I am not fast-forwarding here, so the two builds will be different shas, if only the job would trigger.

The only solution I can come up with currently is to have a second job that polls the target branch for changes, and build that separately, which would be an idiotic workaround.

[pabloa]

I have the same problem. Please fix. :D

[adveres]

Gitlab plugin version: 1.5.2
Gitlab version: 10.1.4-ee
Jenkins version: 2.9.4

I will note I'm using a generic webhook and NOT the "Jenkins CI" integration, because it's missing a bunch of triggers, like a "REBUILD" comment.

---

Problem seems to be on plugin side, because I do see the Merge Request Hook being fired in GitLab when I merge the MR.

---

What I want to do...

1. Create a merge request from my branch foo, in project "PROJECT"
2. Trigger my single jenkins job for "PROJECT"
3. Since ${gitlabSourceBranch} isn't master, we'll just build the app and run tests. Pipeline returns success/failure.
4. Merge merge-request after approvals, etc.
5. The webhook should fires because I've merged a merge-request.
6. My single Jenkins job for "PROJECT" should run again using the master branch, and this time I'm going to build/test, but also use a conditional step to compare ${gitlabSourceBranch} to master, and now I'll do a deploy as well.
7. Rename build to show what branch was built, ie: #${BUILD_NUMBER}: origin/${gitlabSourceBranch}

---

I've just tested using the actual "Jenkins CI" integration instead of a webhook. No difference. When I merged my PR into master, nothing happened on Jenkins.
This is a bug!

[jimrippon]

I'm also seeing this issue, would be very interested in any solution or workaround!

[omehegan]

@petewilcock the presumption here is that your workflow would involve a rebase or merge of the target branch onto your source branch before you merge. This would then trigger a build of the source branch, and if you merge after that, triggering another build of the target would be redundant.

I'd be happy to consider a PR to give an option to override this "obvious, obvious" flaw so that you don't need to perform an "idiotic" workaround :)

[petewilcock]

@omehegan The operative word there is 'presumption'. If we were using a fast-forward merge strategy to merge into master/target, then you'd be correct. The head of master and the head of the PR of the source branch would be the same SHA1 - and rebuilding this would be redundant.

However when using a merge commit strategy, the head of master and the head of the PR are different SHA1 (although yes, functionally identical) post-merge. The act of merging to master will trigger the webhook, but it looks at the head of the PR (not the head of the now-merged target) and discards it as already built, whereas what I actually want to build is the merge SHA1 commit of the target.

I had written my pipeline to detect and build the correct commit SHA for this scenario, but obviously the job never triggers because of the 'presumption' that stops the job from triggering in the first place.

I apologise if you regard my language as strong, but to me this is an 'obvious' flaw. The user can be left to configure their workflow as they see fit without making presumptions on their behalf, and when there's no way to workaround this issue natively, the workaround I have to implement strikes me, yes, idiotic. What about other scenario, where I've lost my build artifact for some reason and need to rebuild the same branch? The plugin won't let me without deleting the original build job ID entirely. Urgh.

Anyway, delighted to see this finally acknowledged. Thanks.

[omehegan]

@petewilcock regarding your "strong" language, put yourself in my shoes. I'm a volunteer maintainer of a free open source plugin for a free open source product. The plugin is almost four years old and has nearly 18,000 active installs. Now you come along and open this ticket ranting about this "obvious" flaw (the code for which was written two years ago), a tone which implies that those of us maintaining the code must be imbeciles not to have realized that this is a problem for your workflow.

I get that you're frustrated, but check your tone and try coming in sounding like you have some respect for the people who volunteer their time to actually code, test, document, release, and support this plugin for free for users like you. It's a lot more likely to inspire us to try and solve the problem.

[petewilcock]

@omehegan Having already apologised, your response is now just overly-sensitive. I use and pay for Gitlab EE, and if there was a commercial enterprise version of this plugin, or any other commercial alternative, I'd already be using it.

As it is I took the time to document the issue as clearly as possible, only to confirm that others also have this problem, and it takes months to have it even acknowledged. Yes, frustration grows but it's not like I've attacked you or anyone else even slightly here. Gitlab is a bit of a niche choice, but it's a commercial company. If there's literally no company-level support in maintaining the single plugin for the most popular CI platform, I'd question their priorities a tad. My workflow is not bizarre, it's common. Hopefully a fix for this bug will be forthcoming.

[rtyler]

@petewilcock I recommend discussing the lack of enterprise support, or any GitLab corporate support, for this plugin with your GitLab account representative.

Failing that, it seems that you've developed a non-trivial understanding of the problem, but also where a potential solution might lie. Since the Jenkins project is largely comprised of a collection of volunteers, perhaps you could propose a Pull Request which fixes the issue at hand?

[petewilcock]

I'd be delighted to attempt it, despite this not being within my skill base, assuming I've established acknowledgment of the issue on the part of the plugin maintainers. 👍

[adveres]

FWIW I did ping GitLab support about this issue, not realizing this was a volunteer plugin.

Their reply:

I see that both issues have been created on the Jenkins CI GitLab Plugin project. Please note that this plugin is maintain by Jenkins CI and not by GitLab. Perhaps adding relevant labels (bug?) will help get developers' attention.

GitLab is selling their GitLab Runner idea pretty hard, so I suspect they intend to offer no real support here. Thank you, then, to the volunteers who keep this plugin going. :)

[omehegan]

Indeed, this plugin is not maintained or supported by anyone at GitLab. We're just a few people who use (or used to use, in my case) GitLab and Jenkins. We work on the plugin in our spare time, so there are no guarantees of support.

[petewilcock]

Maybe they're just upset you're hosting the source code on Github ;)

My instinct for a fix would be to either remove the check entirely and not assume this behaviour is desired, or at least check whether the evaluation is occurring in the event of a 'merged' webhook and instead evaluating the status of the sha1 at the head of the target branch. This might be too specific. Thoughts?

[omehegan]

@petewilcock I hesitate to remove it completely because I do think that for the rebase or merge-from-target-first (there must be a better way to say that) workflows, this is a nice-to-have. What I was considering is just a check-box option, either global or per-job, which would override this. That way the default behavior stays as it has been so there are no surprises for people not currently experiencing this issue. Fortunately the code that performs this check is concise and easy to skip with an option.

[Zordrak]

The reason I would like to see this implemented is because I cannot filter which branches will be affected by PUSH and which by MERGE. i.e. I cannot say PUSH_BRANCH_FILTER=master.

If I enable Jenkins to build the job on PUSH events, it will never build on a Merge Request event because every merge request is preceded by a push to the source branch with the same Commit ID. If I don't enable Jenkins to build on PUSH events, the post-merge build that I need to instruct the job that a merge has taken place and a new master artefact needs to be created will not happen, because Gitlab won't send a repeat event on the merge request per the OP's comments.

So, given a need to rebuild the job so that the master branch is built after a feature branch is merged, my only option at present is to enable PUSH events, which will suppress all of the useful functionality of MERGE events.

EDIT: Ignore me, I am talking rubbish. My specific scenario is resolved by the existing filter mechanism as the filter applies only to the target branch, so if I include only "master" it will build on MRs where master is the target, and successful merges which result in a push to master. Therefore I don't have a problem. But I still 👍 this issue on the basis that if I wanted to support feature-to-feature merges, or needed for some reason my master builds to come from MERGE instead of PUSH, this would then apply.

[benzman81]

Same problem here. Revert to 1.5.2 helped to work aroung this.

[onlynone]

I just ran into this from (I think) a slightly different angle. I opened a new merge request, jenkins gitlab plugin received the webhook just fine and built the project. I then closed that merge request and later opened up a new one with the same source branch. The webhook was fired, but the plugin didn't trigger a build with the following message: Last commit in Merge Request has already been built in build #39.

Now, whether the target branch has changed or not, I'd still want the new merge request to get built/tested and status reported back to gitlab. It's more about the reporting per merge request than the commits of the source and target branches. I'd suggest always triggering a build when a new merge request is involved.

[jinfang134]

I also meet the same problem, and for my need, I modified some code.
please acceess my commit for more details . https://github.com/jinfang134/gitlab-plugin/commit/da2ba799032493e1685c01039b8cdab59db52a37

[eriknellessen]

I think I got the same problem as described above:
I have a merge request pipeline that merges the feature branch into the target branch locally and then builds the result of that merge action. I configured the pipeline to be built when there are changes on the feature branch or on the target branch.

In the past, the pipeline was triggered every time there were changes on the target branch. After updating to 1.5.9, the pipeline is no more triggered when there are changes on the target branch. It is still triggered when there are changes on the feature branch.

I am not sure from which version I updated. It could have been 1.5.8 or 1.5.6.

[omehegan]

@eriknellessen if you look at the Jenkins Update Manager UI, it should show you what version you were on before, where the "downgrade" button is.

[eriknellessen]

@omehegan Thank you for the hint! It says 1.5.8. I downgraded and will watch on the next occasion if the behavior changed and give you an update then. Maybe we did another update of the GitLab plugin shortly before this one. We also updated Jenkins recently (our current version is 2.140). So it is not absolutely clear to us that upgrading from 1.5.8 to 1.5.9 changed the behavior.

What is the expected behavior for 1.5.9? Is it a bug that our merge requests do not get rebuilt on target branch change or is it expected behavior?

[eriknellessen]

Downgrading to 1.5.8 did not solve the problem. I still wonder if it is expected behavior we are seeing.

@omehegan How should I proceed with this? Shall I open a separate bug report? Or a feature request?

[omehegan]

@eriknellessen let's make sure you are hitting the same issue found here. Look at the logs in Jenkins when you hit this issue, and see if there is a Last commit in Merge Request has already been built in build xxx line. If so, it's the same issue, and we are tracking it as an RFE.

[eriknellessen]

@omehegan I tried to reproduce the issue with a minimal working example, but it was not possible. I then compared my minimal working example with the project containing the problem. I found out that in the project with the problem, the push events notification was missing in the GitLab web hook. After reactivating it, the merge request pipeline behaved as expected.

So the error was in my GitLab configuration. Sorry for creating unnecessary work!

[mielientiev]

I hesitate to remove it completely because I do think that for the rebase or merge-from-target-first (there must be a better way to say that) workflows, this is a nice-to-have. What I was considering is just a check-box option, either global or per-job, which would override this. That way the default behavior stays as it has been so there are no surprises for people not currently experiencing this issue. Fortunately the code that performs this check is concise and easy to skip with an option.

Hello @omehegan
Any news about it?

[ricktbaker]

Any progress here? Running into the same issue: Last commit in Merge Request has already been built in build.

[amdokamal]

There are workarounds #958 (comment). They work only if setup Merge method: Merge commit as merge strategy. If Fast-forward merge or Semi-linear, then it don't work.

[interone-ms]

Hi,

I have a different use case where I hit the "Last commit in Merge Request has already been built" bug. I (want to) have a central entrypoint where all events in a specific repository are dispatched to - think like building an environment when a MR gets created, deploying to the environment when the MR gets a push - and destroying the environment when the MR gets closed/merged.

The last thing is where I hit the issue: the MR deploy trigger has already fired with the tip of the MR branch to deploy the code, which means that when the MR-closed trigger fires, the Gitlab plugin sees "I have already built the tip of the MR branch" and ignores that this is not a push but a MR close event.

edit: This issue also bites when having a single job that only gets triggered by merge request events - MR close, MR merge and MR reopen will not be picked up at all if the MR is just one single commit (as the branch tip will always be the same commit).