Skip to content

Bug 1889838: fix race in Operator reconcilation #1823

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 21, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions pkg/controller/operators/operator_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,16 @@ func (r *OperatorReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
}
}

if !create {
if rv, ok := r.getLastResourceVersion(req.NamespacedName); ok && rv == in.ResourceVersion {
log.V(1).Info("Operator is already up-to-date")
return reconcile.Result{}, nil
}
rv, ok := r.getLastResourceVersion(req.NamespacedName)
if !create && ok && rv == in.ResourceVersion {
log.V(1).Info("Operator is already up-to-date")
return reconcile.Result{}, nil
}

// Set the cached resource version to 0 so we can handle
// the race with requests enqueuing via mapComponentRequests
r.setLastResourceVersion(req.NamespacedName, "0")

// Wrap with convenience decorator
operator, err := r.factory.NewOperator(in)
if err != nil {
Expand All @@ -162,7 +165,10 @@ func (r *OperatorReconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
}
}

r.setLastResourceVersion(req.NamespacedName, operator.GetResourceVersion())
// Only set the resource version if it already exists.
// If it does not exist, it means mapComponentRequests was called
// while we were reconciling and we need to reconcile again
r.setLastResourceVersionIfExists(req.NamespacedName, operator.GetResourceVersion())

return ctrl.Result{}, nil
}
Expand Down Expand Up @@ -224,6 +230,14 @@ func (r *OperatorReconciler) setLastResourceVersion(name types.NamespacedName, r
r.lastResourceVersion[name] = rv
}

func (r *OperatorReconciler) setLastResourceVersionIfExists(name types.NamespacedName, rv string) {
r.mu.Lock()
defer r.mu.Unlock()
if _, ok := r.lastResourceVersion[name]; ok {
r.lastResourceVersion[name] = rv
}
}

func (r *OperatorReconciler) unsetLastResourceVersion(name types.NamespacedName) {
r.mu.Lock()
defer r.mu.Unlock()
Expand Down