From 8f5f7218892ddc19a0e0358ccf375b3c1bb69ba5 Mon Sep 17 00:00:00 2001
From: Tanmay Satam <tsatam@redhat.com>
Date: Wed, 8 Nov 2023 17:15:39 -0500
Subject: [PATCH] Handle Azure HTTP 429 responses in reconcile; requeue after
 provided Retry-After value

---
 .../storageaccount_controller.go              | 23 +++++++++++++++++
 .../storageaccount_controller_test.go         | 25 +++++++++++++++++++
 2 files changed, 48 insertions(+)

diff --git a/pkg/operator/controllers/storageaccounts/storageaccount_controller.go b/pkg/operator/controllers/storageaccounts/storageaccount_controller.go
index e21578ef8db..4d3ca05c24f 100644
--- a/pkg/operator/controllers/storageaccounts/storageaccount_controller.go
+++ b/pkg/operator/controllers/storageaccounts/storageaccount_controller.go
@@ -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"
@@ -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
 	}
 
@@ -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
 	}
 
@@ -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 {
diff --git a/pkg/operator/controllers/storageaccounts/storageaccount_controller_test.go b/pkg/operator/controllers/storageaccounts/storageaccount_controller_test.go
index 4538e7700e7..c482a67a309 100644
--- a/pkg/operator/controllers/storageaccounts/storageaccount_controller_test.go
+++ b/pkg/operator/controllers/storageaccounts/storageaccount_controller_test.go
@@ -6,6 +6,7 @@ package storageaccounts
 import (
 	"context"
 	"fmt"
+	"net/http"
 	"strconv"
 	"testing"
 	"time"
@@ -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
@@ -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)