Open haminhcong opened 5 years ago
sniiick
commented
5 years ago Same problem. Seems that already triggered builds from "Open MR events" or "Push events" on same commit hash blocks from triggering on MR closing. Just deleted recently built builds and close event was correctly handled.
haminhcong
commented
5 years ago @snicks92 The problem is close MR is not handled correctly after version 1.5.10.
Start from commit fe6b1b11b78d24a77e8a1a6b90c0b52a4af47538, chain of general OR conditions is used for allow or deny a merrge request build. In Enum Action, we have theses action is accepted
package com.dabsquared.gitlabjenkins.gitlab.hook.model;
/**
* @author Robin Müller
*/
public enum Action {
open, update, approved, merge, closed, reopen
}but when we click to close Merge Request, the close merge request hook sent to Jenkins has following content:
{
"object_kind": "merge_request",
"event_type": "merge_request",
"user": {
"name": "user",
"username": "user",
"avatar_url": "http://privateURL"
},
"project": {
"id": 11,
"name": "private-repository",
"description": "repository",
"web_url": "http://192.168.120.2:8180/private_repo",
"avatar_url": null,
"git_ssh_url": "ssh://git@192.168.120.2:8122/private_repo.git",
"git_http_url": "http://192.168.120.2:8180/private_repo.git",
"namespace": "user",
"visibility_level": 0,
"path_with_namespace": "private_repo",
"default_branch": "master",
"ci_config_path": null,
"homepage": "http://192.168.120.2:8180/private_repo",
"url": "ssh://git@192.168.120.2:8122/private_repo.git",
"ssh_url": "ssh://git@192.168.120.2:8122/private_repo.git",
"http_url": "http://192.168.120.2:8180/private_repo.git"
},
"object_attributes": {
"assignee_id": null,
"author_id": 5,
"created_at": "2019-03-25 09:29:04 UTC",
"description": "",
"head_pipeline_id": 4833,
"id": 972,
"iid": 7,
"last_edited_at": null,
"last_edited_by_id": null,
"merge_commit_sha": null,
"merge_error": null,
"merge_params": {
"force_remove_source_branch": "0"
},
"merge_status": "can_be_merged",
"merge_user_id": null,
"merge_when_pipeline_succeeds": false,
"milestone_id": null,
"source_branch": "check-mr-2",
"source_project_id": 11,
"state": "closed",
"target_branch": "check-mr",
"target_project_id": 11,
"time_estimate": 0,
"title": "Test again",
"updated_at": "2019-03-28 01:16:53 UTC",
"updated_by_id": null,
"url": "http://192.168.120.2:8180/private_repo/merge_requests/7",
"source": {
"id": 11,
"name": "private-repository",
"description": "private repository application",
"web_url": "http://192.168.120.2:8180/private_repo",
"avatar_url": null,
"git_ssh_url": "ssh://git@192.168.120.2:8122/private_repo.git",
"git_http_url": "http://192.168.120.2:8180/private_repo.git",
"namespace": "user",
"visibility_level": 0,
"path_with_namespace": "private_repo",
"default_branch": "master",
"ci_config_path": null,
"homepage": "http://192.168.120.2:8180/private_repo",
"url": "ssh://git@192.168.120.2:8122/private_repo.git",
"ssh_url": "ssh://git@192.168.120.2:8122/private_repo.git",
"http_url": "http://192.168.120.2:8180/private_repo.git"
},
"target": {
"id": 11,
"name": "private-repository",
"description": "private repository application",
"web_url": "http://192.168.120.2:8180/private_repo",
"avatar_url": null,
"git_ssh_url": "ssh://git@192.168.120.2:8122/private_repo.git",
"git_http_url": "http://192.168.120.2:8180/private_repo.git",
"namespace": "user",
"visibility_level": 0,
"path_with_namespace": "private_repo",
"default_branch": "master",
"ci_config_path": null,
"homepage": "http://192.168.120.2:8180/private_repo",
"url": "ssh://git@192.168.120.2:8122/private_repo.git",
"ssh_url": "ssh://git@192.168.120.2:8122/private_repo.git",
"http_url": "http://192.168.120.2:8180/private_repo.git"
},
"last_commit": {
"id": "6c63bff3382f47e54fab2e036697b21a351f1ebe",
"message": "Re check\n",
"timestamp": "2019-03-26T01:07:17Z",
"url": "http://192.168.120.2:8180/private_repo/commit/6c63bff3382f47e54fab2e036697b21a351f1ebe",
"author": {
"name": "user",
"email": "user@gmail.com.vn"
}
},
"work_in_progress": false,
"total_time_spent": 0,
"human_total_time_spent": null,
"human_time_estimate": null,
"action": "close"
},
"labels": [
],
"changes": {
"state": {
"previous": "opened",
"current": "closed"
},
"updated_at": {
"previous": "2019-03-27 09:42:59 UTC",
"current": "2019-03-28 01:16:53 UTC"
},
"total_time_spent": {
"previous": null,
"current": 0
}
},
"repository": {
"name": "private-repository",
"url": "ssh://git@192.168.120.2:8122/private_repo.git",
"description": "repo application",
"homepage": "http://192.168.120.2:8180/private_repo"
}
}the action attribute value is close, not closed.
"action": "close"because of this, the mergeRequestHook object converted from request hook content has null value in objectAttributes.action field (because close not exists in Action enum)
public MergeRequestBuildAction(Item project, String json, String secretToken) {
LOGGER.log(Level.FINE, "MergeRequest: {0}", toPrettyPrint(json));
this.project = project;
this.mergeRequestHook = JsonUtil.read(json, MergeRequestHook.class);
this.secretToken = secretToken;
}
then this null value is reject by rejectRules filter in TriggerConfigChain, and Close Merge Request Build is dropped:
class MergeRequestHookTriggerHandlerImpl extends AbstractWebHookTriggerHandler<MergeRequestHook> implements MergeRequestHookTriggerHandler {
@Override
public void handle(Job<?, ?> job, MergeRequestHook hook, boolean ciSkip, BranchFilter branchFilter, MergeRequestLabelFilter mergeRequestLabelFilter) {
if (isAllowedByConfig(objectAttributes)
&& isLastCommitNotYetBuild(job, hook)
&& isNotSkipWorkInProgressMergeRequest(objectAttributes)) {
List<String> labelsNames = new ArrayList<>();
if (hook.getLabels() != null) {
for (MergeRequestLabel label : hook.getLabels()) {
labelsNames.add(label.getTitle());
}
}
if (mergeRequestLabelFilter.isMergeRequestAllowed(labelsNames)) {
super.handle(job, hook, ciSkip, branchFilter, mergeRequestLabelFilter);
}
}
private boolean isAllowedByConfig(MergeRequestObjectAttributes objectAttributes) {
return triggerConfig.apply(objectAttributes);
}
}
public class TriggerConfigChain implements Predicate<MergeRequestObjectAttributes> {
@Override
public boolean apply(@Nullable MergeRequestObjectAttributes mergeRequestObjectAttributes) {
for (Predicate<MergeRequestObjectAttributes> predicate : rejectRules) {
if (predicate.apply(mergeRequestObjectAttributes)) {
return false;
}
}
for (Predicate<MergeRequestObjectAttributes> predicate : acceptRules) {
if (predicate.apply(mergeRequestObjectAttributes)) {
return true;
}
}
return false;
}
}class StateAndActionConfig implements Predicate<MergeRequestObjectAttributes> {
private final Predicate<State> states;
private final Predicate<Action> actions;
...
static <T> Predicate<T> nullOrContains(final Collection<T> collection) {
return collection == null ? Predicates.<T>alwaysTrue() : new Predicate<T>() {
@Override
public boolean apply(@Nullable T t) {
return t == null || collection.contains(t);
}
};
}
...
}I guess that, if we can re-correct Action enum by change from closed to close, the problem will be resolved.
sniiick
commented
5 years ago @haminhcong You are absolutely right. Also manually changing the request from action "close" to "closed" doesn't work due to isLastCommitNotYetBuild check.
com.dabsquared.gitlabjenkins.trigger.handler.merge.MergeRequestHookTriggerHandlerImpl isLastCommitNotYetBuild INFO: Last commit in Merge Request has already been built in build #..
class MergeRequestHookTriggerHandlerImpl extends AbstractWebHookTriggerHandler<MergeRequestHook> implements MergeRequestHookTriggerHandler {
@Override
public void handle(Job<?, ?> job, MergeRequestHook hook, boolean ciSkip, BranchFilter branchFilter, MergeRequestLabelFilter mergeRequestLabelFilter) {
if (isAllowedByConfig(objectAttributes)
&& isLastCommitNotYetBuild(job, hook)
&& isNotSkipWorkInProgressMergeRequest(objectAttributes)) {
List<String> labelsNames = new ArrayList<>();
if (hook.getLabels() != null) {
for (MergeRequestLabel label : hook.getLabels()) {
labelsNames.add(label.getTitle());
}
}
if (mergeRequestLabelFilter.isMergeRequestAllowed(labelsNames)) {
super.handle(job, hook, ciSkip, branchFilter, mergeRequestLabelFilter);
}
}
private boolean isAllowedByConfig(MergeRequestObjectAttributes objectAttributes) {
return triggerConfig.apply(objectAttributes);
}
}@snicks92 I don't need isLastCommitNotYetBuild feature in my Jenkins system, so in my private fork i disabled this check and build this plugin from modified source :)
@Override
public void handle(Job<?, ?> job, MergeRequestHook hook, boolean ciSkip, BranchFilter branchFilter, MergeRequestLabelFilter mergeRequestLabelFilter) {
MergeRequestObjectAttributes objectAttributes = hook.getObjectAttributes();
if (isAllowedByConfig(objectAttributes)
&& isNotSkipWorkInProgressMergeRequest(objectAttributes)) {
List<String> labelsNames = new ArrayList<>();
if (hook.getLabels() != null) {
for (MergeRequestLabel label : hook.getLabels()) {
labelsNames.add(label.getTitle());
}
}
if (mergeRequestLabelFilter.isMergeRequestAllowed(labelsNames)) {
super.handle(job, hook, ciSkip, branchFilter, mergeRequestLabelFilter);
}
}
}
Issue
Context
I have a CI system with jenkins and Gitlab. Three months ago, my boss upgade gitlab plugin from 1.5.9 to 1.5.10. A day later, i close a merge request and see that close merge request event are not trigger any build, although I still see merge request hook still received and handled by Gitlab plugin.
Then i go to my PC and setup a demo environment with gitlab v10 and v11, with Jenkins Gitlab plugin version is 1.5.9, and merge request build work again. Therefore, i think that we has some problem with Jenkins Gitlab plugin version 1.5.10 and 1.5.11.
Can you help me resolve this problem ? Thank you very much.
Below is logs in Jenkins log from my test when i click "close merge request" button in gitlab.
Logs & Traces
With v1.5.10 and v1.5.11
With v1.5.9
Here is my jenkins job configuration:
bash-4.4# cat /var/jenkins_home/jobs/spring_test_close_mr/config.xml