jenkinsci / github-checks-plugin

Jenkins Plugin for GitHub Checks API
https://plugins.jenkins.io/github-checks/
MIT License
78 stars 38 forks source link

Status check "COMPLETED" skipped if branch deleted in the meantime #196

Open alexbde opened 3 years ago

alexbde commented 3 years ago

We experience an issue with the status checks reporting:

Steps to reproduce:

Expected behaviour:

The status COMPLETED is reported to GitHub, e.g.:

image

Actual behaviour:

The status COMPLETED is not reported to GitHub.

image

This is not bad for the remote branch test (because it was deleted anyhow), but the status check on the branch main spins endlessly:

image

image

Not sure if related: we're using two shared declarative pipelines for the build.

Versions

Jenkins: 2.289.3
GitHub Enterprise Server: 3.0.13
Jenkins plugins:
ace-editor: 1.1
antisamy-markup-formatter: 2.1
apache-httpcomponents-client-4-api: 4.5.13-1.0
authentication-tokens: 1.4
authorize-project: 1.4.0
basic-branch-build-strategies: 1.3.2
blueocean: 1.24.8
blueocean-autofavorite: 1.2.4
blueocean-bitbucket-pipeline: 1.24.8
blueocean-commons: 1.24.8
blueocean-config: 1.24.8
blueocean-core-js: 1.24.8
blueocean-dashboard: 1.24.8
blueocean-display-url: 2.4.1
blueocean-events: 1.24.8
blueocean-git-pipeline: 1.24.8
blueocean-github-pipeline: 1.24.8
blueocean-i18n: 1.24.8
blueocean-jira: 1.24.8
blueocean-jwt: 1.24.8
blueocean-personalization: 1.24.8
blueocean-pipeline-api-impl: 1.24.8
blueocean-pipeline-editor: 1.24.8
blueocean-pipeline-scm-api: 1.24.8
blueocean-rest: 1.24.8
blueocean-rest-impl: 1.24.8
blueocean-web: 1.24.8
bootstrap4-api: 4.6.0-3
bootstrap5-api: 5.1.0-1
bouncycastle-api: 2.22
branch-api: 2.6.5
caffeine-api: 2.9.2-29.v717aac953ff3
checks-api: 1.7.2
cloudbees-bitbucket-branch-source: 2.9.10
cloudbees-disk-usage-simple: 0.10
cloudbees-folder: 6.16
command-launcher: 1.6
config-file-provider: 3.8.1
credentials: 2.5
credentials-binding: 1.27
cucumber-reports: 5.5.0
display-url-api: 2.3.5
docker-commons: 1.17
docker-workflow: 1.26
durable-task: 1.39
echarts-api: 5.1.2-9
email-ext: 2.83
extended-read-permission: 3.2
external-monitor-job: 1.7
favorite: 2.3.3
font-awesome-api: 5.15.3-4
git: 4.8.1
git-client: 3.9.0
git-server: 1.10
github: 1.34.0
github-api: 1.123
github-branch-source: 2.11.2
github-checks: 1.0.13
github-pr-comment-build: 2.3
h2-api: 1.4.199
handlebars: 3.0.8
handy-uri-templates-2-api: 2.1.8-1.0
htmlpublisher: 1.25
http_request: 1.10
jackson2-api: 2.12.4
jacoco: 3.3.0
javadoc: 1.6
jdk-tool: 1.5
jenkins-design-language: 1.24.8
jira: 3.5
jjwt-api: 0.11.2-9.c8b45b8bb173
jquery-detached: 1.2.1
jquery3-api: 3.6.0-2
jsch: 0.1.55.2
junit: 1.52
ldap: 2.7
lockable-resources: 2.11
mailer: 1.34
matrix-auth: 2.6.8
matrix-project: 1.19
maven-plugin: 3.12
mercurial: 2.15
metrics: 4.0.2.8
momentjs: 1.1.1
nexus-artifact-uploader: 2.13
okhttp-api: 3.14.9
openJDK-native-plugin: 1.4
pam-auth: 1.6
pipeline-aws: 1.43
pipeline-build-step: 2.15
pipeline-github: 2.7
pipeline-graph-analysis: 1.11
pipeline-input-step: 2.12
pipeline-maven: 3.10.0
pipeline-milestone-step: 1.3.2
pipeline-model-api: 1.9.1
pipeline-model-definition: 1.9.1
pipeline-model-extensions: 1.9.1
pipeline-multibranch-defaults: 2.1
pipeline-rest-api: 2.19
pipeline-stage-step: 2.5
pipeline-stage-tags-metadata: 1.9.1
pipeline-stage-view: 2.19
pipeline-utility-steps: 2.8.0
plain-credentials: 1.7
plugin-util-api: 2.4.0
popper-api: 1.16.1-2
popper2-api: 2.9.3-1
prometheus: 2.0.10
pubsub-light: 1.16
resource-disposer: 0.16
role-strategy: 3.2.0
saml: 2.0.7
scm-api: 2.6.5
script-security: 1.78
selenium: 3.141.59
seleniumhtmlreport: 1.1
serenity: 1.4
slave-proxy: 1.1
snakeyaml-api: 1.29.1
sonar: 2.13.1
sse-gateway: 1.24
ssh-agent: 1.23
ssh-credentials: 1.19
ssh-slaves: 1.32.0
sshd: 3.1.0
structs: 1.23
swarm: 3.27
token-macro: 266.v44a80cf277fd
trilead-api: 1.0.13
variant: 1.4
versionnumber: 1.9
windows-slaves: 1.8
workflow-aggregator: 2.6
workflow-api: 2.46
workflow-basic-steps: 2.24
workflow-cps: 2.93
workflow-cps-global-lib: 2.21
workflow-durable-task-step: 2.39
workflow-job: 2.41
workflow-multibranch: 2.26
workflow-scm-step: 2.13
workflow-step-api: 2.24
workflow-support: 3.8
ws-cleanup: 0.39
KalleOlaviNiemitalo commented 3 years ago

I have seen in-progress build statuses posted to Bitbucket Server get stuck in a similar way. Those are implemented in cloudbees-bitbucket-branch-source-plugin or atlassian-bitbucket-server-integration-plugin, and not depend on checks-api-plugin at all. So I'm wondering if the problem might be in workflow-multibranch-plugin or in the Jenkins core.

KalleOlaviNiemitalo commented 3 years ago

And now there is https://github.com/jenkinsci/bitbucket-branch-source-plugin/pull/472 for fixing that in one plugin. Is it a coincidence or is someone reading my comments here? 😀 Let's see how it goes; maybe it is possible to use a similar solution in checks-api-plugin or github-checks-plugin.

alexbde commented 3 years ago

I can confirm that this also happens when a branch/PR is merged in the meantime :/

That's a great coincidence with the bitbucket plugin. But I just saw your comment, the same fix will not work for the github-checks-plugin. Do you already have an idea how to solve it?

alexbde commented 3 years ago

Hi again! Some additional information: I tested merging a PR during the current build and thought it is the same issue as if the branch was deleted in between. But I just realized the reporting of the COMPLETED status is not missing in this case, but failing because it reports the status to the shared pipeline repository instead:

image

This is kind of interesting, because I thought issues like that are fixed via #70. How can this still happen? 😕

KalleOlaviNiemitalo commented 3 years ago

Well, https://github.com/jenkinsci/github-checks-plugin/pull/70 tries GitHubSCMSourceChecksContext first, but I suppose GitHubSCMSourceChecksContext doesn't find anything if the branch has been deleted from the repository and Jenkins has switched the branch to NullSCMSource. And then it falls back to GitSCMChecksContext and finds the pipeline shared library.

alexbde commented 3 years ago

Sounds reasonable. If we're lucky the same fix will solve both issues.

So, I'm willing to help here. I had a look into the code base, but I'm not familiar enough with Jenkins plugin mechanisms to fully understand it. I wouldn't hesitate to file a PR, but only if it's helpful of course :D If I can support, just say a word.

KalleOlaviNiemitalo commented 3 years ago

I don't use github-checks-plugin myself and am not going to try to fix this.

alexbde commented 3 years ago

I don't use github-checks-plugin myself and am not going to try to fix this.

@KalleOlaviNiemitalo I thought you're a maintainer due to your understanding of the issue, but looks like I got that wrong, sorry for that!

Next steps for me: implement workaround via GitHub API (idea: calling the commits/${env.GIT_COMMIT}/checks_run endpoint and setting state to COMPLETED in pipelines deleting/merging the triggering branch). Have a look how the github-branch-source-plugin detects the commit sha and reports the status - looks like this is working fine in all situations (starting point).

KalleOlaviNiemitalo commented 3 years ago

@alexbde I studied the SCM selection logic in github-checks-plugin when I was working on a different plugin.

alexbde commented 3 years ago

Workaround time! As promised I implemented a workaround to deal with this issue. It's not beautiful, it's working.

Snippet for declarative pipeline that can be added after a stage/pipeline which deletes/merges the triggering branch:

post {
    always {
        // Begin of workaround to mitigate https://github.com/jenkinsci/github-checks-plugin/issues/196
        script {
            def buildResult = currentBuild.currentResult.toLowerCase().replace('unstable', 'neutral')
            // @see https://docs.github.com/en/rest/reference/checks#list-check-runs-for-a-git-reference
            def checkRuns = // curl GET <github-api>/commits/${env.GIT_COMMIT}/check-runs?check_name=Jenkins&status=in_progress
            checkRuns.check_runs.each { checkRun ->
                // @see https://docs.github.com/en/rest/reference/checks#update-a-check-run
                // curl PATCH <github-api>/check-runs/${checkRun.id} -d {"conclusion": "${buildResult}"}
            }
        }
        // End of workaround
    }
}

Note: it is no problem, if the delete/merge only happens conditionally, as the usual status check publishing at the end of the build run will just overwrite the workaround.

timja commented 3 years ago

Aren't you always setting the check_run to success there? even on failure?

alexbde commented 3 years ago

Aren't you always setting the check_run to success there? even on failure?

@timja That's true, thank you for the hint! It doesn't matter in my case because merging/deleting is always the last step. If it fails, the branch is still there and the failure will be reported via github-checks-plugin.

So, I'd rather hope for this issue getting fixed soon, instead of making this workaround beautiful. The important thing for me is, that builds are not shown as running for always.

But if somebody reads and wants to handle this: there are different post conditions, you don't need to use always, but could react to successful/failed builds: see Jenkins docs. As an alternative you could use the result of the currentbuild global var: currentBuild.currentResult. See <your-jenkins-host>/pipeline-syntax/globals for details.

Update: okay, solution with currentBuild.currentResult was really straightforward, I implemented it and adjusted the snippet above.

KalleOlaviNiemitalo commented 3 years ago

atlassian-bitbucket-server-integration adds the commit ID and repository info to the Run as an invisible BitbucketRevisionAction.

alexbde commented 2 years ago

Hi all! Any update on this? It is still an issue for us :/

timja commented 2 years ago

No one is actively working on the repo currently.

PRs are reviewed and merged, so anyone can contribute a fix if they wish.