Skip to content

Commit

Permalink
Normalize reconciler name (#231)
Browse files Browse the repository at this point in the history 
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. Sequences don't have a name,
the index value is used instead.

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 <andrewssc@vmware.com>
  • Loading branch information
@scothis
scothis committed May 9, 2022
1 parent da4c0ef commit 9b6fe64
Showing 1 changed file with 73 additions and 25 deletions.
98 changes: 73 additions & 25 deletions reconcilers/reconcilers.go
View file
Expand Up @@ -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
Expand All @@ -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()))
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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{}
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -1087,7 +1091,11 @@ func (r *ChildReconciler) ourChild(parent, obj client.Object) bool {
type Sequence []SubReconciler

func (r Sequence) SetupWithManager(ctx context.Context, mgr ctrl.Manager, bldr *builder.Builder) error {
for _, reconciler := range r {
for i, reconciler := range r {
log := logr.FromContextOrDiscard(ctx).
WithName(fmt.Sprintf("%d", i))
ctx = logr.NewContext(ctx, log)

err := reconciler.SetupWithManager(ctx, mgr, bldr)
if err != nil {
return err
Expand All @@ -1098,7 +1106,11 @@ func (r Sequence) SetupWithManager(ctx context.Context, mgr ctrl.Manager, bldr *

func (r Sequence) Reconcile(ctx context.Context, parent client.Object) (ctrl.Result, error) {
aggregateResult := ctrl.Result{}
for _, reconciler := range r {
for i, reconciler := range r {
log := logr.FromContextOrDiscard(ctx).
WithName(fmt.Sprintf("%d", i))
ctx = logr.NewContext(ctx, log)

result, err := reconciler.Reconcile(ctx, parent)
if err != nil {
return ctrl.Result{}, err
Expand All @@ -1118,8 +1130,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
Expand All @@ -1134,15 +1146,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
}
Expand Down Expand Up @@ -1214,6 +1226,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.
Expand All @@ -1226,6 +1244,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
}
Expand All @@ -1252,6 +1278,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
Expand All @@ -1265,6 +1295,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.
Expand All @@ -1280,6 +1316,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
}
Expand All @@ -1301,6 +1345,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
Expand Down

0 comments on commit 9b6fe64

Please sign in to comment.