perf(github): cache check-run lookups with retry

pkg/matcher/annotation_matcher_test.go

@@ -1623,9 +1623,8 @@ func TestMatchPipelinerunAnnotationAndRepositories(t *testing.T) {
 			ctx, _ := rtesting.SetupFakeContext(t)
 			fakeclient, mux, ghTestServerURL, teardown := ghtesthelper.SetupGH()
 			defer teardown()
-			vcx := &ghprovider.Provider{
-				Token: github.Ptr("None"),
-			}
+			vcx := ghprovider.New()
+			vcx.Token = github.Ptr("None")
 			vcx.SetGithubClient(fakeclient)
 			if tt.args.runevent.Request == nil {
 				tt.args.runevent.Request = &info.Request{Header: http.Header{}, Payload: nil}
@@ -2338,7 +2337,7 @@ func TestMatchPipelinerunByAnnotation(t *testing.T) {
 
 			eventEmitter := events.NewEventEmitter(cs.Clients.Kube, logger)
 			repo := tt.repo
-			matches, err := MatchPipelinerunByAnnotation(ctx, logger, tt.args.pruns, cs, &tt.args.runevent, &ghprovider.Provider{}, eventEmitter, repo, true)
+			matches, err := MatchPipelinerunByAnnotation(ctx, logger, tt.args.pruns, cs, &tt.args.runevent, ghprovider.New(), eventEmitter, repo, true)
 			if tt.wantErrNoFailedPipelineToRetest {
 				assert.Assert(t, err != nil, "expected ErrNoFailedPipelineToRetest")
 				assert.Assert(t, errors.Is(err, NoFailedPipelineToRetestError("/pac ")), "expected ErrNoFailedPipelineToRetest, got: %v", err)
@@ -3284,7 +3283,7 @@ func TestFilterSuccessfulTemplates(t *testing.T) {
 				return
 			}
 
-			filtered := filterSuccessfulTemplates(ctx, logger, cs, event, repo, &ghprovider.Provider{}, tt.matchedPRs)
+			filtered := filterSuccessfulTemplates(ctx, logger, cs, event, repo, ghprovider.New(), tt.matchedPRs)
 
 			// Check that the correct number of templates remain
 			assert.Equal(t, len(tt.expectedNames), len(filtered),

pkg/pipelineascode/match_test.go

@@ -557,10 +557,9 @@ func TestGetPipelineRunsFromRepo(t *testing.T) {
 			k8int := &kitesthelper.KinterfaceTest{
 				ConsoleURL: "https://console.url",
 			}
-			vcx := &ghprovider.Provider{
-				Token:  github.Ptr("None"),
-				Logger: logger,
-			}
+			vcx := ghprovider.New()
+			vcx.Token = github.Ptr("None")
+			vcx.Logger = logger
 			vcx.SetGithubClient(fakeclient)
 			pacInfo := &info.PacOpts{
 				Settings: settings.Settings{
@@ -789,7 +788,9 @@ func TestVerifyRepoAndUser(t *testing.T) {
 				func(rw http.ResponseWriter, _ *http.Request) { fmt.Fprint(rw, `{}`) },
 			)
 
-			vcx := &ghprovider.Provider{Token: github.Ptr("token"), Logger: logger}
+			vcx := ghprovider.New()
+			vcx.Token = github.Ptr("token")
+			vcx.Logger = logger
 			vcx.SetGithubClient(ghClient)
 			vcx.SetPacInfo(pacInfo)
 

pkg/pipelineascode/pipelineascode_startpr_test.go

@@ -207,11 +207,10 @@ func setupStartPRTest(t *testing.T) (*params.Run, *info.Event, *zap.SugaredLogge
 
 // setupProviderForTest creates and configures a GitHub provider for testing.
 func setupProviderForTest(cs *params.Run, logger *zap.SugaredLogger, fakeclient *github.Client, pacInfo *info.PacOpts) *ghprovider.Provider {
-	vcx := &ghprovider.Provider{
-		Run:    cs,
-		Token:  github.Ptr("test-token"),
-		Logger: logger,
-	}
+	vcx := ghprovider.New()
+	vcx.Run = cs
+	vcx.Token = github.Ptr("test-token")
+	vcx.Logger = logger
 	vcx.SetGithubClient(fakeclient)
 	vcx.SetPacInfo(pacInfo)
 	return vcx

pkg/pipelineascode/pipelineascode_test.go

@@ -662,11 +662,10 @@ func TestRun(t *testing.T) {
 					HubCatalogs:        &hubCatalogs,
 				},
 			}
-			vcx := &ghprovider.Provider{
-				Run:    cs,
-				Token:  github.Ptr("None"),
-				Logger: logger,
-			}
+			vcx := ghprovider.New()
+			vcx.Run = cs
+			vcx.Token = github.Ptr("None")
+			vcx.Logger = logger
 			vcx.SetGithubClient(fakeclient)
 			vcx.SetPacInfo(pacInfo)
 			p := NewPacs(&tt.runevent, vcx, cs, pacInfo, k8int, logger, nil)

pkg/provider/github/github.go

@@ -72,13 +72,25 @@ type Provider struct {
 	pacUserLogin       string // user/bot login used by PAC
 	clock              clockwork.Clock
 	graphQLClient      *graphQLClient
+	checkRunsCache     checkRunsCache
 }
 
 type skippedRun struct {
 	mutex      *sync.Mutex
 	checkRunID int64
 }
 
+type checkRunsCache struct {
+	mu      sync.Mutex
+	entries map[string]*checkRunsCacheEntry
+}
+
+type checkRunsCacheEntry struct {
+	runs    []*github.CheckRun
+	loading bool
+	done    chan struct{}
+}
+
 func New() *Provider {
 	return &Provider{
 		APIURL:        github.Ptr(keys.PublicGithubAPIURL),
@@ -87,6 +99,9 @@ func New() *Provider {
 			mutex: &sync.Mutex{},
 		},
 		clock: clockwork.NewRealClock(),
+		checkRunsCache: checkRunsCache{
+			entries: map[string]*checkRunsCacheEntry{},
+		},
 	}
 }
 

pkg/provider/github/status.go

@@ -23,8 +23,10 @@ import (
 )
 
 const (
-	botType         = "Bot"
-	pendingApproval = "Pending approval, waiting for an /ok-to-test"
+	botType                      = "Bot"
+	pendingApproval              = "Pending approval, waiting for an /ok-to-test"
+	checkRunsFetchMaxRetries     = 2
+	checkRunsFetchInitialBackoff = 200 * time.Millisecond
 )
 
 const taskStatusTemplate = `
@@ -42,7 +44,9 @@ const taskStatusTemplate = `
 {{- end }}
 </table>`
 
-func (v *Provider) getExistingCheckRunID(ctx context.Context, runevent *info.Event, status providerstatus.StatusOpts) (*int64, error) {
+// fetchAllCheckRunPages retrieves every page of check runs for the event SHA.
+func (v *Provider) fetchAllCheckRunPages(ctx context.Context, runevent *info.Event) ([]*github.CheckRun, error) {
+	var all []*github.CheckRun
 	opt := github.ListOptions{PerPage: v.PaginedNumber}
 	for {
 		res, resp, err := wrapAPI(v, "list_check_runs_for_ref", func() (*github.ListCheckRunsResults, *github.Response, error) {
@@ -55,25 +59,114 @@ func (v *Provider) getExistingCheckRunID(ctx context.Context, runevent *info.Eve
 		if err != nil {
 			return nil, err
 		}
+		all = append(all, res.CheckRuns...)
+		if resp.NextPage == 0 {
+			break
+		}
+		opt.Page = resp.NextPage
+	}
+	return all, nil
+}
 
-		for _, checkrun := range res.CheckRuns {
-			// if it is a Pending approval CheckRun then overwrite it
-			if isPendingApprovalCheckrun(checkrun) || isFailedCheckrun(checkrun) {
-				if v.canIUseCheckrunID(checkrun.ID) {
-					return checkrun.ID, nil
-				}
+func (v *Provider) searchCheckRuns(runs []*github.CheckRun, status providerstatus.StatusOpts) *int64 {
+	for _, checkrun := range runs {
+		if isPendingApprovalCheckrun(checkrun) || isFailedCheckrun(checkrun) {
+			if v.canIUseCheckrunID(checkrun.ID) {
+				return checkrun.ID
 			}
-			if *checkrun.ExternalID == status.PipelineRunName {
-				return checkrun.ID, nil
+		}
+		if checkrun.ExternalID != nil && *checkrun.ExternalID == status.PipelineRunName {
+			return checkrun.ID
+		}
+	}
+	return nil
+}
+
+func (v *Provider) getExistingCheckRunID(ctx context.Context, runevent *info.Event, status providerstatus.StatusOpts) (*int64, error) {
+	appID := "none"
+	if v.ApplicationID != nil {
+		appID = strconv.FormatInt(*v.ApplicationID, 10)
+	}
+	cacheKey := fmt.Sprintf("%s/%s/%s/%s", runevent.Organization, runevent.Repository, appID, runevent.SHA)
+
+	// Loop handles the wait from channel and fetch of the check runs from the API
+	for {
+		// Check if the check runs are already cached
+		v.checkRunsCache.mu.Lock()
+		entry, ok := v.checkRunsCache.entries[cacheKey]
+		if ok {
+			// Wait for fetch to complete if the check runs are still loading.
+			if entry.loading {
+				done := entry.done
+				v.checkRunsCache.mu.Unlock()
+				<-done
+				continue
 			}
+			// Return the check run ID if the check runs are loaded.
+			runs := entry.runs
+			v.checkRunsCache.mu.Unlock()
+			return v.searchCheckRuns(runs, status), nil
 		}
-		if resp.NextPage == 0 {
-			break
+
+		// Create a new "loading" entry if the check runs are not cached.
+		entry = &checkRunsCacheEntry{
+			loading: true,
+			done:    make(chan struct{}),
 		}
-		opt.Page = resp.NextPage
+		v.checkRunsCache.entries[cacheKey] = entry
+		v.checkRunsCache.mu.Unlock()
+
+		// Fetch check runs from the API.
+		runs, err := v.fetchAllCheckRunPagesWithRetry(ctx, runevent)
+
+		v.checkRunsCache.mu.Lock()
+		// Delete the entry if fetch failed.
+		if err != nil {
+			delete(v.checkRunsCache.entries, cacheKey)
+		} else {
+			// Update the entry if fetch succeeded.
+			entry.runs = runs
+			entry.loading = false
+		}
+		close(entry.done)
+		v.checkRunsCache.mu.Unlock()
+
+		// Return the error if fetch failed.
+		if err != nil {
+			return nil, err
+		}
+		// Return the check run ID if fetch succeeded.
+		return v.searchCheckRuns(runs, status), nil
 	}
+}
+
+func (v *Provider) fetchAllCheckRunPagesWithRetry(ctx context.Context, runevent *info.Event) ([]*github.CheckRun, error) {
+	var lastErr error
+	for attempt := range checkRunsFetchMaxRetries + 1 {
+		runs, err := v.fetchAllCheckRunPages(ctx, runevent)
+		if err == nil {
+			return runs, nil
+		}
+		lastErr = err
+
+		if attempt == checkRunsFetchMaxRetries {
+			return nil, err
+		}
+
+		backoff := time.Duration(1<<uint(attempt)) * checkRunsFetchInitialBackoff
+		if v.Logger != nil {
+			v.Logger.Debugf("check-runs lookup failed for %s/%s@%s (attempt %d/%d): %v; retrying in %v",
+				runevent.Organization, runevent.Repository, runevent.SHA,
+				attempt+1, checkRunsFetchMaxRetries+1, err, backoff)
+		}
 
-	return nil, nil
+		select {
+		case <-ctx.Done():
+			return nil, ctx.Err()
+		case <-v.getClock().After(backoff):
+		}
+	}
+	return nil, lastErr
 }
 
 func isPendingApprovalCheckrun(run *github.CheckRun) bool {