From a5e3f503f40e25ff108ad540fd26373ffd9ff164 Mon Sep 17 00:00:00 2001 From: Shubham Singh Date: Sun, 2 Aug 2026 14:36:42 +0530 Subject: [PATCH] fix(testing): make FakeClient thread-safe Signed-off-by: Shubham Singh --- pkg/client/testing/fake_client.go | 65 +++++++++++++++++++------- pkg/client/testing/fake_client_test.go | 31 ++++++++++++ 2 files changed, 78 insertions(+), 18 deletions(-) diff --git a/pkg/client/testing/fake_client.go b/pkg/client/testing/fake_client.go index f5b80b33c..3aacd7d1c 100644 --- a/pkg/client/testing/fake_client.go +++ b/pkg/client/testing/fake_client.go @@ -2,14 +2,15 @@ package testing import ( "context" + "sync" "k8s.io/apimachinery/pkg/api/meta" "k8s.io/apimachinery/pkg/runtime" client "sigs.k8s.io/controller-runtime/pkg/client" ) -// TODO: not thread safe type FakeClient struct { + mu sync.RWMutex GetFn func(ctx context.Context, call int, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error CreateFn func(ctx context.Context, call int, obj client.Object, opts ...client.CreateOption) error UpdateFn func(ctx context.Context, call int, obj client.Object, opts ...client.UpdateOption) error @@ -23,47 +24,73 @@ type FakeClient struct { } func (c *FakeClient) Get(ctx context.Context, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { - defer func() { c.numCalls++ }() - return c.GetFn(ctx, c.numCalls, key, obj, opts...) + c.mu.Lock() + call := c.numCalls + c.numCalls++ + c.mu.Unlock() + return c.GetFn(ctx, call, key, obj, opts...) } func (c *FakeClient) List(ctx context.Context, list client.ObjectList, opts ...client.ListOption) error { - defer func() { c.numCalls++ }() - return c.ListFn(ctx, c.numCalls, list, opts...) + c.mu.Lock() + call := c.numCalls + c.numCalls++ + c.mu.Unlock() + return c.ListFn(ctx, call, list, opts...) } func (c *FakeClient) Create(ctx context.Context, obj client.Object, opts ...client.CreateOption) error { - defer func() { c.numCalls++ }() - return c.CreateFn(ctx, c.numCalls, obj, opts...) + c.mu.Lock() + call := c.numCalls + c.numCalls++ + c.mu.Unlock() + return c.CreateFn(ctx, call, obj, opts...) } func (c *FakeClient) Update(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - defer func() { c.numCalls++ }() - return c.UpdateFn(ctx, c.numCalls, obj, opts...) + c.mu.Lock() + call := c.numCalls + c.numCalls++ + c.mu.Unlock() + return c.UpdateFn(ctx, call, obj, opts...) } func (c *FakeClient) Delete(ctx context.Context, obj client.Object, opts ...client.DeleteOption) error { - defer func() { c.numCalls++ }() - return c.DeleteFn(ctx, c.numCalls, obj, opts...) + c.mu.Lock() + call := c.numCalls + c.numCalls++ + c.mu.Unlock() + return c.DeleteFn(ctx, call, obj, opts...) } func (c *FakeClient) Patch(ctx context.Context, obj client.Object, patch client.Patch, opts ...client.PatchOption) error { - defer func() { c.numCalls++ }() - return c.PatchFn(ctx, c.numCalls, obj, patch, opts...) + c.mu.Lock() + call := c.numCalls + c.numCalls++ + c.mu.Unlock() + return c.PatchFn(ctx, call, obj, patch, opts...) } func (c *FakeClient) IsObjectNamespaced(obj runtime.Object) (bool, error) { - defer func() { c.numCalls++ }() - return c.IsObjectNamespacedFn(c.numCalls, obj) + c.mu.Lock() + call := c.numCalls + c.numCalls++ + c.mu.Unlock() + return c.IsObjectNamespacedFn(call, obj) } func (c *FakeClient) RESTMapper() meta.RESTMapper { - defer func() { c.numCalls++ }() - return c.RESTMapperFn(c.numCalls) + c.mu.Lock() + call := c.numCalls + c.numCalls++ + c.mu.Unlock() + return c.RESTMapperFn(call) } func (c *FakeClient) SubResource(subResource string) client.SubResourceClient { - defer func() { c.numCalls++ }() + c.mu.Lock() + c.numCalls++ + c.mu.Unlock() if c.SubResourceFn != nil { return c.SubResourceFn(subResource) } @@ -71,6 +98,8 @@ func (c *FakeClient) SubResource(subResource string) client.SubResourceClient { } func (c *FakeClient) NumCalls() int { + c.mu.RLock() + defer c.mu.RUnlock() return c.numCalls } diff --git a/pkg/client/testing/fake_client_test.go b/pkg/client/testing/fake_client_test.go index 12e901dc2..67410bb3b 100644 --- a/pkg/client/testing/fake_client_test.go +++ b/pkg/client/testing/fake_client_test.go @@ -153,6 +153,37 @@ func TestFakeClient(t *testing.T) { assert.NotNil(t, sw) assert.Equal(t, 1, c.NumCalls()) }) + + t.Run("Concurrent calls", func(t *testing.T) { + c := &FakeClient{ + GetFn: func(ctx context.Context, call int, key client.ObjectKey, obj client.Object, opts ...client.GetOption) error { + return nil + }, + CreateFn: func(ctx context.Context, call int, obj client.Object, opts ...client.CreateOption) error { + return nil + }, + } + + const numGoroutines = 50 + done := make(chan bool, numGoroutines*2) + + for i := 0; i < numGoroutines; i++ { + go func() { + _ = c.Get(context.Background(), client.ObjectKey{}, nil) + done <- true + }() + go func() { + _ = c.Create(context.Background(), nil) + done <- true + }() + } + + for i := 0; i < numGoroutines*2; i++ { + <-done + } + + assert.Equal(t, numGoroutines*2, c.NumCalls()) + }) } func TestFakeSubResourceWriter(t *testing.T) {