jenkinsci / gitlab-plugin

A Jenkins plugin for interfacing with GitLab
https://plugins.jenkins.io/gitlab-plugin/
GNU General Public License v2.0
1.44k stars 613 forks source link

Multibranch Pipeline Jobs ignores ciSkip #495

Open KekSfabrik opened 7 years ago

KekSfabrik commented 7 years ago

Issue

Multibranch Pipelines ignore the ciSkip field and trigger themselves if they push changes back to GitLab

Context

Logs & Traces

// ... in preparation stage
def gitLabTrigger = [
    $class: "GitLabPushTrigger",
    triggerOnPush: true,
    triggerOnMergeRequest: true,
    acceptMergeRequestOnSuccess: true,
    branchFilterType: "All",
    ciSkip: true, // this is the relevant bit
    triggerOpenMergeRequestOnPush: "both",
    mergeRequestLabelFilterConfig: [
        $class: "com.dabsquared.gitlabjenkins.trigger.filter.MergeRequestLabelFilterConfig",
            include: "master, develop",
             exclude: ""
        ]
]
// ... more triggers combined in a list "pipelineTriggerCollection"
properties([
    pipelineTriggers(pipelineTriggerCollection),
    [$class: 'GitLabConnectionProperty', gitLabConnection: 'GitLab']
])
# in later stage execute this shellscript
git add .
git commit -m "version of ${JOB_NAME} changed to ${MAJOR_VERSION_NUMBER}.${BUILD_NUMBER} by jenkins [ci-skip]" # this commit should not be built because of [ci-skip]
git config credential.username ${GIT_USERNAME}
export tempFile="$(mktemp)"
echo "${GIT_PASSWORD}" > ${tempFile}
git config credential.helper '!f() { echo "password=`cat ${tempFile}`"; }; f' # this somehow fails with just echo ${GIT_PASSWORD}
git pull origin ${GIT_BRANCH#*/}
git push origin ${GIT_BRANCH#*/}
rm -f "${tempFile}"
unset tempFile
git config --unset credential.username
git config --unset credential.helper

Problem description

After the git push instruction a new Build is triggered which should be ignored because of the [ci-skip] bit in the commit message yet it's executed like it weren't there.


Possible other problem

A (possibly completely unrelated) Problem is that the jenkins.log is filled with this sort of stuff:

INFO: Retrieving gitlab project ids
Feb 01, 2017 1:28:13 PM com.dabsquared.gitlabjenkins.util.CommitStatusUpdater retrieveGitlabProjectIds
INFO: Retrieving the gitlab project id from remote url http://MY-GITLAB/GROUP/PROJECT.git

that is probably because i use gitlabCommitStatus over my entire node block and have a custom stage block that wraps updateGitlabCommitStatus around the code

def call(stageName, Closure bodyParam) {
    bodyParam.resolveStrategy = Closure.OWNER_ONLY
    stage(stageName) {
        try {
            updateGitlabCommitStatus state: 'running', name: stageName
            bodyParam()
            updateGitlabCommitStatus state: 'success', name: stageName
        } catch (e) {
            updateGitlabCommitStatus state: 'failed', name: stageName
            throw e
        }
    }
}
omehegan commented 7 years ago

@KekSfabrik there are a couple of other issues filed about ci-skip, which is making me think that our implementation of it has some bugs. But it's not a workflow I use and I don't have a lot of free time to test/reproduce things right now.

Can you confirm if a workflow similar to what you've described works with other job types, such as regular Pipeline or Freestyle?

KekSfabrik commented 7 years ago

@omehegan can confirm it works with "regular" pipeline job (with the same Jenkinsfile on the same repo) but not multibranch pipeline (haven't tested with a freestyle job but i'd guess that works aswell)

this shows up in jenkins.log:

Feb 03, 2017 1:04:30 PM com.dabsquared.gitlabjenkins.webhook.GitLabWebHook getDynamic
INFO: WebHook called with url: /project/pipeline-test
Feb 03, 2017 1:04:30 PM com.dabsquared.gitlabjenkins.trigger.handler.AbstractWebHookTriggerHandler handle
INFO: Skipping due to ci-skip.
NaffanDroo commented 6 years ago

Hi, we're facing this issue too - the scenario is when we automate release processes e.g. with the maven unleash plugin. This will update the next development version - and we can configure it to include [ci-skip] in the comment - but it is still ignored.

Also tested the [ci-skip] in a non multi branch project in the same Jenkins instance and it skips the build with the log @KekSfabrik mentioned above.

Is this bug scheduled to be looked at soon?

omehegan commented 6 years ago

I think I just realized why this doesn't work. It's because the plugin ignores the JSON payload when it's sent to multibranch jobs. See the note at https://github.com/jenkinsci/gitlab-plugin#pipeline-multibranch-jobs I would like to find a way to fix this, but I didn't author the multibranch support.

omehegan commented 6 years ago

704 basically.

byahia commented 5 years ago

Hey all,

Was there any update on this issue? We're facing the same problem, and it's painful to handle the auto-triggered builds avec a maven release step (maven commits back to the repo with a tag and next dev version).

Thanks

stevelowery commented 5 years ago

We are also struggling with this. This is preventing us from having Jenkins update some of the metadata in the repo and pushing it back to gitlab without creating an infinite build loop.

Radoslaw152 commented 5 years ago

We are observing the same behaviour. After digging into code we've found a wrong code block in com/dabsquared/gitlabjenkins/trigger/handler/pipeline/PipelineHookTriggerHandlerImpl.java:59

if (ciSkip && isCiSkip(hook)) { LOGGER.log(Level.INFO, "Skipping due to ci-skip."); return; } .... @Override protected boolean isCiSkip(PipelineHook hook) { //we don't get a commit message or suchlike that could contain ci-skip return false; }

Because isCiSkip() will always return false, then the (ciSkip && isCiSkip(hook)) is always false, thus the pipeline is always started.

Same is true for com/dabsquared/gitlabjenkins/trigger/handler/AbstractWebHookTriggerHandler.java:43

If "[ci-skip]" tag is not present, then the ciSkip flag is ignored.

Simply changing from "ciSkip && isCiSkip(hook)" to "ciSkip || isCiSkip(hook)" makes it work properly.

Katsz commented 4 years ago

Any news on that?

longstone commented 4 years ago

May be we have news now?