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<