Skip to content

Commit 4380117

Browse files
OlivierCazadeclaude
andcommitted
Fix potential race condition in concurrent reconciliation test
Move assertion out of goroutines to avoid panic from calling t.FailNow() in non-test goroutine. Collect errors via channel and check them in the main test goroutine. Filter out 409 conflict errors which are expected when multiple goroutines concurrently update the same resource status in the fake client. Co-Authored-By: Claude Sonnet 4.5 <[email protected]>
1 parent acadaca commit 4380117

1 file changed

Lines changed: 15 additions & 6 deletions

File tree

pkg/controller/observability/observability_controller_test.go

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import (
1313
configv1 "github.com/openshift/api/config/v1"
1414
"github.com/openshift/cluster-network-operator/pkg/controller/statusmanager"
1515
corev1 "k8s.io/api/core/v1"
16+
"k8s.io/apimachinery/pkg/api/errors"
1617
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1718
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
1819
"k8s.io/apimachinery/pkg/runtime"
@@ -1266,20 +1267,28 @@ func TestReconcile_ConcurrentReconciliations(t *testing.T) {
12661267
req := ctrl.Request{NamespacedName: types.NamespacedName{Name: "cluster"}}
12671268

12681269
// Run 5 concurrent reconciliations
1269-
done := make(chan bool, 5)
1270+
errChan := make(chan error, 5)
12701271
for i := 0; i < 5; i++ {
12711272
go func() {
12721273
_, err := r.Reconcile(context.TODO(), req)
1273-
// All should complete without error (idempotent)
1274-
g.Expect(err).NotTo(HaveOccurred())
1275-
done <- true
1274+
errChan <- err
12761275
}()
12771276
}
12781277

1279-
// Wait for all to complete
1278+
// Wait for all to complete and collect errors
1279+
var unexpectedErrors []error
12801280
for i := 0; i < 5; i++ {
1281-
<-done
1281+
if err := <-errChan; err != nil {
1282+
// Filter out 409 conflict errors which are expected when multiple
1283+
// goroutines try to update the same resource status concurrently
1284+
if !errors.IsConflict(err) {
1285+
unexpectedErrors = append(unexpectedErrors, err)
1286+
}
1287+
}
12821288
}
1289+
1290+
// Assert no unexpected errors occurred (safe to do in main test goroutine)
1291+
g.Expect(unexpectedErrors).To(BeEmpty(), "All concurrent reconciliations should complete without unexpected errors")
12831292
}
12841293

12851294
// Status Manager Tests

0 commit comments

Comments
 (0)