From a1ed1fc4b4e0735baf18c555ace954389cece14f Mon Sep 17 00:00:00 2001 From: Hidde Beydals Date: Tue, 3 Aug 2021 13:23:50 +0200 Subject: [PATCH] source: `GetRequeueAfter` in place of `GetInterval` The problem with `GetInterval()` was that the returned type was of `metav1.Duration`, while almost anywhere it was used, a type of `time.Duration` was requested. The result of this was that we had to call `GetInterval().Duration` all the time, which would become a bit cumbersome after awhile. To prevent this, we introduce a new `GetRequeueAfter() time.Duration` method, which both results the right type, and bears a name that is easier to remember where the value is used most; while setting the `Result.RequeueAfter` during reconcile operations. The introduced of this method deprecates `GetInterval()`, which should be removed in a future MINOR release. Signed-off-by: Hidde Beydals --- api/v1beta1/bucket_types.go | 8 ++++++++ api/v1beta1/gitrepository_types.go | 8 ++++++++ api/v1beta1/helmchart_types.go | 8 ++++++++ api/v1beta1/helmrepository_types.go | 8 ++++++++ api/v1beta1/source.go | 5 +++++ controllers/bucket_controller.go | 4 ++-- controllers/gitrepository_controller.go | 18 +++++++++--------- controllers/helmchart_controller.go | 4 ++-- controllers/helmrepository_controller.go | 4 ++-- 9 files changed, 52 insertions(+), 15 deletions(-) diff --git a/api/v1beta1/bucket_types.go b/api/v1beta1/bucket_types.go index dd91fdf5..9aebad62 100644 --- a/api/v1beta1/bucket_types.go +++ b/api/v1beta1/bucket_types.go @@ -17,6 +17,8 @@ limitations under the License. package v1beta1 import ( + "time" + "github.com/fluxcd/pkg/apis/meta" "github.com/fluxcd/pkg/runtime/conditions" apimeta "k8s.io/apimachinery/pkg/api/meta" @@ -162,7 +164,13 @@ func (in *Bucket) SetConditions(conditions []metav1.Condition) { in.Status.Conditions = conditions } +// GetRequeueAfter returns the duration after which the source must be reconciled again. +func (in Bucket) GetRequeueAfter() time.Duration { + return in.Spec.Interval.Duration +} + // GetInterval returns the interval at which the source is reconciled. +// Deprecated: use GetRequeueAfter instead. func (in Bucket) GetInterval() metav1.Duration { return in.Spec.Interval } diff --git a/api/v1beta1/gitrepository_types.go b/api/v1beta1/gitrepository_types.go index dabbe69e..9a232d63 100644 --- a/api/v1beta1/gitrepository_types.go +++ b/api/v1beta1/gitrepository_types.go @@ -17,6 +17,8 @@ limitations under the License. package v1beta1 import ( + "time" + "github.com/fluxcd/pkg/apis/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -211,7 +213,13 @@ func (in *GitRepository) SetConditions(conditions []metav1.Condition) { in.Status.Conditions = conditions } +// GetRequeueAfter returns the duration after which the source must be reconciled again. +func (in GitRepository) GetRequeueAfter() time.Duration { + return in.Spec.Interval.Duration +} + // GetInterval returns the interval at which the source is reconciled. +// Deprecated: use GetRequeueAfter instead. func (in GitRepository) GetInterval() metav1.Duration { return in.Spec.Interval } diff --git a/api/v1beta1/helmchart_types.go b/api/v1beta1/helmchart_types.go index d13e1948..e216a9f9 100644 --- a/api/v1beta1/helmchart_types.go +++ b/api/v1beta1/helmchart_types.go @@ -17,6 +17,8 @@ limitations under the License. package v1beta1 import ( + "time" + "github.com/fluxcd/pkg/apis/meta" "github.com/fluxcd/pkg/runtime/conditions" apimeta "k8s.io/apimachinery/pkg/api/meta" @@ -173,7 +175,13 @@ func (in *HelmChart) SetConditions(conditions []metav1.Condition) { in.Status.Conditions = conditions } +// GetRequeueAfter returns the duration after which the source must be reconciled again. +func (in HelmChart) GetRequeueAfter() time.Duration { + return in.Spec.Interval.Duration +} + // GetInterval returns the interval at which the source is reconciled. +// Deprecated: use GetRequeueAfter instead. func (in HelmChart) GetInterval() metav1.Duration { return in.Spec.Interval } diff --git a/api/v1beta1/helmrepository_types.go b/api/v1beta1/helmrepository_types.go index d4067dd9..593710a5 100644 --- a/api/v1beta1/helmrepository_types.go +++ b/api/v1beta1/helmrepository_types.go @@ -17,6 +17,8 @@ limitations under the License. package v1beta1 import ( + "time" + "github.com/fluxcd/pkg/apis/meta" "github.com/fluxcd/pkg/runtime/conditions" apimeta "k8s.io/apimachinery/pkg/api/meta" @@ -151,7 +153,13 @@ func (in *HelmRepository) SetConditions(conditions []metav1.Condition) { in.Status.Conditions = conditions } +// GetRequeueAfter returns the duration after which the source must be reconciled again. +func (in HelmRepository) GetRequeueAfter() time.Duration { + return in.Spec.Interval.Duration +} + // GetInterval returns the interval at which the source is reconciled. +// Deprecated: use GetRequeueAfter instead. func (in HelmRepository) GetInterval() metav1.Duration { return in.Spec.Interval } diff --git a/api/v1beta1/source.go b/api/v1beta1/source.go index 5c10d00f..f22780dd 100644 --- a/api/v1beta1/source.go +++ b/api/v1beta1/source.go @@ -17,6 +17,8 @@ limitations under the License. package v1beta1 import ( + "time" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) @@ -29,9 +31,12 @@ const ( // Source interface must be supported by all API types. // +k8s:deepcopy-gen=false type Source interface { + // GetRequeueAfter returns the duration after which the source must be reconciled again. + GetRequeueAfter() time.Duration // GetArtifact returns the latest artifact from the source if present in the // status sub-resource. GetArtifact() *Artifact // GetInterval returns the interval at which the source is updated. + // Deprecated: use GetRequeueAfter instead. GetInterval() metav1.Duration } diff --git a/controllers/bucket_controller.go b/controllers/bucket_controller.go index e1ca4641..82844eb5 100644 --- a/controllers/bucket_controller.go +++ b/controllers/bucket_controller.go @@ -169,10 +169,10 @@ func (r *BucketReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr log.Info(fmt.Sprintf("Reconciliation finished in %s, next run in %s", time.Now().Sub(start).String(), - bucket.GetInterval().Duration.String(), + bucket.GetRequeueAfter().String(), )) - return ctrl.Result{RequeueAfter: bucket.GetInterval().Duration}, nil + return ctrl.Result{RequeueAfter: bucket.GetRequeueAfter()}, nil } func (r *BucketReconciler) reconcile(ctx context.Context, bucket sourcev1.Bucket) (sourcev1.Bucket, error) { diff --git a/controllers/gitrepository_controller.go b/controllers/gitrepository_controller.go index 122c2b2f..92375742 100644 --- a/controllers/gitrepository_controller.go +++ b/controllers/gitrepository_controller.go @@ -217,7 +217,7 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.G // Reconcile the source from upstream var artifact sourcev1.Artifact if result, err := r.reconcileSource(ctx, obj, &artifact, tmpDir); err != nil || result.IsZero() { - return ctrl.Result{RequeueAfter: obj.Spec.Interval.Duration}, err + return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, err } // Reconcile includes from the storage @@ -231,7 +231,7 @@ func (r *GitRepositoryReconciler) reconcile(ctx context.Context, obj *sourcev1.G return result, err } - return ctrl.Result{RequeueAfter: obj.Spec.Interval.Duration}, nil + return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil } // reconcileStorage ensures the current state of the storage matches the desired and previously observed state. @@ -264,7 +264,7 @@ func (r *GitRepositoryReconciler) reconcileStorage(ctx context.Context, obj *sou r.Storage.SetArtifactURL(obj.GetArtifact()) obj.Status.URL = r.Storage.SetHostname(obj.Status.URL) - return ctrl.Result{RequeueAfter: obj.Spec.Interval.Duration}, nil + return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil } // reconcileSource ensures the upstream Git repository can be reached and checked out using the declared configuration, @@ -364,7 +364,7 @@ func (r *GitRepositoryReconciler) reconcileSource(ctx context.Context, if !obj.GetArtifact().HasRevision(revision) { conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "NewRevision", "New upstream revision '%s'", revision) } - return ctrl.Result{RequeueAfter: obj.Spec.Interval.Duration}, nil + return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil } // reconcileArtifact archives a new artifact to the storage, if the current observation on the object does not match the @@ -394,7 +394,7 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so // The artifact is up-to-date if obj.GetArtifact().HasRevision(artifact.Revision) && !includes.Diff(obj.Status.IncludedArtifacts) { ctrl.LoggerFrom(ctx).Info("Artifact is up-to-date") - return ctrl.Result{RequeueAfter: obj.GetInterval().Duration}, nil + return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil } // Ensure target path exists and is a directory @@ -453,7 +453,7 @@ func (r *GitRepositoryReconciler) reconcileArtifact(ctx context.Context, obj *so if url != "" { obj.Status.URL = url } - return ctrl.Result{RequeueAfter: obj.Spec.Interval.Duration}, nil + return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil } // reconcileInclude reconciles the declared includes from the object by copying their artifact (sub)contents to the @@ -508,7 +508,7 @@ func (r *GitRepositoryReconciler) reconcileInclude(ctx context.Context, obj *sou if artifacts.Diff(obj.Status.IncludedArtifacts) { conditions.MarkTrue(obj, sourcev1.ArtifactOutdatedCondition, "IncludeChange", "Included artifacts differ from last observed includes") } - return ctrl.Result{RequeueAfter: obj.Spec.Interval.Duration}, nil + return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil } // reconcileDelete handles the delete of an object. It first garbage collects all artifacts for the object from the @@ -532,7 +532,7 @@ func (r *GitRepositoryReconciler) verifyCommitSignature(ctx context.Context, obj // Check if there is a commit verification is configured and remove any old observations if there is none if obj.Spec.Verification == nil || obj.Spec.Verification.Mode == "" { conditions.Delete(obj, sourcev1.SourceVerifiedCondition) - return ctrl.Result{RequeueAfter: obj.Spec.Interval.Duration}, nil + return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil } // Get secret with GPG data @@ -557,7 +557,7 @@ func (r *GitRepositoryReconciler) verifyCommitSignature(ctx context.Context, obj conditions.MarkTrue(obj, sourcev1.SourceVerifiedCondition, meta.SucceededReason, "Verified signature of commit '%s'", commit.Hash()) r.Eventf(ctx, obj, events.EventSeverityInfo, "VerifiedCommit", "Verified signature of commit '%s'", commit.Hash()) - return ctrl.Result{RequeueAfter: obj.Spec.Interval.Duration}, nil + return ctrl.Result{RequeueAfter: obj.GetRequeueAfter()}, nil } // garbageCollect performs a garbage collection for the given v1beta1.GitRepository. It removes all but the current diff --git a/controllers/helmchart_controller.go b/controllers/helmchart_controller.go index 50583bca..38611bcc 100644 --- a/controllers/helmchart_controller.go +++ b/controllers/helmchart_controller.go @@ -252,9 +252,9 @@ func (r *HelmChartReconciler) Reconcile(ctx context.Context, req ctrl.Request) ( log.Info(fmt.Sprintf("Reconciliation finished in %s, next run in %s", time.Now().Sub(start).String(), - chart.GetInterval().Duration.String(), + chart.GetRequeueAfter().String(), )) - return ctrl.Result{RequeueAfter: chart.GetInterval().Duration}, nil + return ctrl.Result{RequeueAfter: chart.GetRequeueAfter()}, nil } type HelmChartReconcilerOptions struct { diff --git a/controllers/helmrepository_controller.go b/controllers/helmrepository_controller.go index b7f8cd51..de559c13 100644 --- a/controllers/helmrepository_controller.go +++ b/controllers/helmrepository_controller.go @@ -164,10 +164,10 @@ func (r *HelmRepositoryReconciler) Reconcile(ctx context.Context, req ctrl.Reque log.Info(fmt.Sprintf("Reconciliation finished in %s, next run in %s", time.Now().Sub(start).String(), - repository.GetInterval().Duration.String(), + repository.GetRequeueAfter().String(), )) - return ctrl.Result{RequeueAfter: repository.GetInterval().Duration}, nil + return ctrl.Result{RequeueAfter: repository.GetRequeueAfter()}, nil } func (r *HelmRepositoryReconciler) reconcile(ctx context.Context, repository sourcev1.HelmRepository) (sourcev1.HelmRepository, error) {