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

Refactors operator requeues #1967

Merged
merged 1 commit into from
Feb 21, 2022

Conversation

m1kola
Copy link
Contributor

@m1kola m1kola commented Feb 15, 2022

What this PR does / why we need it:

  • Adds the clarifying comment on requeues into the checker controller
  • Removes Requeue: true in places where we use RequeueAfter as it is has no effect.

Test plan for issue:

Unit tests updated where applicable.

Is there any documentation that needs to be updated for this PR?

No, just refactoring

* Adds the clarifying comment on requeues into the checker controller
* Removes `Requeue: true` in places where we use `RequeueAfter`
  as it is has no effect.
@@ -101,9 +101,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
deadline := t.Add(gracePeriod)
now := time.Now()
if deadline.After(now) {
return reconcile.Result{
RequeueAfter: deadline.Sub(now),
}, err
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In this case - err is always nil as we return above when err is not nil.

@m1kola m1kola mentioned this pull request Feb 15, 2022
Copy link
Collaborator

@bennerv bennerv left a comment

Choose a reason for hiding this comment

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

lgtm

@bennerv bennerv merged commit 4e4fd6c into Azure:master Feb 21, 2022
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