Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "set, then unset, pending pipelinerun regardless of concurrency limit in startPR" #1331

Merged
merged 1 commit into from
Jun 26, 2023

Conversation

chmouel
Copy link
Member

@chmouel chmouel commented Jun 26, 2023

This reverts commit 13aa6e5.

We have seen a lot of nightly issues lately and would like to figure out if the
issues is this commit.

Let's keep this for a little while to see if our failures flakyness gets away.

Changes

Submitter Checklist

  • ♽ Run make test before submitting a PR (ie: with pre-commit, no need to waste CPU cycle on CI. (or even better install pre-commit and do pre-commit install in the root of this repo).
  • ✨ We heavily rely on linters to get our code clean and consistent, please ensure that you have run make lint before submitting a PR. The markdownlint error can get usually fixed by running make fix-markdownlint (make sure it's installed first)
  • 📖 If you are adding a user facing feature or make a change of the behavior, please verify that you have documented it
  • 🧪 100% coverage is not a target but most of the time we would rather have a unit test if you make a code change.
  • 🎁 If that's something that is possible to do please ensure to check if we can add a e2e test.
  • 🔎 If there is a flakiness in the CI tests then don't necessary ignore it, better get the flakyness fixed before merging or if that's not possible there is a good reason to bypass it. (token rate limitation may be a good reason to skip).

@pipelines-as-code
Copy link

Golang test coverage difference report

Coverage decreased by 0.01%. 🔔 Shame 🔔

Package report
package                                                                            before    after    delta
-------                                                                           -------  -------  -------
pkg/acl                                                                           100.00%  100.00%         
pkg/action                                                                         76.19%   76.19%         
pkg/adapter                                                                        72.41%   72.41%         
pkg/apis/features                                                                 100.00%  100.00%         
pkg/cli/info                                                                       88.23%   88.23%         
pkg/cli/prompt                                                                     74.46%   74.46%         
pkg/cli/status                                                                     95.23%   95.23%         
pkg/cli/webhook                                                                    59.36%   59.36%         
pkg/cmd/tknpac/bootstrap                                                            5.72%    5.72%         
pkg/cmd/tknpac/completion                                                          50.00%   50.00%         
pkg/cmd/tknpac/create                                                              43.36%   43.36%         
pkg/cmd/tknpac/describe                                                            46.31%   46.31%         
pkg/cmd/tknpac/generate                                                            62.20%   62.20%         
pkg/cmd/tknpac/info                                                                62.50%   62.50%         
pkg/cmd/tknpac/list                                                                46.47%   46.47%         
pkg/cmd/tknpac/resolve                                                             74.67%   74.67%         
pkg/cmd/tknpac/webhook                                                             52.47%   52.47%         
pkg/consoleui                                                                      84.12%   84.12%         
pkg/customparams                                                                   92.64%   92.64%         
pkg/events                                                                         73.33%   73.33%         
pkg/formatting                                                                     98.73%   98.73%         
pkg/git                                                                            84.84%   84.84%         
pkg/hub                                                                            90.62%   90.62%         
pkg/kubeinteraction                                                                52.50%   52.50%         
pkg/kubeinteraction/status                                                         77.27%   77.27%         
pkg/matcher                                                                        86.47%   86.47%         
pkg/params/clients                                                                 14.86%   14.86%         
pkg/params/settings                                                                79.48%   79.48%         
pkg/pipelineascode                                                                 80.90%   80.90%         
pkg/provider                                                                       76.19%   76.19%         
pkg/provider/bitbucketcloud                                                        87.16%   87.16%         
pkg/provider/bitbucketserver                                                       88.32%   88.32%         
pkg/provider/gitea                                                                 32.66%   32.66%         
pkg/provider/gitea/structs                                                         22.68%   22.68%         
pkg/provider/github                                                                83.03%   83.03%         
pkg/provider/github/app                                                            78.33%   78.33%         
pkg/provider/gitlab                                                                86.49%   86.49%         
pkg/random                                                                        100.00%  100.00%         
pkg/reconciler                                                                     46.10%   46.10%         
pkg/resolve                                                                        87.93%   87.93%         
pkg/secrets                                                                        93.02%   93.02%         
pkg/sort                                                                           51.20%   50.60%   -0.60%
pkg/sync                                                                           91.13%   91.13%         
pkg/templates                                                                     100.00%  100.00%         
pkg/webhook                                                                        22.22%   22.22%         
                                                                          total:   67.21%   67.20%   -0.01%

@chmouel chmouel merged commit c9a7c05 into openshift-pipelines:main Jun 26, 2023
@chmouel chmouel deleted the revert-gabe-patch branch June 26, 2023 14:54
@sm43
Copy link
Contributor

sm43 commented Jun 26, 2023

where do you see the flakyness? reporting status or triggerin the run?

@chmouel
Copy link
Member Author

chmouel commented Jun 26, 2023

it's on e2e nightly we have seen random failures where the tests, we are getting things like:

% tkn pr ls -n pac-e2e-test-lm2kh pr-ksdj-q8j6n
NAME            STARTED   DURATION   STATUS
pr-ksdj-q8j6n   ---       ---        Running(PipelineRunPending)
% k get ev -n pac-e2e-test-lm2kh
LAST SEEN   TYPE      REASON            OBJECT                      MESSAGE
2m5s        Normal    Started           pipelinerun/pr-ksdj-q8j6n   
2m5s        Normal    FinalizerUpdate   pipelinerun/pr-ksdj-q8j6n   Updated "pr-ksdj-q8j6n" finalizers
2m5s        Warning   Error             pipelinerun/pr-ksdj-q8j6n   Operation cannot be fulfilled on pipelineruns.tekton.dev "pr-ksdj-q8j6n": the object has been modified; please apply your changes to the latest version and try again
2m5s        Warning   InternalError     pipelinerun/pr-ksdj-q8j6n   1 error occurred:...

and stuck as Pending or as Error, i can't find any other reasons why would it happen and just wondering if that commit was the one triggering a bug in tektoncd/pipeline..

i was mentioning this to a message to @vdemeester :

 me: this comes from the pipeline controller not from paac, and make it fail to create the pipelinerun, any idea ? should that code supposed to fail and not retry on resource conflict ? https://github.com/openshift-pipelines/pipelines-as-code/blob/688e38d32901ddb168fd07ed30899405b139c537/vendor/github.com/tektoncd/pipeline/pkg/client/injection/reconciler/pipeline/v1/pipelinerun/reconciler.go#L374-L389

the update status does it https://github.com/tektoncd/pipeline/blob/main/pkg/client/injection/reconciler/pipeline/v1/pipelinerun/reconciler.go#L306

anyway let's have a try and see if it still randomly failing on mornings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants