Skip to content

Commit

Permalink
Fix race condition in Label Identity Reconciler UT (#4296)
Browse files Browse the repository at this point in the history
Data race caused by worker reading and writing r.localClusterID
at the same time. Refactored the reconcile loop so that the
localClusterID is only set when it is missing.

Signed-off-by: Dyanngg <[email protected]>
  • Loading branch information
Dyanngg authored Oct 18, 2022
1 parent 0a7fd95 commit 5e96c8c
Showing 1 changed file with 19 additions and 7 deletions.
26 changes: 19 additions & 7 deletions multicluster/controllers/multicluster/label_identity_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ type (
LabelIdentityReconciler struct {
client.Client
Scheme *runtime.Scheme
commonAreaMutex sync.Mutex
commonAreaGetter RemoteCommonAreaGetter
remoteCommonArea commonarea.RemoteCommonArea
// labelMutex prevents concurrent access to labelToPodsCache and podLabelCache.
Expand Down Expand Up @@ -87,16 +88,11 @@ func NewLabelIdentityReconciler(
// +kubebuilder:rbac:groups="",resources=namespaces,verbs=get;list;watch
func (r *LabelIdentityReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) {
klog.V(2).InfoS("Reconciling Pod for label identity", "pod", req.NamespacedName)
var commonArea commonarea.RemoteCommonArea
var err error
commonArea, r.localClusterID, err = r.commonAreaGetter.GetRemoteCommonAreaAndLocalID()
if commonArea == nil {
return ctrl.Result{Requeue: true}, err
if requeue := r.checkRemoteCommonArea(); requeue {
return ctrl.Result{Requeue: true}, nil
}
r.remoteCommonArea = commonArea
var pod v1.Pod
var ns v1.Namespace

if err := r.Client.Get(ctx, req.NamespacedName, &pod); err != nil {
if apierrors.IsNotFound(err) {
klog.V(2).InfoS("Pod is deleted", "pod", req.NamespacedName)
Expand All @@ -114,6 +110,22 @@ func (r *LabelIdentityReconciler) Reconcile(ctx context.Context, req ctrl.Reques
return ctrl.Result{}, nil
}

// checkRemoteCommonArea initializes remoteCommonArea for the reconciler if necessary,
// or tells the Reconcile function to requeue if the remoteCommonArea is not ready.
func (r *LabelIdentityReconciler) checkRemoteCommonArea() bool {
r.commonAreaMutex.Lock()
defer r.commonAreaMutex.Unlock()

if r.remoteCommonArea == nil {
commonArea, localClusterID, _ := r.commonAreaGetter.GetRemoteCommonAreaAndLocalID()
if commonArea == nil {
return true
}
r.remoteCommonArea, r.localClusterID = commonArea, localClusterID
}
return false
}

// SetupWithManager sets up the controller with the Manager.
func (r *LabelIdentityReconciler) SetupWithManager(mgr ctrl.Manager) error {
return ctrl.NewControllerManagedBy(mgr).
Expand Down

0 comments on commit 5e96c8c

Please sign in to comment.