Skip to content

errors: add SilentlyRequeueOnConflicts#556

Merged
turkenh merged 2 commits intocrossplane:masterfrom
sttts:sttts-silently-ignore-conflicts
Oct 2, 2023
Merged

errors: add SilentlyRequeueOnConflicts#556
turkenh merged 2 commits intocrossplane:masterfrom
sttts:sttts-silently-ignore-conflicts

Conversation

@sttts
Copy link
Copy Markdown
Contributor

@sttts sttts commented Sep 21, 2023

Description of your changes

Example:

func Reconcile(...) (reconcile.Result, error) {
    ...

    return errors.SilentlyRequeueOnConflict(reconcile.Result{}, r.hcClient.Patch(ctx, obj, client.MergeFrom(orig)))
}

If the returned error is a (possibly wrapped) Kube conflict error, it is dropped (replaced with nil) and the result is replaced with an immediate requeue.

This way, the conflict errors, which are part of the Kube's optimistic concurrency protocol and not actual errors, do not show up as ugly backtrace in the logs.

Alternative way to use:

errors.WithSilentRequeueOnConflict(NewReconciler())

I have:

  • Read and followed Crossplane's contribution process.
  • Run make reviewable test to ensure this PR is ready for review.

How has this code been tested

Unit tested.

@sttts sttts requested review from a team as code owners September 21, 2023 18:44
@sttts sttts requested review from bobh66 and lsviben September 21, 2023 18:44
@sttts sttts force-pushed the sttts-silently-ignore-conflicts branch from 18e6832 to 7ae4423 Compare September 21, 2023 18:52
Copy link
Copy Markdown
Contributor

@lsviben lsviben left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks @sttts. Added just a comment about license boilerplate that should be fixed before merging

Comment on lines +1 to +2
// Copyright 2023 Upbound Inc.
// All rights reserved
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be the Crossplane copyright stuff here.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@sttts sttts force-pushed the sttts-silently-ignore-conflicts branch from 14bed81 to f591d3b Compare September 26, 2023 13:44
Signed-off-by: Dr. Stefan Schimanski <stefan.schimanski@upbound.io>
Signed-off-by: Dr. Stefan Schimanski <stefan.schimanski@upbound.io>
@sttts sttts force-pushed the sttts-silently-ignore-conflicts branch from f591d3b to 8333c5c Compare September 26, 2023 14:21
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

Successfully merging this pull request may close these issues.

4 participants