Skip to content

Commit

Permalink
Handle Azure HTTP 429 responses in reconcile; requeue after provided …
Browse files Browse the repository at this point in the history
…Retry-After value
  • Loading branch information
tsatam committed Nov 8, 2023
1 parent 84ee524 commit 8f5f721
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ package storageaccounts
import (
"context"
"fmt"
"net/http"
"time"

"github.com/Azure/go-autorest/autorest"
"github.com/Azure/go-autorest/autorest/azure"
Expand Down Expand Up @@ -89,6 +91,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.

subnets, err := r.getSubnetsToReconcile(ctx, instance, subscriptionId, manager)
if err != nil {
if retryAfter, ok := errIsRateLimited(err); ok {
return reconcile.Result{RequeueAfter: retryAfter}, nil
}
return reconcile.Result{}, err
}

Expand All @@ -99,6 +104,9 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.

err = manager.reconcileAccounts(ctx, subnets, storageAccounts)
if err != nil {
if retryAfter, ok := errIsRateLimited(err); ok {
return reconcile.Result{RequeueAfter: retryAfter}, nil
}
return reconcile.Result{}, err
}

Expand Down Expand Up @@ -174,6 +182,21 @@ func (r *Reconciler) getStorageAccounts(ctx context.Context, instance *arov1alph
}, nil
}

func errIsRateLimited(err error) (time.Duration, bool) {
if detailedErr, ok := err.(autorest.DetailedError); ok {
if detailedErr.StatusCode == http.StatusTooManyRequests {
retryAfter, err := time.ParseDuration(detailedErr.Response.Header.Get("Retry-After") + "s")
if err != nil {
return 0, false
}

return retryAfter, true
}
}

return 0, false
}

// SetupWithManager creates the controller
func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error {
aroClusterPredicate := predicate.NewPredicateFuncs(func(o client.Object) bool {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package storageaccounts
import (
"context"
"fmt"
"net/http"
"strconv"
"testing"
"time"
Expand Down Expand Up @@ -152,6 +153,14 @@ func TestReconcile(t *testing.T) {
log := logrus.NewEntry(logrus.StandardLogger())

errUnknown := fmt.Errorf("unknown err")
errTooManyRequests := autorest.DetailedError{
StatusCode: http.StatusTooManyRequests,
Response: &http.Response{
Header: http.Header{
"Retry-After": []string{"3600"},
},
},
}

for _, tt := range []struct {
name string
Expand Down Expand Up @@ -200,6 +209,22 @@ func TestReconcile(t *testing.T) {
},
wantErr: errUnknown.Error(),
},
{
name: "too many requests error during subnet checks - requeues",
operatorFlag: true,
fakeCheckClusterSubnetsToReconcile: func(ctx context.Context, subnets []string) ([]string, error) {
return nil, errTooManyRequests
},
wantRequeueAfter: 3600 * time.Second,
},
{
name: "error during account reconciliation - returns direct error",
operatorFlag: true,
fakeReconcileAccounts: func(ctx context.Context, subnets []string, storageAccounts []string) error {
return errTooManyRequests
},
wantRequeueAfter: 3600 * time.Second,
},
} {
t.Run(tt.name, func(t *testing.T) {
instance := getValidClusterInstance(tt.operatorFlag)
Expand Down

0 comments on commit 8f5f721

Please sign in to comment.