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

Skip manifest changes if item is in exponential backoff #225

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kovayur
Copy link

@kovayur kovayur commented Jul 31, 2023

Fixes pt. 2 in #224

Filter manifest changes when controller has entered exponential backoff. This prevents any further updates from being made after the rollback.

@@ -0,0 +1,61 @@
/*
Copyright 2023.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Copyright 2023.
Copyright 2023 The Operator-SDK Authors.

crthandler.EventHandler
}

// RequeueFilter wraps the EventHandler and skips the event if it was requeued for the given object
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// RequeueFilter wraps the EventHandler and skips the event if it was requeued for the given object
// RequeueFilter wraps the EventHandler but skips the event if it was requeued for the given object.

}

func (q *delegatingQueue) Add(item interface{}) {
if q.RateLimitingInterface.NumRequeues(item) > 0 {
Copy link
Member

Choose a reason for hiding this comment

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

The documentation is rather terse on exact semantics of NumRequeues, and what exactly is considered to be a requeue, so this looks scary to me. For example, does it mean errors will never be retried (apart from the periodic multiple-hour resync)?

@openshift-merge-robot openshift-merge-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 13, 2024
@openshift-merge-robot
Copy link

PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/sdk needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants