diff --git a/go.mod b/go.mod index ee3b0db..65ae277 100644 --- a/go.mod +++ b/go.mod @@ -12,6 +12,7 @@ require ( github.com/prometheus/client_golang v1.22.0 github.com/prometheus/client_model v0.6.2 github.com/samber/lo v1.50.0 + github.com/stretchr/testify v1.10.0 go.uber.org/multierr v1.11.0 go.uber.org/zap v1.27.0 golang.org/x/time v0.12.0 @@ -50,6 +51,7 @@ require ( github.com/modern-go/reflect2 v1.0.2 // indirect github.com/munnerz/goautoneg v0.0.0-20191010083416-a7dc8b61c822 // indirect github.com/pkg/errors v0.9.1 // indirect + github.com/pmezard/go-difflib v1.0.0 // indirect github.com/prometheus/common v0.62.0 // indirect github.com/prometheus/procfs v0.15.1 // indirect github.com/spf13/pflag v1.0.5 // indirect diff --git a/reconciler/reconciler.go b/reconciler/reconciler.go new file mode 100644 index 0000000..3e6c75b --- /dev/null +++ b/reconciler/reconciler.go @@ -0,0 +1,80 @@ +package reconciler + +import ( + "context" + + "k8s.io/client-go/util/workqueue" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +// Result is a wrapper around reconcile.Result that adds RequeueWithBackoff functionality. +type Result struct { + reconcile.Result + RequeueWithBackoff bool +} + +// KeyExtractor extracts a rate limiter key from a context and reconcile.Request. +type KeyExtractor[K any] interface { + Extract(ctx context.Context, req reconcile.Request) K +} + +// RequestKeyExtractor extracts a reconcile.Request key for rate limiting. +type RequestKeyExtractor struct{} + +// Extract returns the reconcile.Request as the key. +func (e RequestKeyExtractor) Extract(ctx context.Context, req reconcile.Request) reconcile.Request { + return req +} + +// Reconciler defines the interface for standard reconcilers +type Reconciler interface { + Reconcile(ctx context.Context) (Result, error) +} + +// ReconcilerFunc is a function type that implements the Reconciler interface. +type ReconcilerFunc func(ctx context.Context) (Result, error) + +// Reconcile implements the Reconciler interface. +func (f ReconcilerFunc) Reconcile(ctx context.Context) (Result, error) { + return f(ctx) +} + +// AsReconciler creates a reconciler from a standard reconciler +func AsReconciler(reconciler Reconciler) reconcile.Reconciler { + return AsGenericReconciler( + func(ctx context.Context, req reconcile.Request) (Result, error) { + return reconciler.Reconcile(ctx) + }, + RequestKeyExtractor{}, + ) +} + +// AsGenericReconciler creates a reconciler with a specific key extractor +func AsGenericReconciler[K comparable]( + reconcileFunc func(ctx context.Context, req reconcile.Request) (Result, error), + keyExtractor KeyExtractor[K], +) reconcile.Reconciler { + return AsGenericReconcilerWithRateLimiter( + reconcileFunc, + keyExtractor, + workqueue.DefaultTypedControllerRateLimiter[K](), + ) +} + +// AsGenericReconcilerWithRateLimiter creates a reconciler with a custom rate limiter +func AsGenericReconcilerWithRateLimiter[K comparable]( + reconcileFunc func(ctx context.Context, req reconcile.Request) (Result, error), + keyExtractor KeyExtractor[K], + rateLimiter workqueue.TypedRateLimiter[K], +) reconcile.Reconciler { + return reconcile.Func(func(ctx context.Context, req reconcile.Request) (reconcile.Result, error) { + result, err := reconcileFunc(ctx, req) + if err != nil { + return reconcile.Result{}, err + } + if result.RequeueWithBackoff { + return reconcile.Result{RequeueAfter: rateLimiter.When(keyExtractor.Extract(ctx, req))}, nil + } + return result.Result, nil + }) +} diff --git a/reconciler/suite_test.go b/reconciler/suite_test.go new file mode 100644 index 0000000..d78892d --- /dev/null +++ b/reconciler/suite_test.go @@ -0,0 +1,365 @@ +package reconciler_test + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/awslabs/operatorpkg/reconciler" + "github.com/stretchr/testify/assert" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +// MockRateLimiter is a mock implementation of workqueue.TypedRateLimiter for testing +type MockRateLimiter[K comparable] struct { + whenFunc func(K) time.Duration + numRequeues int + backoffDuration time.Duration +} + +func (m *MockRateLimiter[K]) When(key K) time.Duration { + if m.whenFunc != nil { + return m.whenFunc(key) + } + m.numRequeues++ + return m.backoffDuration +} + +func (m *MockRateLimiter[K]) NumRequeues(key K) int { + return m.numRequeues +} + +func (m *MockRateLimiter[K]) Forget(key K) { + m.numRequeues = 0 +} + +// MockKeyExtractor is a mock implementation of KeyExtractor for testing +type MockKeyExtractor[K any] struct { + extractFunc func(context.Context, reconcile.Request) K + key K +} + +func (m *MockKeyExtractor[K]) Extract(ctx context.Context, req reconcile.Request) K { + if m.extractFunc != nil { + return m.extractFunc(ctx, req) + } + return m.key +} + +// MockReconciler is a mock implementation of Reconciler for testing +type MockReconciler struct { + reconcileFunc func(context.Context) (reconciler.Result, error) + result reconciler.Result + err error +} + +func (m *MockReconciler) Reconcile(ctx context.Context) (reconciler.Result, error) { + if m.reconcileFunc != nil { + return m.reconcileFunc(ctx) + } + return m.result, m.err +} + +func TestResult(t *testing.T) { + t.Run("Basic functionality", func(t *testing.T) { + t.Run("Without backoff", func(t *testing.T) { + // Create a Result with RequeueWithBackoff = false and a specific RequeueAfter duration + duration := 5 * time.Second + result := reconciler.Result{ + Result: reconcile.Result{ + RequeueAfter: duration, + }, + RequeueWithBackoff: false, + } + + // Verify that the wrapped Result has the same RequeueAfter value + assert.Equal(t, duration, result.RequeueAfter) + assert.False(t, result.RequeueWithBackoff) + }) + + t.Run("With requeue", func(t *testing.T) { + // Create a Result with Requeue = true and RequeueWithBackoff = false + result := reconciler.Result{ + Result: reconcile.Result{ + Requeue: true, + }, + RequeueWithBackoff: false, + } + + // Verify that the wrapped Result has Requeue = true + assert.True(t, result.Requeue) + assert.False(t, result.RequeueWithBackoff) + }) + }) +} + +func TestAsGenericReconciler(t *testing.T) { + t.Run("With string key", func(t *testing.T) { + t.Run("Result with backoff", func(t *testing.T) { + // Create a mock rate limiter + backoffDuration := 10 * time.Second + mockRateLimiter := &MockRateLimiter[string]{ + backoffDuration: backoffDuration, + } + + // Create a mock reconcile function that returns a Result with RequeueWithBackoff = true + reconcileFunc := func(ctx context.Context, req reconcile.Request) (reconciler.Result, error) { + return reconciler.Result{ + Result: reconcile.Result{}, + RequeueWithBackoff: true, + }, nil + } + + // Create a mock key extractor + testKey := "test-controller" + mockKeyExtractor := &MockKeyExtractor[string]{ + key: testKey, + } + + // Create the reconciler adapter + adapter := reconciler.AsGenericReconcilerWithRateLimiter( + reconcileFunc, + mockKeyExtractor, + mockRateLimiter, + ) + + // Call the adapter + ctx := context.Background() + req := reconcile.Request{} + result, err := adapter.Reconcile(ctx, req) + + // Verify the result + assert.NoError(t, err) + assert.Equal(t, backoffDuration, result.RequeueAfter) + assert.False(t, result.Requeue) + assert.Equal(t, 1, mockRateLimiter.NumRequeues(testKey)) + }) + + t.Run("Multiple backoffs", func(t *testing.T) { + // Create a mock rate limiter that increases backoff duration + initialBackoff := 1 * time.Second + mockLimiter := &MockRateLimiter[string]{} + mockLimiter.whenFunc = func(key string) time.Duration { + mockLimiter.numRequeues++ + return time.Duration(mockLimiter.numRequeues) * initialBackoff + } + + // Create a mock reconcile function that returns a Result with RequeueWithBackoff = true + reconcileFunc := func(ctx context.Context, req reconcile.Request) (reconciler.Result, error) { + return reconciler.Result{ + Result: reconcile.Result{}, + RequeueWithBackoff: true, + }, nil + } + + // Create a mock key extractor + testKey := "test-controller" + mockKeyExtractor := &MockKeyExtractor[string]{ + key: testKey, + } + + // Create the reconciler adapter + adapter := reconciler.AsGenericReconcilerWithRateLimiter( + reconcileFunc, + mockKeyExtractor, + mockLimiter, + ) + + // Call the adapter multiple times + ctx := context.Background() + req := reconcile.Request{} + + // First call + result1, err := adapter.Reconcile(ctx, req) + assert.NoError(t, err) + assert.Equal(t, 1*initialBackoff, result1.RequeueAfter) + + // Second call + result2, err := adapter.Reconcile(ctx, req) + assert.NoError(t, err) + assert.Equal(t, 2*initialBackoff, result2.RequeueAfter) + + // Third call + result3, err := adapter.Reconcile(ctx, req) + assert.NoError(t, err) + assert.Equal(t, 3*initialBackoff, result3.RequeueAfter) + }) + }) + + t.Run("With request key", func(t *testing.T) { + t.Run("Result with backoff", func(t *testing.T) { + // Create a mock rate limiter + backoffDuration := 10 * time.Second + mockRateLimiter := &MockRateLimiter[reconcile.Request]{ + backoffDuration: backoffDuration, + } + + // Create a mock reconcile function that returns a Result with RequeueWithBackoff = true + reconcileFunc := func(ctx context.Context, req reconcile.Request) (reconciler.Result, error) { + return reconciler.Result{ + Result: reconcile.Result{}, + RequeueWithBackoff: true, + }, nil + } + + // Create a mock key extractor + testReq := reconcile.Request{} + mockKeyExtractor := &MockKeyExtractor[reconcile.Request]{ + key: testReq, + } + + // Create the reconciler adapter + adapter := reconciler.AsGenericReconcilerWithRateLimiter( + reconcileFunc, + mockKeyExtractor, + mockRateLimiter, + ) + + // Call the adapter + ctx := context.Background() + result, err := adapter.Reconcile(ctx, testReq) + + // Verify the result + assert.NoError(t, err) + assert.Equal(t, backoffDuration, result.RequeueAfter) + assert.False(t, result.Requeue) + assert.Equal(t, 1, mockRateLimiter.NumRequeues(testReq)) + }) + }) + + t.Run("Error handling", func(t *testing.T) { + t.Run("Error propagation", func(t *testing.T) { + // Create a mock reconcile function that returns an error + expectedErr := errors.New("test error") + reconcileFunc := func(ctx context.Context, req reconcile.Request) (reconciler.Result, error) { + return reconciler.Result{}, expectedErr + } + + // Create a mock key extractor + mockKeyExtractor := &MockKeyExtractor[string]{ + key: "test-controller", + } + + // Create the reconciler adapter + adapter := reconciler.AsGenericReconciler( + reconcileFunc, + mockKeyExtractor, + ) + + // Call the adapter + ctx := context.Background() + req := reconcile.Request{} + _, err := adapter.Reconcile(ctx, req) + + // Verify that the error is propagated + assert.Error(t, err) + assert.Equal(t, expectedErr, err) + }) + }) + + t.Run("No backoff", func(t *testing.T) { + t.Run("Result without backoff", func(t *testing.T) { + // Create a mock reconcile function that returns a Result with RequeueWithBackoff = false + expectedRequeueAfter := 5 * time.Second + reconcileFunc := func(ctx context.Context, req reconcile.Request) (reconciler.Result, error) { + return reconciler.Result{ + Result: reconcile.Result{ + RequeueAfter: expectedRequeueAfter, + }, + RequeueWithBackoff: false, + }, nil + } + + // Create a mock key extractor + mockKeyExtractor := &MockKeyExtractor[string]{ + key: "test-controller", + } + + // Create the reconciler adapter + adapter := reconciler.AsGenericReconciler( + reconcileFunc, + mockKeyExtractor, + ) + + // Call the adapter + ctx := context.Background() + req := reconcile.Request{} + result, err := adapter.Reconcile(ctx, req) + + // Verify the result + assert.NoError(t, err) + assert.Equal(t, expectedRequeueAfter, result.RequeueAfter) + assert.False(t, result.Requeue) + }) + }) +} + +func TestKeyExtractors(t *testing.T) { + t.Run("RequestKeyExtractor", func(t *testing.T) { + t.Run("Extract request key", func(t *testing.T) { + // Create a request key extractor + extractor := reconciler.RequestKeyExtractor{} + + // Extract the key + ctx := context.Background() + req := reconcile.Request{} + key := extractor.Extract(ctx, req) + + // Verify the key + assert.Equal(t, req, key) + }) + }) +} + +func TestAsReconciler(t *testing.T) { + t.Run("Standard reconciler adapter", func(t *testing.T) { + // Create a mock reconciler + mockReconciler := &MockReconciler{ + result: reconciler.Result{ + Result: reconcile.Result{ + RequeueAfter: 5 * time.Second, + }, + RequeueWithBackoff: false, + }, + } + + // Create the reconciler adapter + adapter := reconciler.AsReconciler(mockReconciler) + + // Call the adapter + ctx := context.Background() + req := reconcile.Request{} + result, err := adapter.Reconcile(ctx, req) + + // Verify the result + assert.NoError(t, err) + assert.Equal(t, 5*time.Second, result.RequeueAfter) + assert.False(t, result.Requeue) + }) +} + +func TestReconcilerFunc(t *testing.T) { + t.Run("Implementation", func(t *testing.T) { + // Create a ReconcilerFunc + expectedResult := reconciler.Result{ + Result: reconcile.Result{ + RequeueAfter: 5 * time.Second, + }, + RequeueWithBackoff: false, + } + expectedErr := errors.New("test error") + + reconcileFunc := reconciler.ReconcilerFunc(func(ctx context.Context) (reconciler.Result, error) { + return expectedResult, expectedErr + }) + + // Call the function + ctx := context.Background() + result, err := reconcileFunc.Reconcile(ctx) + + // Verify the result + assert.Equal(t, expectedResult, result) + assert.Equal(t, expectedErr, err) + }) +} diff --git a/singleton/controller.go b/singleton/controller.go index d1cb1dc..fc0285a 100644 --- a/singleton/controller.go +++ b/singleton/controller.go @@ -4,6 +4,7 @@ import ( "context" "time" + "github.com/awslabs/operatorpkg/reconciler" "k8s.io/client-go/util/workqueue" "sigs.k8s.io/controller-runtime/pkg/event" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -17,16 +18,48 @@ const ( RequeueImmediately = 1 * time.Nanosecond ) +// Reconciler defines the interface for singleton reconcilers type Reconciler interface { - Reconcile(ctx context.Context) (reconcile.Result, error) + Name() string + Reconcile(ctx context.Context) (reconciler.Result, error) } -func AsReconciler(reconciler Reconciler) reconcile.Reconciler { - return reconcile.Func(func(ctx context.Context, r reconcile.Request) (reconcile.Result, error) { - return reconciler.Reconcile(ctx) - }) +// StringKeyExtractor extracts the controller name as the rate limiter key +type StringKeyExtractor struct { + reconciler Reconciler +} + +// Extract returns the controller name as the key +func (e StringKeyExtractor) Extract(ctx context.Context, req reconcile.Request) string { + return e.reconciler.Name() +} + +// In response to Requeue: True being deprecated via: https://github.com/kubernetes-sigs/controller-runtime/pull/3107/files +// This uses a bucket and per item delay but the item will be the same because the key is the controller name. +// This implements the same behavior as Requeue: True. + +// AsReconciler creates a controller-runtime reconciler from a singleton reconciler +func AsReconciler(rec Reconciler) reconcile.Reconciler { + return reconciler.AsGenericReconciler( + func(ctx context.Context, req reconcile.Request) (reconciler.Result, error) { + return rec.Reconcile(ctx) + }, + StringKeyExtractor{reconciler: rec}, + ) +} + +// AsReconcilerWithRateLimiter creates a controller-runtime reconciler with a custom rate limiter +func AsReconcilerWithRateLimiter(rec Reconciler, rateLimiter workqueue.TypedRateLimiter[string]) reconcile.Reconciler { + return reconciler.AsGenericReconcilerWithRateLimiter( + func(ctx context.Context, req reconcile.Request) (reconciler.Result, error) { + return rec.Reconcile(ctx) + }, + StringKeyExtractor{reconciler: rec}, + rateLimiter, + ) } +// Source creates a source for singleton controllers func Source() source.Source { eventSource := make(chan event.GenericEvent, 1) eventSource <- event.GenericEvent{} diff --git a/singleton/suite_test.go b/singleton/suite_test.go new file mode 100644 index 0000000..0e10b3c --- /dev/null +++ b/singleton/suite_test.go @@ -0,0 +1,153 @@ +package singleton_test + +import ( + "context" + "errors" + "testing" + "time" + + "github.com/awslabs/operatorpkg/reconciler" + "github.com/awslabs/operatorpkg/singleton" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +func Test(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Singleton") +} + +var ( + mockReconciler *MockReconciler + rec singleton.Reconciler +) + +// MockReconciler for testing +type MockReconciler struct { + name string + result reconciler.Result + err error +} + +func (m *MockReconciler) Name() string { + return m.name +} + +func (m *MockReconciler) Reconcile(ctx context.Context) (reconciler.Result, error) { + return m.result, m.err +} + +var _ = Describe("Singleton Controller", func() { + Context("AsReconciler with rate limiting", func() { + BeforeEach(func() { + mockReconciler = &MockReconciler{ + name: "test-controller", + } + }) + + Context("when RequeueWithBackoff is false", func() { + It("should return the original result without backoff", func() { + mockReconciler.result = reconciler.Result{ + RequeueWithBackoff: false, + } + rec = mockReconciler + result, err := rec.Reconcile(context.Background()) + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(BeZero()) + }) + It("should return the original result without backoff when RequeueWithBackoff is not set", func() { + mockReconciler.result = reconciler.Result{} + rec = mockReconciler + result, err := rec.Reconcile(context.Background()) + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(BeZero()) + }) + }) + + Context("when RequeueWithBackoff is true", func() { + BeforeEach(func() { + mockReconciler.result = reconciler.Result{ + RequeueWithBackoff: true, + } + rec = mockReconciler + }) + + It("should return a result with RequeueAfter set", func() { + result, err := rec.Reconcile(context.Background()) + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(BeNumerically(">=", 0)) + }) + + It("should use controller name for rate limiting", func() { + // Test with different controller names to ensure they're handled independently + controller1 := &MockReconciler{ + name: "controller-1", + result: reconciler.Result{RequeueWithBackoff: true}, + } + controller2 := &MockReconciler{ + name: "controller-2", + result: reconciler.Result{RequeueWithBackoff: true}, + } + + reconciler1 := singleton.AsReconciler(controller1) + reconciler2 := singleton.AsReconciler(controller2) + + // Each controller should get its own rate limiting + result1, err1 := reconciler1.Reconcile(context.Background(), reconcile.Request{}) + result2, err2 := reconciler2.Reconcile(context.Background(), reconcile.Request{}) + + Expect(err1).NotTo(HaveOccurred()) + Expect(err2).NotTo(HaveOccurred()) + Expect(result1.RequeueAfter).To(BeNumerically(">=", 0)) + Expect(result2.RequeueAfter).To(BeNumerically(">=", 0)) + }) + + It("should implement exponential backoff on repeated calls", func() { + // Multiple calls to the same controller should show increasing delays + delays := make([]time.Duration, 5) + + for i := 0; i < 5; i++ { + result, err := rec.Reconcile(context.Background()) + Expect(err).NotTo(HaveOccurred()) + delays[i] = result.RequeueAfter + } + + // Verify generally increasing pattern (allowing for some variance in rate limiting) + for i := 1; i < len(delays); i++ { + Expect(delays[i]).To(BeNumerically(">=", delays[i-1]), + "Delay at index %d (%v) should be >= delay at index %d (%v)", + i, delays[i], i-1, delays[i-1]) + } + }) + }) + + Context("when reconciler returns an error", func() { + BeforeEach(func() { + mockReconciler.result = reconciler.Result{RequeueWithBackoff: true} + mockReconciler.err = errors.New("test error") + rec = mockReconciler + }) + + It("should return the error without processing backoff", func() { + result, err := rec.Reconcile(context.Background()) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(Equal("test error")) + Expect(result.RequeueAfter).To(BeZero()) + }) + }) + + Context("integration with RequeueImmediately constant", func() { + It("should work with immediate requeue pattern", func() { + mockReconciler.result = reconciler.Result{ + Result: reconcile.Result{RequeueAfter: singleton.RequeueImmediately}, + } + rec = mockReconciler + + result, err := rec.Reconcile(context.Background()) + Expect(err).NotTo(HaveOccurred()) + Expect(result.RequeueAfter).To(Equal(singleton.RequeueImmediately)) + }) + }) + }) +})