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

By default requeue the resource with 10 hour delay #97

Merged

Conversation

vijtrip2
Copy link
Contributor

Issue #, if available: aws-controllers-k8s/community#1355

Description of changes:

  • implement ReSync using requeue.NeededAfter instead of upstream controlle-runime's SyncPeriod
  • ACK controllers do not use cached go-client for reading objects from APIServer, hence upstream controller-runime ReSync was not working. Upstream controller-runtime ReSync only works for cached objects.
  • As part of this change, whenever the RequeueOnSuccess duration for an ACK resource is 0 , requeue it with a 10 hour delay to trigger ReSync.
  • I did not change the default value of RequeueOnSuccessSeconds from 0 to 10 hours because RequeueOnSuccessSeconds = 0 still means resource will not be requeued again to the developers. This 10 hour ReSync is an ACK runtime behavior and hence i only made change in ACK runtime. There will no change needed in existing ACK controllers.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Copy link
Contributor

@RedbackThomson RedbackThomson left a comment

Choose a reason for hiding this comment

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

Just two nits. PTAL

pkg/runtime/reconciler.go Outdated Show resolved Hide resolved
pkg/runtime/reconciler.go Outdated Show resolved Hide resolved
pkg/runtime/reconciler.go Outdated Show resolved Hide resolved
@RedbackThomson
Copy link
Contributor

/lgtm

@ack-bot ack-bot added the lgtm Indicates that a PR is ready to be merged. label Jun 22, 2022
@ack-bot
Copy link
Collaborator

ack-bot commented Jun 22, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: RedbackThomson, vijtrip2

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [RedbackThomson,vijtrip2]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ack-bot ack-bot merged commit b9dd8fb into aws-controllers-k8s:main Jun 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved lgtm Indicates that a PR is ready to be merged.
Projects
None yet
3 participants