From 82afba73474c98a1c28b4ebad55e3f6181f6981a Mon Sep 17 00:00:00 2001 From: Chmouel Boudjnah Date: Wed, 12 Aug 2026 12:01:33 +0200 Subject: [PATCH] fix: retry gh transient 404 on check-run update GitHub can answer with 404 for a check run that Pipelines as Code has just created and immediately updates. PAC treated that answer as terminal, so it aborted PipelineRun creation and left the pull request with a failed check that /retest could not recover when triggering is restricted to pull_request events. Create the check run fully formed, with its output, annotations, and conclusion when the run is already finished, so a freshly created check run needs no follow-up update at all. Retry the update briefly when the check-run id came from the PipelineRun annotation, since another reconcile may have created that check run moments earlier and GitHub can still report it as missing. Use three retries at 500ms, 1s, and 2s, which covers the create-to- update lag observed in the report while still failing quickly on a genuine error. Keep every other 404 terminal. An id discovered through a check-run lookup is not retried, so a deleted or inaccessible check run still surfaces as an error instead of being hidden behind repeated requests. Retry only an explicit 404 status response. Transport errors and timeouts are excluded because such a request may already have reached GitHub, and repeating it could apply the same update twice. Return the original error once the retries are exhausted. Leave the generic provider retry transport unchanged. Only this call site knows whether the identifier came from the PipelineRun annotation, which is the condition that makes the retry safe. Fixes #2920 Jira: https://issues.redhat.com/browse/SRVKP-13334 Co-Authored-By: Claude Signed-off-by: Chmouel Boudjnah --- docs/content/docs/api/configmap.md | 6 + pkg/provider/github/status.go | 133 +++++++--- pkg/provider/github/status_test.go | 394 +++++++++++++++++++++++++++++ 3 files changed, 499 insertions(+), 34 deletions(-) diff --git a/docs/content/docs/api/configmap.md b/docs/content/docs/api/configmap.md index c27297f432..3bd314823a 100644 --- a/docs/content/docs/api/configmap.md +++ b/docs/content/docs/api/configmap.md @@ -399,6 +399,12 @@ Pipelines-as-Code only repeats an operation when it can do so safely. It does not repeat provider changes after an uncertain network or server failure when doing so could create duplicate comments, statuses, or other mutations. +Independently of this setting, Pipelines-as-Code always retries a GitHub +check-run update that returns a 404 when the check-run id comes from the +annotation on a PipelineRun. Another reconcile may have created that check run +moments earlier and GitHub can still report it as missing, so the update is +retried a few times with a short backoff before the error is reported. + ```yaml enable-api-retry: "false" ``` diff --git a/pkg/provider/github/status.go b/pkg/provider/github/status.go index 61dd9bf8bd..68584c3892 100644 --- a/pkg/provider/github/status.go +++ b/pkg/provider/github/status.go @@ -2,7 +2,9 @@ package github import ( "context" + "errors" "fmt" + "net/http" "regexp" "strconv" "strings" @@ -27,6 +29,12 @@ const ( pendingApproval = "Pending approval, waiting for an /ok-to-test" checkRunsFetchMaxRetries = 2 checkRunsFetchInitialBackoff = 200 * time.Millisecond + + // GitHub may answer with a 404 for a check run created moments earlier by + // another reconcile, whose id we read from the PipelineRun annotation, so + // retry that update briefly before giving up on it. + checkRunUpdateMaxRetries = 3 + checkRunUpdateInitialBackoff = 500 * time.Millisecond ) const taskStatusTemplate = ` @@ -183,23 +191,27 @@ func (v *Provider) canIUseCheckrunID(checkrunid *int64) bool { return false } -func (v *Provider) createCheckRunStatus(ctx context.Context, runevent *info.Event, status providerstatus.StatusOpts) (*int64, error) { +// createCheckRunStatus creates a check run with its complete state, including +// the output annotations and, when the run is already finished, its conclusion +// and completion time. Creating the check run fully formed avoids a follow-up +// update on a check run GitHub may not report as existing yet. +func (v *Provider) createCheckRunStatus(ctx context.Context, runevent *info.Event, status providerstatus.StatusOpts, output *github.CheckRunOutput, conclusion string) (*int64, error) { now := github.Timestamp{Time: time.Now()} checkrunoption := github.CreateCheckRunOptions{ - Name: provider.GetCheckName(status, v.pacInfo), - HeadSHA: runevent.SHA, - Status: github.Ptr(status.Status), // take status from statusOpts because it can be in_progress, queued, or failure // same for conclusion as well - Output: &github.CheckRunOutput{ - Title: github.Ptr(status.Title), - Summary: github.Ptr(status.Summary), - Text: github.Ptr(status.Text), - }, + Name: provider.GetCheckName(status, v.pacInfo), + HeadSHA: runevent.SHA, + Status: github.Ptr(status.Status), // take status from statusOpts because it can be in_progress, queued, or failure // same for conclusion as well + Output: output, DetailsURL: github.Ptr(status.DetailsURL), ExternalID: github.Ptr(status.PipelineRunName), StartedAt: &now, } - if status.Status != "in_progress" && status.Status != "queued" { + switch { + case conclusion != "": + checkrunoption.Conclusion = github.Ptr(conclusion) + checkrunoption.CompletedAt = &now + case status.Status != "in_progress" && status.Status != "queued": checkrunoption.Conclusion = github.Ptr(string(status.Conclusion)) } @@ -277,6 +289,49 @@ func (v *Provider) getFailuresMessageAsAnnotations(ctx context.Context, pr *tekt return annotations } +// isNotFoundError reports whether err is an explicit HTTP 404 response from the +// GitHub API. Transport errors and timeouts are not considered not-found. +func isNotFoundError(err error) bool { + var errResp *github.ErrorResponse + if !errors.As(err, &errResp) { + return false + } + return errResp.Response != nil && errResp.Response.StatusCode == http.StatusNotFound +} + +// updateCheckRun updates a check run. When retryNotFound is set, an explicit 404 +// is retried with a short backoff: the check-run id then comes from the +// PipelineRun annotation, so another reconcile may have created that run only +// milliseconds ago and GitHub can still report it as missing. Any other +// failure, and a 404 on a check run we know exists, is returned as is. +func (v *Provider) updateCheckRun(ctx context.Context, runevent *info.Event, checkRunID int64, opts github.UpdateCheckRunOptions, retryNotFound bool) error { + for attempt := range checkRunUpdateMaxRetries + 1 { + _, _, err := wrapAPI(v, "update_check_run", func() (*github.CheckRun, *github.Response, error) { + return v.Client().Checks.UpdateCheckRun(ctx, runevent.Organization, runevent.Repository, checkRunID, opts) + }) + if err == nil { + return nil + } + + if !retryNotFound || !isNotFoundError(err) || attempt == checkRunUpdateMaxRetries { + return err + } + + backoff := time.Duration(1<