Skip to content

Commit

Permalink
Refactor ClusterExtensionReconciler to improve conflict resolution an…
Browse files Browse the repository at this point in the history
…d data consistency

The error `the object has been modified; please apply your changes to the latest version and try again` occurs when we attempt to update a resource that has changed since it was initially fetched.

These changes ensure that we fetch the current state of the resource just before issuing updates, rather than using the version obtained at the start of reconciliation. Additionally:

- Deep copy the ClusterExtension object before reconciliation to preserve the original state.
- Re-fetch the latest resource version before updating status and finalizers to handle conflicts from concurrent updates.

Closes: operator-framework#1414
  • Loading branch information
camilamacedo86 committed Nov 12, 2024
1 parent aaa0e00 commit ea7ec88
Showing 1 changed file with 11 additions and 12 deletions.
23 changes: 11 additions & 12 deletions internal/controllers/clusterextension_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,29 +113,28 @@ func (r *ClusterExtensionReconciler) Reconcile(ctx context.Context, req ctrl.Req
reconciledExt := existingExt.DeepCopy()
res, reconcileErr := r.reconcile(ctx, reconciledExt)

// Do checks before any Update()s, as Update() may modify the resource structure!
updateStatus := !equality.Semantic.DeepEqual(existingExt.Status, reconciledExt.Status)
updateFinalizers := !equality.Semantic.DeepEqual(existingExt.Finalizers, reconciledExt.Finalizers)

// If any unexpected fields have changed, panic before updating the resource
unexpectedFieldsChanged := checkForUnexpectedFieldChange(*existingExt, *reconciledExt)
if unexpectedFieldsChanged {
panic("spec or metadata changed by reconciler")
}

// Save the finalizers off to the side. If we update the status, the reconciledExt will be updated
// to contain the new state of the ClusterExtension, which contains the status update, but (critically)
// does not contain the finalizers. After the status update, we need to re-add the finalizers to the
// reconciledExt before updating the object.
finalizers := reconciledExt.Finalizers
if updateStatus {
// Re-fetch the object to get the latest resource version
// This is necessary because the object may have been updated by another controller
// between the time we fetched it and the time we finished reconciling it
if err := r.Client.Get(ctx, req.NamespacedName, existingExt); err != nil {
return ctrl.Result{}, client.IgnoreNotFound(err)
}
if !equality.Semantic.DeepEqual(existingExt.Status, reconciledExt.Status) {
if err := r.Client.Status().Update(ctx, reconciledExt); err != nil {
reconcileErr = errors.Join(reconcileErr, fmt.Errorf("error updating status: %v", err))
}
}
reconciledExt.Finalizers = finalizers

if updateFinalizers {
if err := r.Client.Get(ctx, req.NamespacedName, existingExt); err != nil {
return ctrl.Result{}, client.IgnoreNotFound(err)
}
if !equality.Semantic.DeepEqual(existingExt.Finalizers, reconciledExt.Finalizers) {
if err := r.Client.Update(ctx, reconciledExt); err != nil {
reconcileErr = errors.Join(reconcileErr, fmt.Errorf("error updating finalizers: %v", err))
}
Expand Down

0 comments on commit ea7ec88

Please sign in to comment.