From a129b984b2484c7366bf4c685d156d457ff13cea Mon Sep 17 00:00:00 2001 From: Scott Andrews Date: Mon, 9 May 2022 16:11:17 -0400 Subject: [PATCH] Normalize reconciler name All (sub)reconcilers now have a name field. The name will be defaulted during as part of setup if not defined. While name should ideally be unique, the default names likely won't be. The name of each reconciler is added to the logger as its name. There were a couple places during setup where the reconciler name was defaulted after being added to the logger, these have been corrected. Signed-off-by: Scott Andrews --- reconcilers/reconcilers.go | 86 ++++++++++++++++++++++++++++---------- 1 file changed, 63 insertions(+), 23 deletions(-) diff --git a/reconcilers/reconcilers.go b/reconcilers/reconcilers.go index 22dd46c..bb5a4af 100644 --- a/reconcilers/reconcilers.go +++ b/reconcilers/reconcilers.go @@ -86,7 +86,8 @@ func NewConfig(mgr ctrl.Manager, apiType client.Object, syncPeriod time.Duration // resource's status is compared with the original status, updating the API // server if needed. type ParentReconciler struct { - // Name used to identify this reconciler. Defaults to `{Type}ParentReconciler`. Ideally unique, but not required to be so. + // Name used to identify this reconciler. Defaults to `{Type}ParentReconciler`. Ideally + // unique, but not required to be so. // // +optional Name string @@ -103,6 +104,10 @@ type ParentReconciler struct { } func (r *ParentReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error { + if r.Name == "" { + r.Name = fmt.Sprintf("%sParentReconciler", typeName(r.Type)) + } + log := logr.FromContextOrDiscard(ctx). WithName(r.Name). WithValues("parentType", gvk(r.Type, r.Config.Scheme())) @@ -113,10 +118,6 @@ func (r *ParentReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manage ctx = StashParentType(ctx, r.Type) ctx = StashCastParentType(ctx, r.Type) - if r.Name == "" { - r.Name = fmt.Sprintf("%sParentReconciler", typeName(r.Type)) - } - bldr := ctrl.NewControllerManagedBy(mgr).For(r.Type) if err := r.Reconciler.SetupWithManager(ctx, mgr, bldr); err != nil { return err @@ -334,12 +335,15 @@ var ( _ SubReconciler = (*ChildReconciler)(nil) _ SubReconciler = (Sequence)(nil) _ SubReconciler = (*CastParent)(nil) + _ SubReconciler = (*WithConfig)(nil) + _ SubReconciler = (*WithFinalizer)(nil) ) // SyncReconciler is a sub reconciler for custom reconciliation logic. No // behavior is defined directly. type SyncReconciler struct { - // Name used to identify this reconciler. Ideally unique, but not required to be so. + // Name used to identify this reconciler. Defaults to `SyncReconciler`. Ideally unique, but + // not required to be so. // // +optional Name string @@ -376,10 +380,12 @@ type SyncReconciler struct { } func (r *SyncReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, bldr *builder.Builder) error { - log := logr.FromContextOrDiscard(ctx) - if r.Name != "" { - log = log.WithName(r.Name) + if r.Name == "" { + r.Name = "SyncReconciler" } + + log := logr.FromContextOrDiscard(ctx). + WithName(r.Name) ctx = logr.NewContext(ctx, log) if r.Setup == nil { @@ -453,10 +459,8 @@ func (r *SyncReconciler) validate(ctx context.Context) error { } func (r *SyncReconciler) Reconcile(ctx context.Context, parent client.Object) (ctrl.Result, error) { - log := logr.FromContextOrDiscard(ctx) - if r.Name != "" { - log = log.WithName(r.Name) - } + log := logr.FromContextOrDiscard(ctx). + WithName(r.Name) ctx = logr.NewContext(ctx, log) result := ctrl.Result{} @@ -660,15 +664,15 @@ type ChildReconciler struct { func (r *ChildReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, bldr *builder.Builder) error { c := RetrieveConfigOrDie(ctx) + if r.Name == "" { + r.Name = fmt.Sprintf("%sChildReconciler", typeName(r.ChildType)) + } + log := logr.FromContextOrDiscard(ctx). WithName(r.Name). WithValues("childType", gvk(r.ChildType, c.Scheme())) ctx = logr.NewContext(ctx, log) - if r.Name == "" { - r.Name = fmt.Sprintf("%sChildReconciler", typeName(r.ChildType)) - } - if err := r.validate(ctx); err != nil { return err } @@ -1118,8 +1122,8 @@ func (r Sequence) Reconcile(ctx context.Context, parent client.Object) (ctrl.Res // cast parent are read-only. Attempts to mutate the parent will result in the // reconciler erring. type CastParent struct { - // Name used to identify this reconciler. Defaults to `{Type}CastParent`. Ideally unique, - // but not required to be so. + // Name used to identify this reconciler. Defaults to `{Type}CastParent`. Ideally unique, but + // not required to be so. // // +optional Name string @@ -1134,15 +1138,15 @@ type CastParent struct { } func (r *CastParent) SetupWithManager(ctx context.Context, mgr ctrl.Manager, bldr *builder.Builder) error { + if r.Name == "" { + r.Name = fmt.Sprintf("%sCastParent", typeName(r.Type)) + } + log := logr.FromContextOrDiscard(ctx). WithName(r.Name). WithValues("castParentType", typeName(r.Type)) ctx = logr.NewContext(ctx, log) - if r.Name == "" { - r.Name = fmt.Sprintf("%sCastParent", typeName(r.Type)) - } - if err := r.validate(ctx); err != nil { return err } @@ -1214,6 +1218,12 @@ func (r *CastParent) cast(ctx context.Context, parent client.Object) (context.Co // The specified config can be accessed with `RetrieveConfig(ctx)`, the original config used to // load the parent resource can be accessed with `RetrieveParentConfig(ctx)`. type WithConfig struct { + // Name used to identify this reconciler. Defaults to `WithConfig`. Ideally unique, but + // not required to be so. + // + // +optional + Name string + // Config to use for this portion of the reconciler hierarchy. This method is called during // setup and during reconciliation, if context is needed, it should be available durring both // phases. @@ -1226,6 +1236,14 @@ type WithConfig struct { } func (r *WithConfig) SetupWithManager(ctx context.Context, mgr ctrl.Manager, bldr *builder.Builder) error { + if r.Name == "" { + r.Name = "WithConfig" + } + + log := logr.FromContextOrDiscard(ctx). + WithName(r.Name) + ctx = logr.NewContext(ctx, log) + if err := r.validate(ctx); err != nil { return err } @@ -1252,6 +1270,10 @@ func (r *WithConfig) validate(ctx context.Context) error { } func (r *WithConfig) Reconcile(ctx context.Context, parent client.Object) (ctrl.Result, error) { + log := logr.FromContextOrDiscard(ctx). + WithName(r.Name) + ctx = logr.NewContext(ctx, log) + c, err := r.Config(ctx, RetrieveConfig(ctx)) if err != nil { return ctrl.Result{}, err @@ -1265,6 +1287,12 @@ func (r *WithConfig) Reconcile(ctx context.Context, parent client.Object) (ctrl. // already set, before calling the nested reconciler. When the resource is terminating, the // finalizer is cleared after returning from the nested reconciler without error. type WithFinalizer struct { + // Name used to identify this reconciler. Defaults to `WithFinalizer`. Ideally unique, but + // not required to be so. + // + // +optional + Name string + // Finalizer to set on the parent resource. The value must be unique to this specific // reconciler instance and not shared. Reusing a value may result in orphaned state when // the parent resource is deleted. @@ -1280,6 +1308,14 @@ type WithFinalizer struct { } func (r *WithFinalizer) SetupWithManager(ctx context.Context, mgr ctrl.Manager, bldr *builder.Builder) error { + if r.Name == "" { + r.Name = "WithFinalizer" + } + + log := logr.FromContextOrDiscard(ctx). + WithName(r.Name) + ctx = logr.NewContext(ctx, log) + if err := r.validate(ctx); err != nil { return err } @@ -1301,6 +1337,10 @@ func (r *WithFinalizer) validate(ctx context.Context) error { } func (r *WithFinalizer) Reconcile(ctx context.Context, parent client.Object) (ctrl.Result, error) { + log := logr.FromContextOrDiscard(ctx). + WithName(r.Name) + ctx = logr.NewContext(ctx, log) + if parent.GetDeletionTimestamp() == nil { if err := AddParentFinalizer(ctx, parent, r.Finalizer); err != nil { return ctrl.Result{}, err