pipelines-as-code icon indicating copy to clipboard operation
pipelines-as-code copied to clipboard

Resolve inconsistent status of a PipelineRun after a failure

Open savitaashture opened this issue 1 year ago โ€ข 6 comments

In Pipelines as Code for some CI failures failed checkrun persist like below

Pipelines a Code CI - Failed

on the GitHub status page.

These are the 2 scenarios

  1. If there is a pull request on which the CI has failed checkrun, updating the PR results in the override of the failed check run. This is because a PR update generates a new commit SHA, leading to the creation of new check runs for the updated SHA.

  2. If there is a pull request on which the CI has failed checkrun, if a user adds a /test or /retest comment on the same commit SHA without updating the PR, the failed check run will be retained. Alongside this, new check runs will be added for the existing SHA.

However, in both scenarios, a challenge arises when there are multiple pipeline runs (n pipelineruns) in the .tekton directory, resulting in the creation of n check run IDs for each failed pipeline run.

With the proposed fix, Pipelines as Code now effectively addresses this issue. The solution ensures that, for failed pipeline runs, the check run is appropriately overridden without generating n check run IDs for each failed pipelinerun.

Fixes: https://issues.redhat.com/browse/SRVKP-3400 & https://issues.redhat.com/browse/SRVKP-3379

https://github.com/openshift-pipelines/pipelines-as-code/assets/9441662/ae8fd3bc-e607-4be6-8145-a08c46616be0

Changes

Submitter Checklist

  • [ ] ๐Ÿ“ Please ensure your commit message is clear and informative. For guidance on crafting effective commit messages, refer to the How to write a git commit message guide. We prefer the commit message to be included in the PR body itself rather than a link to an external website (ie: Jira ticket).

  • [ ] โ™ฝ Before submitting a PR, run make test lint to avoid unnecessary CI processing. For an even more efficient workflow, consider installing pre-commit and running pre-commit install in the root of this repository.

  • [ ] โœจ We use linters to maintain clean and consistent code. Please ensure you've run make lint before submitting a PR. Some linters offer a --fix mode, which can be executed with the command make fix-linters (ensure markdownlint and golangci-lint tools are installed first).

  • [ ] ๐Ÿ“– If you're introducing a user-facing feature or changing existing behavior, please ensure it's properly documented.

  • [ ] ๐Ÿงช While 100% coverage isn't a requirement, we encourage unit tests for any code changes where possible.

  • [ ] ๐ŸŽ If feasible, please check if an end-to-end test can be added. See README for more details.

  • [ ] ๐Ÿ”Ž If there's any flakiness in the CI tests, don't necessarily ignore it. It's better to address the issue before merging, or provide a valid reason to bypass it if fixing isn't possible (e.g., token rate limitations).

savitaashture avatar Jan 24 '24 17:01 savitaashture

The issue is not reproducible now with bad yaml I have to return error intentionally in the pipelines-as-code and test the changes

Will add e2e to test bad yaml for GitHubApp

savitaashture avatar Jan 24 '24 17:01 savitaashture

Codecov Report

Attention: Patch coverage is 81.81818% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 64.18%. Comparing base (df22211) to head (838ba13). Report is 1 commits behind head on main.

Files Patch % Lines
pkg/pipelineascode/pipelineascode.go 58.33% 5 Missing :warning:
pkg/provider/github/status.go 94.44% 1 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1566      +/-   ##
==========================================
+ Coverage   64.12%   64.18%   +0.06%     
==========================================
  Files         141      141              
  Lines       10878    10895      +17     
==========================================
+ Hits         6975     6993      +18     
+ Misses       3387     3386       -1     
  Partials      516      516              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jan 24 '24 17:01 codecov[bot]

/retest

savitaashture avatar Mar 12 '24 10:03 savitaashture

/test

savitaashture avatar Mar 12 '24 10:03 savitaashture

/test Pipelines as Code Dogfooding CI

savitaashture avatar Mar 12 '24 11:03 savitaashture

/test

savitaashture avatar Mar 12 '24 16:03 savitaashture

/test doc-generation

chmouel avatar Apr 10 '24 13:04 chmouel

/test go-testing

chmouel avatar Apr 10 '24 13:04 chmouel

/test

savitaashture avatar Apr 11 '24 10:04 savitaashture