ghreconciler: handle grpc ResourceExhausted with jitter

Signed-off-by: Nghia Tran <[email protected]>

Author: tcnghia

pkg/githubreconciler/reconciler.go

@@ -9,9 +9,12 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"math/rand"
 	"time"
 
 	"github.com/google/go-github/v75/github"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 
 	"github.com/chainguard-dev/clog"
 	"github.com/chainguard-dev/terraform-infra-common/pkg/workqueue"
@@ -56,6 +59,16 @@ func (r *Resource) String() string {
 	return fmt.Sprintf("%s/%s#%d", r.Owner, r.Repo, r.Number)
 }
 
+// addJitter adds random jitter to a duration to avoid thundering herd.
+// Jitter is 0% to +10% of the base duration.
+//
+//nolint:gosec // Using weak random for jitter is fine, not cryptographic
+func addJitter(d time.Duration) time.Duration {
+	// Add jitter between 0% and +10%
+	jitter := time.Duration(rand.Int63n(int64(d / 10)))
+	return d + jitter
+}
+
 // Reconciler manages the reconciliation of GitHub resources.
 type Reconciler struct {
 	workqueue.UnimplementedWorkqueueServiceServer
@@ -149,9 +162,10 @@ func (r *Reconciler) Reconcile(ctx context.Context, url string) error {
 		if errors.As(err, &rateLimitErr) {
 			// Calculate duration until rate limit resets
 			resetTime := rateLimitErr.Rate.Reset.Time
+			delay := addJitter(time.Until(resetTime))
 			clog.FromContext(ctx).With("reset_at", resetTime).
 				Warn("Rate limited, requeueing after rate limit reset")
-			return workqueue.RequeueAfter(time.Until(resetTime))
+			return workqueue.RequeueAfter(delay)
 		}
 
 		// Check if it's an abuse rate limit error
@@ -162,9 +176,19 @@ func (r *Reconciler) Reconcile(ctx context.Context, url string) error {
 			if abuseRateLimitErr.RetryAfter != nil {
 				retryAfter = *abuseRateLimitErr.RetryAfter
 			}
-			clog.FromContext(ctx).With("retry_after", retryAfter).
+			delay := addJitter(retryAfter)
+			clog.FromContext(ctx).With("retry_after", delay).
 				Warn("Abuse rate limit detected, requeueing after retry period")
-			return workqueue.RequeueAfter(retryAfter)
+			return workqueue.RequeueAfter(delay)
+		}
+
+		// Check if it's a gRPC ResourceExhausted error
+		if status.Code(err) == codes.ResourceExhausted {
+			// Resource exhausted - use a conservative 1 minute retry
+			delay := addJitter(time.Minute)
+			clog.FromContext(ctx).With("retry_after", delay).
+				Warn("gRPC ResourceExhausted detected, requeueing after retry period")
+			return workqueue.RequeueAfter(delay)
 		}
 	}
 	return err

pkg/githubreconciler/reconciler_test.go

@@ -11,9 +11,14 @@ import (
 	"fmt"
 	"strings"
 	"testing"
+	"time"
 
 	"github.com/google/go-github/v75/github"
 	"golang.org/x/oauth2"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+
+	"github.com/chainguard-dev/terraform-infra-common/pkg/workqueue"
 )
 
 // mockTokenSourceFunc returns a mock token source
@@ -245,3 +250,83 @@ func TestReconciler_GetStateManager(t *testing.T) {
 func contains(s, substr string) bool {
 	return strings.Contains(s, substr)
 }
+
+func TestReconciler_RateLimitHandling(t *testing.T) {
+	tests := []struct {
+		name             string
+		reconcilerError  error
+		wantRequeue      bool
+		wantRequeueAfter time.Duration
+		wantErrContains  string
+	}{{
+		name:             "gRPC ResourceExhausted error triggers requeue",
+		reconcilerError:  status.Error(codes.ResourceExhausted, "resource exhausted"),
+		wantRequeue:      true,
+		wantRequeueAfter: time.Minute,
+	}, {
+		name:             "wrapped gRPC ResourceExhausted error triggers requeue",
+		reconcilerError:  fmt.Errorf("failed to call API: %w", status.Error(codes.ResourceExhausted, "quota exceeded")),
+		wantRequeue:      true,
+		wantRequeueAfter: time.Minute,
+	}, {
+		name:             "GitHub RateLimitError triggers requeue",
+		reconcilerError:  &github.RateLimitError{Rate: github.Rate{Reset: github.Timestamp{Time: time.Now().Add(5 * time.Minute)}}},
+		wantRequeue:      true,
+		wantRequeueAfter: 5 * time.Minute,
+	}, {
+		name:             "GitHub AbuseRateLimitError with RetryAfter triggers requeue",
+		reconcilerError:  &github.AbuseRateLimitError{RetryAfter: ptrDuration(2 * time.Minute)},
+		wantRequeue:      true,
+		wantRequeueAfter: 2 * time.Minute,
+	}, {
+		name:             "GitHub AbuseRateLimitError without RetryAfter uses default",
+		reconcilerError:  &github.AbuseRateLimitError{},
+		wantRequeue:      true,
+		wantRequeueAfter: time.Minute,
+	}, {
+		name:            "non-rate-limit error does not trigger requeue",
+		reconcilerError: errors.New("regular error"),
+		wantRequeue:     false,
+		wantErrContains: "regular error",
+	}}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			ctx := context.Background()
+			cc := NewClientCache(mockTokenSourceFunc)
+
+			r := NewReconciler(cc, WithReconciler(func(_ context.Context, _ *Resource, _ *github.Client) error {
+				return tt.reconcilerError
+			}))
+
+			err := r.Reconcile(ctx, "https://github.com/owner/repo/issues/123")
+
+			if tt.wantRequeue {
+				delay, ok := workqueue.GetRequeueDelay(err)
+				if !ok {
+					t.Errorf("Expected requeue error, got = %v", err)
+				}
+				// Jitter adds 0% to +10% to the base delay
+				minDelay := tt.wantRequeueAfter
+				maxDelay := tt.wantRequeueAfter + (tt.wantRequeueAfter / 10)
+				if delay < minDelay || delay > maxDelay {
+					t.Errorf("Requeue delay = %v, want between %v and %v", delay, minDelay, maxDelay)
+				}
+			} else {
+				if err == nil {
+					t.Error("Expected error, got nil")
+				}
+				if _, ok := workqueue.GetRequeueDelay(err); ok {
+					t.Errorf("Did not expect requeue error, got = %v", err)
+				}
+				if tt.wantErrContains != "" && !contains(err.Error(), tt.wantErrContains) {
+					t.Errorf("Error = %v, want error containing %v", err, tt.wantErrContains)
+				}
+			}
+		})
+	}
+}
+
+func ptrDuration(d time.Duration) *time.Duration {
+	return &d
+}