Skip to content
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

Use RetryOnConflict on Status().Update calls #109

Open
mpryc opened this issue Nov 7, 2024 · 0 comments
Open

Use RetryOnConflict on Status().Update calls #109

mpryc opened this issue Nov 7, 2024 · 0 comments
Assignees

Comments

@mpryc
Copy link
Collaborator

mpryc commented Nov 7, 2024

Optimistic concurrency control in k8s is separate for Spec and Status fields. We should use RetryOnConflict and get updated version of the object.

Sample implementation idea:

func (r *NonAdminBackupReconciler) updateStatusWithRetry(
    ctx context.Context,
    logger logr.Logger,
    nab *nacv1alpha1.NonAdminBackup,
    newPhase *nacv1alpha1.NonAdminBackupPhase,
    newCondition *metav1.Condition,
) (bool, error) {
    var updated bool

    err := retry.RetryOnConflict(retry.DefaultRetry, func() error {
        // Try to update with our current version first
        if newPhase != nil {
            updated = updateNonAdminPhase(&nab.Status.Phase, *newPhase) || updated
        }
        if newCondition != nil {
            updated = meta.SetStatusCondition(&nab.Status.Conditions, *newCondition) || updated
        }

        if !updated {
            return nil
        }

        updateErr := r.Status().Update(ctx, nab)
        if !apierrors.IsConflict(updateErr) {
            return updateErr // Either nil or non-conflict error
        }

        // Only fetch the latest version if we hit a conflict
        if err := r.Get(ctx, types.NamespacedName{Name: nab.Name, Namespace: nab.Namespace}, nab); err != nil {
            return err
        }

        // Reapply our changes to the fresh copy
        if newPhase != nil {
            updated = updateNonAdminPhase(&nab.Status.Phase, *newPhase)
        }
        if newCondition != nil {
            updated = meta.SetStatusCondition(&nab.Status.Conditions, *newCondition) || updated
        }

        return r.Status().Update(ctx, nab)
    })

    if err != nil {
        logger.Error(err, statusUpdateError)
        return false, err
    }

    return updated, nil
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant