fixup! fix race condition

Signed-off-by: Bryce Palmer <bpalmer@redhat.com>

Author: everettraven

pkg/controllers/switchedcontroller/switched_controller.go

@@ -106,7 +106,6 @@ func (c *ControllerWithSwitch) sync(ctx context.Context, syncCtx factory.SyncCon
 		c.mutex.Lock()
 		c.switchContextCancel()
 		c.switchContext = nil
-		c.mutex.Unlock()
 
 		// Server-Side-Apply with an empty operator status for the delegate's field manager
 		// will remove any conditions owned by it since the list type in the API definition
@@ -117,6 +116,8 @@ func (c *ControllerWithSwitch) sync(ctx context.Context, syncCtx factory.SyncCon
 			return fmt.Errorf("clearing delegate %q conditions: %v", c.delegateName, err)
 		}
 
+		c.mutex.Unlock()
+
 	default:
 		var e error
 		c.mutex.Lock()

pkg/controllers/switchedcontroller/switched_controller_test.go

@@ -3,6 +3,7 @@ package switchedcontroller
 import (
 	"context"
 	"fmt"
+	"sync"
 	"testing"
 	"time"
 
@@ -394,6 +395,27 @@ type applyStatusCall struct {
 	applyConfiguration *applyoperatorv1.OperatorStatusApplyConfiguration
 }
 
+// synchronizedOperatorClient serializes access to a non-thread-safe OperatorClient
+// (e.g. fakeOperatorClient) so it can be used safely from multiple goroutines.
+type synchronizedOperatorClient struct {
+	v1helpers.OperatorClient
+	mu            sync.Mutex
+	statusApplied chan struct{}
+}
+
+func (c *synchronizedOperatorClient) ApplyOperatorStatus(ctx context.Context, fieldManager string, applyConfiguration *applyoperatorv1.OperatorStatusApplyConfiguration) error {
+	c.mu.Lock()
+	err := c.OperatorClient.ApplyOperatorStatus(ctx, fieldManager, applyConfiguration)
+	c.mu.Unlock()
+	if c.statusApplied != nil {
+		select {
+		case c.statusApplied <- struct{}{}:
+		default:
+		}
+	}
+	return err
+}
+
 type recordingOperatorClient struct {
 	v1helpers.OperatorClient
 	applyStatusCalls []applyStatusCall
@@ -405,11 +427,13 @@ func (r *recordingOperatorClient) ApplyOperatorStatus(ctx context.Context, field
 }
 
 func TestSwitchedControllerClearsDelegateConditionsOnSwitchOff(t *testing.T) {
-	delegateSynced := make(chan struct{})
-
 	recorder := events.NewInMemoryRecorder("switchedcontroller_test", clocktesting.NewFakePassiveClock(time.Now()))
 	fakeClient := v1helpers.NewFakeOperatorClient(&operatorv1.OperatorSpec{ManagementState: operatorv1.Managed}, &operatorv1.OperatorStatus{}, nil)
-	operatorClient := &recordingOperatorClient{OperatorClient: fakeClient}
+	syncClient := &synchronizedOperatorClient{
+		OperatorClient: fakeClient,
+		statusApplied:  make(chan struct{}, 1),
+	}
+	operatorClient := &recordingOperatorClient{OperatorClient: syncClient}
 
 	delegateFn := func(_ context.Context) *factory.Factory {
 		var enqueueSyncHook factory.PostStartHook = func(ctx context.Context, syncContext factory.SyncContext) error {
@@ -418,13 +442,9 @@ func TestSwitchedControllerClearsDelegateConditionsOnSwitchOff(t *testing.T) {
 		}
 		return factory.New().
 			WithSync(func(ctx context.Context, controllerContext factory.SyncContext) error {
-				select {
-				case delegateSynced <- struct{}{}:
-				default:
-				}
 				return fmt.Errorf("route.route.openshift.io \"oauth-openshift\" not found")
 			}).
-			WithSyncDegradedOnError(fakeClient).
+			WithSyncDegradedOnError(syncClient).
 			WithPostStartHooks(enqueueSyncHook)
 	}
 
@@ -449,31 +469,16 @@ func TestSwitchedControllerClearsDelegateConditionsOnSwitchOff(t *testing.T) {
 		t.Fatalf("unexpected error when syncing: %v", err)
 	}
 
-	// Wait for the delegate to sync at least once and set a degraded condition
+	// Wait for the delegate's reportDegraded to call ApplyOperatorStatus.
+	// This signal fires after reportDegraded completes (not just after sync returns),
+	// which avoids a data race on the non-thread-safe fakeOperatorClient.
 	syncTimeoutCtx, syncTimeoutCancel := context.WithTimeout(t.Context(), 500*time.Millisecond)
 	defer syncTimeoutCancel()
 
 	select {
 	case <-syncTimeoutCtx.Done():
-		t.Fatal("timed out waiting for delegate controller to sync")
-	case <-delegateSynced:
-	}
-
-	// Verify the delegate set a degraded condition
-	_, status, _, err := operatorClient.GetOperatorState()
-	if err != nil {
-		t.Fatalf("unexpected error getting operator state: %v", err)
-	}
-
-	foundDegraded := false
-	for _, cond := range status.Conditions {
-		if cond.Type == "test-controllerDegraded" && cond.Status == operatorv1.ConditionTrue {
-			foundDegraded = true
-			break
-		}
-	}
-	if !foundDegraded {
-		t.Fatal("expected test-controllerDegraded condition to be True after delegate sync error")
+		t.Fatal("timed out waiting for delegate controller to report degraded status")
+	case <-syncClient.statusApplied:
 	}
 
 	// Switch off and sync the wrapper — this should clear delegate conditions