feat: added validation and rw mutex

On-behalf-of: SAP [email protected]

Author: OlegErshov

controller/lifecycle/controllerruntime/lifecycle.go

@@ -84,10 +84,15 @@ func (l *LifecycleManager) SetupWithManagerBuilder(mgr ctrl.Manager, maxReconcil
 		return nil, fmt.Errorf("cannot use conditions or spread reconciles in read-only mode")
 	}
 
+	rateLimiter, err := ratelimiter.NewStaticThenExponentialRateLimiter[reconcile.Request](l.rateLimiterConfig)
+	if err != nil {
+		return nil, err
+	}
+
 	eventPredicates = append([]predicate.Predicate{filter.DebugResourcesBehaviourPredicate(debugLabelValue)}, eventPredicates...)
 	opts := controller.Options{
 		MaxConcurrentReconciles: maxReconciles,
-		RateLimiter:             ratelimiter.NewStaticThenExponentialRateLimiter[reconcile.Request](l.rateLimiterConfig),
+		RateLimiter:             rateLimiter,
 	}
 
 	return ctrl.NewControllerManagedBy(mgr).

controller/lifecycle/multicluster/lifecycle.go

@@ -94,10 +94,15 @@ func (l *LifecycleManager) SetupWithManagerBuilder(mgr mcmanager.Manager, maxRec
 		return nil, fmt.Errorf("cannot use conditions or spread reconciles in read-only mode")
 	}
 
+	rateLimiter, err := ratelimiter.NewStaticThenExponentialRateLimiter[mcreconcile.Request](l.rateLimiterConfig)
+	if err != nil{
+		return nil, err
+	}
+
 	eventPredicates = append([]predicate.Predicate{filter.DebugResourcesBehaviourPredicate(debugLabelValue)}, eventPredicates...)
 	opts := controller.TypedOptions[mcreconcile.Request]{
 		MaxConcurrentReconciles: maxReconciles,
-		RateLimiter:             ratelimiter.NewStaticThenExponentialRateLimiter[mcreconcile.Request](l.rateLimiterConfig),
+		RateLimiter:             rateLimiter,
 	}
 
 	return mcbuilder.ControllerManagedBy(mgr).

controller/lifecycle/ratelimiter/config.go

@@ -1,6 +1,7 @@
 package ratelimiter
 
 import (
+	"fmt"
 	"time"
 )
 
@@ -18,6 +19,22 @@ var defaultConfig = Config{
 	ExponentialMaxBackoff:     2 * time.Minute,
 }
 
+func (c Config) validate() error {
+	if c.StaticRequeueDelay < 0 {
+		return fmt.Errorf("the static requeue delay shouldn't be negative")
+	}
+	if c.ExponentialInitialBackoff < 0 {
+		return fmt.Errorf("the initial exponential backoff shouldn't be negative")
+	}
+	if c.StaticRequeueDelay > c.ExponentialInitialBackoff {
+		return fmt.Errorf("the initial exponential backoff should be equal to or greater than the static requeue delay")
+	}
+	if c.StaticWindow < c.StaticRequeueDelay {
+		return fmt.Errorf("the static window duration should be equal to or greater than the static requeue delay")
+	}
+	return nil
+}
+
 type Option func(*Config)
 
 func WithStaticWindow(d time.Duration) Option {
@@ -52,4 +69,4 @@ func NewConfig(options ...Option) Config {
 	}
 
 	return cfg
-}
+}

controller/lifecycle/ratelimiter/static_exponential.go

@@ -9,7 +9,7 @@ import (
 )
 
 type StaticThenExponentialRateLimiter[T comparable] struct {
-	failuresLock sync.Mutex
+	failuresLock sync.RWMutex
 	firstAttempt map[T]time.Time
 
 	staticDelay  time.Duration
@@ -19,7 +19,10 @@ type StaticThenExponentialRateLimiter[T comparable] struct {
 	clock       clock.Clock
 }
 
-func NewStaticThenExponentialRateLimiter[T comparable](cfg Config) *StaticThenExponentialRateLimiter[T] {
+func NewStaticThenExponentialRateLimiter[T comparable](cfg Config) (*StaticThenExponentialRateLimiter[T], error) {
+	if err := cfg.validate(); err != nil {
+		return nil, err
+	}
 	return &StaticThenExponentialRateLimiter[T]{
 		staticDelay:  cfg.StaticRequeueDelay,
 		staticWindow: cfg.StaticWindow,
@@ -29,19 +32,20 @@ func NewStaticThenExponentialRateLimiter[T comparable](cfg Config) *StaticThenEx
 		),
 		firstAttempt: make(map[T]time.Time),
 		clock:        clock.RealClock{},
-	}
+	},nil
 }
 
 func (r *StaticThenExponentialRateLimiter[T]) When(item T) time.Duration {
-	r.failuresLock.Lock()
-	defer r.failuresLock.Unlock()
-
 	now := r.clock.Now()
 
+	r.failuresLock.RLock()
 	first, exists := r.firstAttempt[item]
+	r.failuresLock.RUnlock()
 	if !exists {
-		first = now
-		r.firstAttempt[item] = first
+		r.failuresLock.Lock()
+		r.firstAttempt[item] = now
+		r.failuresLock.Unlock()
+		return r.staticDelay
 	}
 
 	timeSinceFirst := now.Sub(first)

controller/lifecycle/ratelimiter/static_exponential_test.go

@@ -17,7 +17,8 @@ func TestStaticThenExponentialRateLimiter_Forget(t *testing.T) {
 		ExponentialInitialBackoff: 2 * time.Second,
 		ExponentialMaxBackoff:     1 * time.Minute,
 	}
-	limiter := NewStaticThenExponentialRateLimiter[reconcile.Request](cfg)
+	limiter, err := NewStaticThenExponentialRateLimiter[reconcile.Request](cfg)
+	require.Nil(t, err)
 	fakeClock := clocktesting.NewFakeClock(time.Now())
 	limiter.clock = fakeClock