From 74b657862c09c8d2168f8ca7d6ddb2d32b93da94 Mon Sep 17 00:00:00 2001 From: Meghna Baijal Date: Fri, 18 Jun 2021 00:14:26 -0700 Subject: [PATCH 1/2] Initial Commit: Infinite Requeue --- pkg/errors/error.go | 5 +++++ pkg/requeue/requeue.go | 5 +++++ pkg/runtime/reconciler.go | 23 ++++++++++++++--------- pkg/types/aws_resource_manager.go | 3 +++ 4 files changed, 27 insertions(+), 9 deletions(-) diff --git a/pkg/errors/error.go b/pkg/errors/error.go index ca58447..aa0da85 100644 --- a/pkg/errors/error.go +++ b/pkg/errors/error.go @@ -49,6 +49,11 @@ var ( // after some wait time TemporaryOutOfSync = fmt.Errorf( "temporary out of sync, reconcile after some time") + // TemporaryOutOfSync is to indicate the error isn't really an error + // but more of a marker that the status check will be performed + // after some wait time + RequeueOnSuccess = fmt.Errorf( + "resource creation successful but reconcile after some time") // Terminal is returned with resource is in Terminal Condition Terminal = fmt.Errorf( "resource is in terminal condition") diff --git a/pkg/requeue/requeue.go b/pkg/requeue/requeue.go index eb4b983..57d302d 100644 --- a/pkg/requeue/requeue.go +++ b/pkg/requeue/requeue.go @@ -21,6 +21,11 @@ const ( DefaultRequeueAfterDuration time.Duration = 30 * time.Second ) +// GetDurationInSeconds converts an integer into a time duration in seconds +func GetDurationInSeconds(duration int) time.Duration { + return time.Duration(duration) * time.Second +} + // Needed returns a new RequeueNeeded to instruct the ACK runtime to requeue // the processing item without been logged as error. func Needed(err error) *RequeueNeeded { diff --git a/pkg/runtime/reconciler.go b/pkg/runtime/reconciler.go index 535f897..2af7fa6 100644 --- a/pkg/runtime/reconciler.go +++ b/pkg/runtime/reconciler.go @@ -279,15 +279,20 @@ func (r *resourceReconciler) Sync( return err } for _, condition := range latest.Conditions() { - if condition.Type == ackv1alpha1.ConditionTypeResourceSynced && - condition.Status != corev1.ConditionTrue { - rlog.Debug( - "requeueing resource after finding resource synced condition false", - ) - return requeue.NeededAfter( - ackerr.TemporaryOutOfSync, - requeue.DefaultRequeueAfterDuration, - ) + if condition.Type == ackv1alpha1.ConditionTypeResourceSynced { + if condition.Status == corev1.ConditionTrue { + duration, status := r.rmf.GetRequeueOnSuccessSeconds() + if status { + return requeue.NeededAfter( + ackerr.RequeueOnSuccess, requeue.GetDurationInSeconds(duration)) + } + } else { + rlog.Debug( + "requeueing resource after finding resource synced condition false", + ) + return requeue.NeededAfter( + ackerr.TemporaryOutOfSync, requeue.DefaultRequeueAfterDuration) + } } } return nil diff --git a/pkg/types/aws_resource_manager.go b/pkg/types/aws_resource_manager.go index d284741..42d9d99 100644 --- a/pkg/types/aws_resource_manager.go +++ b/pkg/types/aws_resource_manager.go @@ -89,4 +89,7 @@ type AWSResourceManagerFactory interface { ) (AWSResourceManager, error) // IsAdoptable returns true if the resource is able to be adopted IsAdoptable() bool + // GetRequeueOnSuccessSeconds returns true if the resource should be requeued after specified seconds + // Default is false which means resource will not be requeued after success. + GetRequeueOnSuccessSeconds() (int, bool) } From 2770256ae0a9f9a3c39969d9c1a49585fd6e096f Mon Sep 17 00:00:00 2001 From: Meghna Baijal Date: Fri, 18 Jun 2021 14:49:26 -0700 Subject: [PATCH 2/2] Address Review Comments --- pkg/errors/error.go | 5 ----- pkg/requeue/requeue.go | 5 ----- pkg/runtime/reconciler.go | 12 +++++++----- pkg/types/aws_resource_manager.go | 5 +++-- 4 files changed, 10 insertions(+), 17 deletions(-) diff --git a/pkg/errors/error.go b/pkg/errors/error.go index aa0da85..ca58447 100644 --- a/pkg/errors/error.go +++ b/pkg/errors/error.go @@ -49,11 +49,6 @@ var ( // after some wait time TemporaryOutOfSync = fmt.Errorf( "temporary out of sync, reconcile after some time") - // TemporaryOutOfSync is to indicate the error isn't really an error - // but more of a marker that the status check will be performed - // after some wait time - RequeueOnSuccess = fmt.Errorf( - "resource creation successful but reconcile after some time") // Terminal is returned with resource is in Terminal Condition Terminal = fmt.Errorf( "resource is in terminal condition") diff --git a/pkg/requeue/requeue.go b/pkg/requeue/requeue.go index 57d302d..eb4b983 100644 --- a/pkg/requeue/requeue.go +++ b/pkg/requeue/requeue.go @@ -21,11 +21,6 @@ const ( DefaultRequeueAfterDuration time.Duration = 30 * time.Second ) -// GetDurationInSeconds converts an integer into a time duration in seconds -func GetDurationInSeconds(duration int) time.Duration { - return time.Duration(duration) * time.Second -} - // Needed returns a new RequeueNeeded to instruct the ACK runtime to requeue // the processing item without been logged as error. func Needed(err error) *RequeueNeeded { diff --git a/pkg/runtime/reconciler.go b/pkg/runtime/reconciler.go index 2af7fa6..4a1d886 100644 --- a/pkg/runtime/reconciler.go +++ b/pkg/runtime/reconciler.go @@ -15,7 +15,8 @@ package runtime import ( "context" - + "time" + "github.com/go-logr/logr" "github.com/pkg/errors" corev1 "k8s.io/api/core/v1" @@ -281,10 +282,11 @@ func (r *resourceReconciler) Sync( for _, condition := range latest.Conditions() { if condition.Type == ackv1alpha1.ConditionTypeResourceSynced { if condition.Status == corev1.ConditionTrue { - duration, status := r.rmf.GetRequeueOnSuccessSeconds() - if status { - return requeue.NeededAfter( - ackerr.RequeueOnSuccess, requeue.GetDurationInSeconds(duration)) + if duration := r.rmf.RequeueOnSuccessSeconds(); duration > 0 { + rlog.Debug( + "requeueing resource after resource synced condition true", + ) + return requeue.NeededAfter(nil, time.Duration(duration)*time.Second) } } else { rlog.Debug( diff --git a/pkg/types/aws_resource_manager.go b/pkg/types/aws_resource_manager.go index 42d9d99..620d810 100644 --- a/pkg/types/aws_resource_manager.go +++ b/pkg/types/aws_resource_manager.go @@ -70,6 +70,7 @@ type AWSResourceManager interface { // AWSResourceManagerFactory returns an AWSResourceManager that can be used to // manage AWS resources for a particular AWS account +// TODO(jaypipes): Move AWSResourceManagerFactory into its own file type AWSResourceManagerFactory interface { // ResourceDescriptor returns an AWSResourceDescriptor that can be used by // the upstream controller-runtime to introspect the CRs that the resource @@ -89,7 +90,7 @@ type AWSResourceManagerFactory interface { ) (AWSResourceManager, error) // IsAdoptable returns true if the resource is able to be adopted IsAdoptable() bool - // GetRequeueOnSuccessSeconds returns true if the resource should be requeued after specified seconds + // RequeueOnSuccessSeconds returns true if the resource should be requeued after specified seconds // Default is false which means resource will not be requeued after success. - GetRequeueOnSuccessSeconds() (int, bool) + RequeueOnSuccessSeconds() int }