From 1a91f9b3f118382af58a18362eb30e49244c626f Mon Sep 17 00:00:00 2001 From: Terry Howe Date: Tue, 3 Mar 2026 14:20:49 -0700 Subject: [PATCH] fix: race condition in FailingKubeClient.GetWaiterWithOptions Add mutex to protect concurrent access to recordedWaitOptions slice in FailingKubeClient.GetWaiterWithOptions. The field is accessed from multiple goroutines during install/upgrade rollback-on-failure paths, causing data races detected by -race. Signed-off-by: Terry Howe --- pkg/action/hooks_test.go | 2 +- pkg/action/install_test.go | 2 +- pkg/action/release_testing_test.go | 2 +- pkg/action/rollback_test.go | 2 +- pkg/action/uninstall_test.go | 2 +- pkg/action/upgrade_test.go | 2 +- pkg/kube/fake/failing_kube_client.go | 20 ++++++++++++++++---- 7 files changed, 22 insertions(+), 10 deletions(-) diff --git a/pkg/action/hooks_test.go b/pkg/action/hooks_test.go index 4b9b5becb..644eb364b 100644 --- a/pkg/action/hooks_test.go +++ b/pkg/action/hooks_test.go @@ -490,5 +490,5 @@ data: is.NoError(err) // Verify that WaitOptions were passed to GetWaiter - is.NotEmpty(failer.RecordedWaitOptions, "WaitOptions should be passed to GetWaiter") + is.NotEmpty(failer.GetRecordedWaitOptions(), "WaitOptions should be passed to GetWaiter") } diff --git a/pkg/action/install_test.go b/pkg/action/install_test.go index e4d2b7376..f8e2f36e7 100644 --- a/pkg/action/install_test.go +++ b/pkg/action/install_test.go @@ -1295,5 +1295,5 @@ func TestInstallRelease_WaitOptionsPassedDownstream(t *testing.T) { is.NoError(err) // Verify that WaitOptions were passed to GetWaiter - is.NotEmpty(failer.RecordedWaitOptions, "WaitOptions should be passed to GetWaiter") + is.NotEmpty(failer.GetRecordedWaitOptions(), "WaitOptions should be passed to GetWaiter") } diff --git a/pkg/action/release_testing_test.go b/pkg/action/release_testing_test.go index ab35e104a..e747564ea 100644 --- a/pkg/action/release_testing_test.go +++ b/pkg/action/release_testing_test.go @@ -115,5 +115,5 @@ func TestReleaseTesting_WaitOptionsPassedDownstream(t *testing.T) { is.NoError(err) // Verify that WaitOptions were passed to GetWaiter - is.NotEmpty(failer.RecordedWaitOptions, "WaitOptions should be passed to GetWaiter") + is.NotEmpty(failer.GetRecordedWaitOptions(), "WaitOptions should be passed to GetWaiter") } diff --git a/pkg/action/rollback_test.go b/pkg/action/rollback_test.go index deb6c7c80..015eba739 100644 --- a/pkg/action/rollback_test.go +++ b/pkg/action/rollback_test.go @@ -81,5 +81,5 @@ func TestRollback_WaitOptionsPassedDownstream(t *testing.T) { is.NoError(err) // Verify that WaitOptions were passed to GetWaiter - is.NotEmpty(failer.RecordedWaitOptions, "WaitOptions should be passed to GetWaiter") + is.NotEmpty(failer.GetRecordedWaitOptions(), "WaitOptions should be passed to GetWaiter") } diff --git a/pkg/action/uninstall_test.go b/pkg/action/uninstall_test.go index aeac98142..98ef03704 100644 --- a/pkg/action/uninstall_test.go +++ b/pkg/action/uninstall_test.go @@ -204,5 +204,5 @@ func TestUninstall_WaitOptionsPassedDownstream(t *testing.T) { is.NoError(err) // Verify that WaitOptions were passed to GetWaiter - is.NotEmpty(failer.RecordedWaitOptions, "WaitOptions should be passed to GetWaiter") + is.NotEmpty(failer.GetRecordedWaitOptions(), "WaitOptions should be passed to GetWaiter") } diff --git a/pkg/action/upgrade_test.go b/pkg/action/upgrade_test.go index 393692976..73e7d5cce 100644 --- a/pkg/action/upgrade_test.go +++ b/pkg/action/upgrade_test.go @@ -800,5 +800,5 @@ func TestUpgradeRelease_WaitOptionsPassedDownstream(t *testing.T) { req.NoError(err) // Verify that WaitOptions were passed to GetWaiter - is.NotEmpty(failer.RecordedWaitOptions, "WaitOptions should be passed to GetWaiter") + is.NotEmpty(failer.GetRecordedWaitOptions(), "WaitOptions should be passed to GetWaiter") } diff --git a/pkg/kube/fake/failing_kube_client.go b/pkg/kube/fake/failing_kube_client.go index 0f7787f79..255840f6c 100644 --- a/pkg/kube/fake/failing_kube_client.go +++ b/pkg/kube/fake/failing_kube_client.go @@ -19,6 +19,7 @@ package fake import ( "io" + "sync" "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" @@ -47,8 +48,9 @@ type FailingKubeClient struct { WaitForDeleteError error WatchUntilReadyError error WaitDuration time.Duration - // RecordedWaitOptions stores the WaitOptions passed to GetWaiter for testing - RecordedWaitOptions []kube.WaitOption + // recordedWaitOptions stores the WaitOptions passed to GetWaiter for testing + recordedWaitOptions []kube.WaitOption + mu sync.Mutex } var _ kube.Interface = &FailingKubeClient{} @@ -158,9 +160,19 @@ func (f *FailingKubeClient) GetWaiter(ws kube.WaitStrategy) (kube.Waiter, error) return f.GetWaiterWithOptions(ws) } +// GetRecordedWaitOptions returns the recorded WaitOptions in a thread-safe manner. +func (f *FailingKubeClient) GetRecordedWaitOptions() []kube.WaitOption { + f.mu.Lock() + defer f.mu.Unlock() + dst := make([]kube.WaitOption, len(f.recordedWaitOptions)) + copy(dst, f.recordedWaitOptions) + return dst +} + func (f *FailingKubeClient) GetWaiterWithOptions(ws kube.WaitStrategy, opts ...kube.WaitOption) (kube.Waiter, error) { - // Record the WaitOptions for testing - f.RecordedWaitOptions = append(f.RecordedWaitOptions, opts...) + f.mu.Lock() + f.recordedWaitOptions = append(f.recordedWaitOptions, opts...) + f.mu.Unlock() waiter, _ := f.PrintingKubeClient.GetWaiterWithOptions(ws, opts...) printingKubeWaiter, _ := waiter.(*PrintingKubeWaiter) return &FailingKubeWaiter{