From 6e96f61c0b9ba3a2ff40e6f6bcce356b8130b299 Mon Sep 17 00:00:00 2001 From: Matt Moore Date: Mon, 27 Apr 2020 21:01:55 -0700 Subject: [PATCH] Implement the fourth wave of per-reconciler leaderelection. With this, all downstream `// +genreconciler` reconcilers will become leader-aware and expose a model for the typed reconcilers they wrap to themselves become leader-aware. Detailed design: https://docs.google.com/document/d/1i_QHjQO2T3SNv49xjZLWlivcc0UvZN1Tbw2NKxThkyM/edit# Issue: https://github.com/knative/pkg/issues/1181 --- .../v1/customresourcedefinition/controller.go | 23 ++- .../v1/customresourcedefinition/reconciler.go | 119 +++++++++++--- .../stub/reconciler.go | 21 +++ .../customresourcedefinition/controller.go | 23 ++- .../customresourcedefinition/reconciler.go | 116 ++++++++++--- .../stub/reconciler.go | 21 +++ .../core/v1/namespace/controller.go | 23 ++- .../core/v1/namespace/reconciler.go | 116 ++++++++++--- .../core/v1/namespace/stub/reconciler.go | 21 +++ .../generators/reconciler_controller.go | 42 ++++- .../generators/reconciler_reconciler.go | 155 ++++++++++++++---- .../generators/reconciler_reconciler_stub.go | 28 ++++ hack/update-codegen.sh | 6 + 13 files changed, 616 insertions(+), 98 deletions(-) diff --git a/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/controller.go b/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/controller.go index 1130d910d9..315985b6b8 100644 --- a/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/controller.go +++ b/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/controller.go @@ -26,6 +26,8 @@ import ( corev1 "k8s.io/api/core/v1" clientsetscheme "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/scheme" + labels "k8s.io/apimachinery/pkg/labels" + types "k8s.io/apimachinery/pkg/types" watch "k8s.io/apimachinery/pkg/watch" scheme "k8s.io/client-go/kubernetes/scheme" v1 "k8s.io/client-go/kubernetes/typed/core/v1" @@ -35,6 +37,7 @@ import ( kubeclient "knative.dev/pkg/client/injection/kube/client" controller "knative.dev/pkg/controller" logging "knative.dev/pkg/logging" + reconciler "knative.dev/pkg/reconciler" ) const ( @@ -56,9 +59,27 @@ func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsF customresourcedefinitionInformer := customresourcedefinition.Get(ctx) + lister := customresourcedefinitionInformer.Lister() + rec := &reconcilerImpl{ + LeaderAwareFuncs: reconciler.LeaderAwareFuncs{ + PromoteFunc: func(bkt reconciler.Bucket, enq func(reconciler.Bucket, types.NamespacedName)) error { + all, err := lister.List(labels.Everything()) + if err != nil { + return err + } + for _, elt := range all { + // TODO: Consider letting users specify a filter in options. + enq(bkt, types.NamespacedName{ + Namespace: elt.GetNamespace(), + Name: elt.GetName(), + }) + } + return nil + }, + }, Client: client.Get(ctx), - Lister: customresourcedefinitionInformer.Lister(), + Lister: lister, reconciler: r, finalizerName: defaultFinalizerName, } diff --git a/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/reconciler.go b/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/reconciler.go index 4f858c9c2b..e0cd0fc1e7 100644 --- a/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/reconciler.go +++ b/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/reconciler.go @@ -21,6 +21,7 @@ package customresourcedefinition import ( context "context" json "encoding/json" + fmt "fmt" reflect "reflect" zap "go.uber.org/zap" @@ -31,6 +32,7 @@ import ( equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + labels "k8s.io/apimachinery/pkg/labels" types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" @@ -63,8 +65,30 @@ type Finalizer interface { FinalizeKind(ctx context.Context, o *v1.CustomResourceDefinition) reconciler.Event } +// ReadOnlyInterface defines the strongly typed interfaces to be implemented by a +// controller reconciling v1.CustomResourceDefinition if they want to process resources for which +// they are not the leader. +type ReadOnlyInterface interface { + // ObserveKind implements logic to observe v1.CustomResourceDefinition. + // This method should not write to the API. + ObserveKind(ctx context.Context, o *v1.CustomResourceDefinition) reconciler.Event +} + +// ReadOnlyFinalizer defines the strongly typed interfaces to be implemented by a +// controller finalizing v1.CustomResourceDefinition if they want to process tombstoned resources +// even when they are not the leader. Due to the nature of how finalizers are handled +// there are no guarantees that this will be called. +type ReadOnlyFinalizer interface { + // ObserveFinalizeKind implements custom logic to observe the final state of v1.CustomResourceDefinition. + // This method should not write to the API. + ObserveFinalizeKind(ctx context.Context, o *v1.CustomResourceDefinition) reconciler.Event +} + // reconcilerImpl implements controller.Reconciler for v1.CustomResourceDefinition resources. type reconcilerImpl struct { + // LeaderAwareFuncs is inlined to help us implement reconciler.LeaderAware + reconciler.LeaderAwareFuncs + // Client is used to write back status updates. Client clientset.Interface @@ -89,13 +113,39 @@ type reconcilerImpl struct { // Check that our Reconciler implements controller.Reconciler var _ controller.Reconciler = (*reconcilerImpl)(nil) +// Check that our generated Reconciler is always LeaderAware. +var _ reconciler.LeaderAware = (*reconcilerImpl)(nil) + func NewReconciler(ctx context.Context, logger *zap.SugaredLogger, client clientset.Interface, lister apiextensionsv1.CustomResourceDefinitionLister, recorder record.EventRecorder, r Interface, options ...controller.Options) controller.Reconciler { // Check the options function input. It should be 0 or 1. if len(options) > 1 { logger.Fatalf("up to one options struct is supported, found %d", len(options)) } + // Fail fast when users inadvertently implement the other LeaderAware interface. + // For the typed reconcilers, Promote shouldn't take any arguments. + if _, ok := r.(reconciler.LeaderAware); ok { + logger.Fatalf("%T implements the incorrect LeaderAware interface. Promote() should not take an argument as genreconciler handles the enqueuing automatically.", r) + } + // TODO: Consider validating when folks implement ReadOnlyFinalizer, but not Finalizer. + rec := &reconcilerImpl{ + LeaderAwareFuncs: reconciler.LeaderAwareFuncs{ + PromoteFunc: func(bkt reconciler.Bucket, enq func(reconciler.Bucket, types.NamespacedName)) error { + all, err := lister.List(labels.Everything()) + if err != nil { + return err + } + for _, elt := range all { + // TODO: Consider letting users specify a filter in options. + enq(bkt, types.NamespacedName{ + Namespace: elt.GetNamespace(), + Name: elt.GetName(), + }) + } + return nil + }, + }, Client: client, Lister: lister, Recorder: recorder, @@ -119,6 +169,25 @@ func NewReconciler(ctx context.Context, logger *zap.SugaredLogger, client client func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { logger := logging.FromContext(ctx) + // Convert the namespace/name string into a distinct namespace and name + namespace, name, err := cache.SplitMetaNamespaceKey(key) + if err != nil { + logger.Errorf("invalid resource key: %s", key) + return nil + } + // Establish whether we are the leader for use below. + isLeader := r.IsLeaderFor(types.NamespacedName{ + Namespace: namespace, + Name: name, + }) + roi, isROI := r.reconciler.(ReadOnlyInterface) + rof, isROF := r.reconciler.(ReadOnlyFinalizer) + if !isLeader && !isROI && !isROF { + // If we are not the leader, and we don't implement either ReadOnly + // interface, then take a fast-path out. + return nil + } + // If configStore is set, attach the frozen configuration to the context. if r.configStore != nil { ctx = r.configStore.ToContext(ctx) @@ -127,15 +196,6 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { // Add the recorder to context. ctx = controller.WithEventRecorder(ctx, r.Recorder) - // Convert the namespace/name string into a distinct namespace and name - - _, name, err := cache.SplitMetaNamespaceKey(key) - - if err != nil { - logger.Errorf("invalid resource key: %s", key) - return nil - } - // Get the resource with this namespace/name. getter := r.Lister @@ -155,20 +215,28 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { var reconcileEvent reconciler.Event if resource.GetDeletionTimestamp().IsZero() { - // Append the target method to the logger. - logger = logger.With(zap.String("targetMethod", "ReconcileKind")) + if isLeader { + // Append the target method to the logger. + logger = logger.With(zap.String("targetMethod", "ReconcileKind")) + + // Set and update the finalizer on resource if r.reconciler + // implements Finalizer. + if resource, err = r.setFinalizerIfFinalizer(ctx, resource); err != nil { + return fmt.Errorf("failed to set finalizers: %w", err) + } - // Set and update the finalizer on resource if r.reconciler - // implements Finalizer. - if resource, err = r.setFinalizerIfFinalizer(ctx, resource); err != nil { - logger.Warnw("Failed to set finalizers", zap.Error(err)) - } + // Reconcile this copy of the resource and then write back any status + // updates regardless of whether the reconciliation errored out. + reconcileEvent = r.reconciler.ReconcileKind(ctx, resource) - // Reconcile this copy of the resource and then write back any status - // updates regardless of whether the reconciliation errored out. - reconcileEvent = r.reconciler.ReconcileKind(ctx, resource) + } else if isROI { + // Append the target method to the logger. + logger = logger.With(zap.String("targetMethod", "ObserveKind")) - } else if fin, ok := r.reconciler.(Finalizer); ok { + // Observe any changes to this resource, since we are not the leader. + reconcileEvent = roi.ObserveKind(ctx, resource) + } + } else if fin, ok := r.reconciler.(Finalizer); isLeader && ok { // Append the target method to the logger. logger = logger.With(zap.String("targetMethod", "FinalizeKind")) @@ -176,8 +244,14 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { // and reconciled cleanly (nil or normal event), remove the finalizer. reconcileEvent = fin.FinalizeKind(ctx, resource) if resource, err = r.clearFinalizer(ctx, resource, reconcileEvent); err != nil { - logger.Warnw("Failed to clear finalizers", zap.Error(err)) + return fmt.Errorf("failed to clear finalizers: %w", err) } + } else if !isLeader && isROF { + // Append the target method to the logger. + logger = logger.With(zap.String("targetMethod", "ObserveFinalizeKind")) + + // For finalizing reconcilers, just observe when we aren't the leader. + reconcileEvent = rof.ObserveFinalizeKind(ctx, resource) } // Synchronize the status. @@ -186,6 +260,9 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { // This is important because the copy we loaded from the injectionInformer's // cache may be stale and we don't want to overwrite a prior update // to status with this stale state. + } else if !isLeader { + logger.Warn("Saw status changes when we aren't the leader!") + // TODO: Consider logging the diff at Debug? } else if err = r.updateStatus(original, resource); err != nil { logger.Warnw("Failed to update resource status", zap.Error(err)) r.Recorder.Eventf(resource, corev1.EventTypeWarning, "UpdateFailed", diff --git a/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/stub/reconciler.go b/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/stub/reconciler.go index 2f95a95a28..16ae428b8a 100644 --- a/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/stub/reconciler.go +++ b/client/injection/apiextensions/reconciler/apiextensions/v1/customresourcedefinition/stub/reconciler.go @@ -46,6 +46,15 @@ var _ customresourcedefinition.Interface = (*Reconciler)(nil) // Optionally check that our Reconciler implements Finalizer //var _ customresourcedefinition.Finalizer = (*Reconciler)(nil) +// Optionally check that our Reconciler implements ReadOnlyInterface +// Implement this to observe resources even when we are not the leader. +//var _ customresourcedefinition.ReadOnlyInterface = (*Reconciler)(nil) + +// Optionally check that our Reconciler implements ReadOnlyFinalizer +// Implement this to observe tombstoned resources even when we are not +// the leader (best effort). +//var _ customresourcedefinition.ReadOnlyFinalizer = (*Reconciler)(nil) + // ReconcileKind implements Interface.ReconcileKind. func (r *Reconciler) ReconcileKind(ctx context.Context, o *apiextensionsv1.CustomResourceDefinition) reconciler.Event { // TODO: use this if the resource implements InitializeConditions. @@ -64,3 +73,15 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, o *apiextensionsv1.Custo // // TODO: add custom finalization logic here. // return nil //} + +// Optionally, use ObserveKind to observe the resource when we are not the leader. +// func (r *Reconciler) ObserveKind(ctx context.Context, o *apiextensionsv1.CustomResourceDefinition) reconciler.Event { +// // TODO: add custom observation logic here. +// return nil +// } + +// Optionally, use ObserveFinalizeKind to observe resources being finalized when we are no the leader. +//func (r *Reconciler) ObserveFinalizeKind(ctx context.Context, o *apiextensionsv1.CustomResourceDefinition) reconciler.Event { +// // TODO: add custom observation logic here. +// return nil +//} diff --git a/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/controller.go b/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/controller.go index 4f20eeb03e..548b8e1607 100644 --- a/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/controller.go +++ b/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/controller.go @@ -26,6 +26,8 @@ import ( corev1 "k8s.io/api/core/v1" clientsetscheme "k8s.io/apiextensions-apiserver/pkg/client/clientset/clientset/scheme" + labels "k8s.io/apimachinery/pkg/labels" + types "k8s.io/apimachinery/pkg/types" watch "k8s.io/apimachinery/pkg/watch" scheme "k8s.io/client-go/kubernetes/scheme" v1 "k8s.io/client-go/kubernetes/typed/core/v1" @@ -35,6 +37,7 @@ import ( kubeclient "knative.dev/pkg/client/injection/kube/client" controller "knative.dev/pkg/controller" logging "knative.dev/pkg/logging" + reconciler "knative.dev/pkg/reconciler" ) const ( @@ -56,9 +59,27 @@ func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsF customresourcedefinitionInformer := customresourcedefinition.Get(ctx) + lister := customresourcedefinitionInformer.Lister() + rec := &reconcilerImpl{ + LeaderAwareFuncs: reconciler.LeaderAwareFuncs{ + PromoteFunc: func(bkt reconciler.Bucket, enq func(reconciler.Bucket, types.NamespacedName)) error { + all, err := lister.List(labels.Everything()) + if err != nil { + return err + } + for _, elt := range all { + // TODO: Consider letting users specify a filter in options. + enq(bkt, types.NamespacedName{ + Namespace: elt.GetNamespace(), + Name: elt.GetName(), + }) + } + return nil + }, + }, Client: client.Get(ctx), - Lister: customresourcedefinitionInformer.Lister(), + Lister: lister, reconciler: r, finalizerName: defaultFinalizerName, } diff --git a/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/reconciler.go b/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/reconciler.go index 30d0fa9cba..46db0ef985 100644 --- a/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/reconciler.go +++ b/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/reconciler.go @@ -32,6 +32,7 @@ import ( equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + labels "k8s.io/apimachinery/pkg/labels" types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" cache "k8s.io/client-go/tools/cache" @@ -64,8 +65,30 @@ type Finalizer interface { FinalizeKind(ctx context.Context, o *v1beta1.CustomResourceDefinition) reconciler.Event } +// ReadOnlyInterface defines the strongly typed interfaces to be implemented by a +// controller reconciling v1beta1.CustomResourceDefinition if they want to process resources for which +// they are not the leader. +type ReadOnlyInterface interface { + // ObserveKind implements logic to observe v1beta1.CustomResourceDefinition. + // This method should not write to the API. + ObserveKind(ctx context.Context, o *v1beta1.CustomResourceDefinition) reconciler.Event +} + +// ReadOnlyFinalizer defines the strongly typed interfaces to be implemented by a +// controller finalizing v1beta1.CustomResourceDefinition if they want to process tombstoned resources +// even when they are not the leader. Due to the nature of how finalizers are handled +// there are no guarantees that this will be called. +type ReadOnlyFinalizer interface { + // ObserveFinalizeKind implements custom logic to observe the final state of v1beta1.CustomResourceDefinition. + // This method should not write to the API. + ObserveFinalizeKind(ctx context.Context, o *v1beta1.CustomResourceDefinition) reconciler.Event +} + // reconcilerImpl implements controller.Reconciler for v1beta1.CustomResourceDefinition resources. type reconcilerImpl struct { + // LeaderAwareFuncs is inlined to help us implement reconciler.LeaderAware + reconciler.LeaderAwareFuncs + // Client is used to write back status updates. Client clientset.Interface @@ -90,13 +113,39 @@ type reconcilerImpl struct { // Check that our Reconciler implements controller.Reconciler var _ controller.Reconciler = (*reconcilerImpl)(nil) +// Check that our generated Reconciler is always LeaderAware. +var _ reconciler.LeaderAware = (*reconcilerImpl)(nil) + func NewReconciler(ctx context.Context, logger *zap.SugaredLogger, client clientset.Interface, lister apiextensionsv1beta1.CustomResourceDefinitionLister, recorder record.EventRecorder, r Interface, options ...controller.Options) controller.Reconciler { // Check the options function input. It should be 0 or 1. if len(options) > 1 { logger.Fatalf("up to one options struct is supported, found %d", len(options)) } + // Fail fast when users inadvertently implement the other LeaderAware interface. + // For the typed reconcilers, Promote shouldn't take any arguments. + if _, ok := r.(reconciler.LeaderAware); ok { + logger.Fatalf("%T implements the incorrect LeaderAware interface. Promote() should not take an argument as genreconciler handles the enqueuing automatically.", r) + } + // TODO: Consider validating when folks implement ReadOnlyFinalizer, but not Finalizer. + rec := &reconcilerImpl{ + LeaderAwareFuncs: reconciler.LeaderAwareFuncs{ + PromoteFunc: func(bkt reconciler.Bucket, enq func(reconciler.Bucket, types.NamespacedName)) error { + all, err := lister.List(labels.Everything()) + if err != nil { + return err + } + for _, elt := range all { + // TODO: Consider letting users specify a filter in options. + enq(bkt, types.NamespacedName{ + Namespace: elt.GetNamespace(), + Name: elt.GetName(), + }) + } + return nil + }, + }, Client: client, Lister: lister, Recorder: recorder, @@ -120,6 +169,25 @@ func NewReconciler(ctx context.Context, logger *zap.SugaredLogger, client client func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { logger := logging.FromContext(ctx) + // Convert the namespace/name string into a distinct namespace and name + namespace, name, err := cache.SplitMetaNamespaceKey(key) + if err != nil { + logger.Errorf("invalid resource key: %s", key) + return nil + } + // Establish whether we are the leader for use below. + isLeader := r.IsLeaderFor(types.NamespacedName{ + Namespace: namespace, + Name: name, + }) + roi, isROI := r.reconciler.(ReadOnlyInterface) + rof, isROF := r.reconciler.(ReadOnlyFinalizer) + if !isLeader && !isROI && !isROF { + // If we are not the leader, and we don't implement either ReadOnly + // interface, then take a fast-path out. + return nil + } + // If configStore is set, attach the frozen configuration to the context. if r.configStore != nil { ctx = r.configStore.ToContext(ctx) @@ -128,15 +196,6 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { // Add the recorder to context. ctx = controller.WithEventRecorder(ctx, r.Recorder) - // Convert the namespace/name string into a distinct namespace and name - - _, name, err := cache.SplitMetaNamespaceKey(key) - - if err != nil { - logger.Errorf("invalid resource key: %s", key) - return nil - } - // Get the resource with this namespace/name. getter := r.Lister @@ -156,20 +215,28 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { var reconcileEvent reconciler.Event if resource.GetDeletionTimestamp().IsZero() { - // Append the target method to the logger. - logger = logger.With(zap.String("targetMethod", "ReconcileKind")) + if isLeader { + // Append the target method to the logger. + logger = logger.With(zap.String("targetMethod", "ReconcileKind")) + + // Set and update the finalizer on resource if r.reconciler + // implements Finalizer. + if resource, err = r.setFinalizerIfFinalizer(ctx, resource); err != nil { + return fmt.Errorf("failed to set finalizers: %w", err) + } - // Set and update the finalizer on resource if r.reconciler - // implements Finalizer. - if resource, err = r.setFinalizerIfFinalizer(ctx, resource); err != nil { - return fmt.Errorf("failed to set finalizers: %w", err) - } + // Reconcile this copy of the resource and then write back any status + // updates regardless of whether the reconciliation errored out. + reconcileEvent = r.reconciler.ReconcileKind(ctx, resource) - // Reconcile this copy of the resource and then write back any status - // updates regardless of whether the reconciliation errored out. - reconcileEvent = r.reconciler.ReconcileKind(ctx, resource) + } else if isROI { + // Append the target method to the logger. + logger = logger.With(zap.String("targetMethod", "ObserveKind")) - } else if fin, ok := r.reconciler.(Finalizer); ok { + // Observe any changes to this resource, since we are not the leader. + reconcileEvent = roi.ObserveKind(ctx, resource) + } + } else if fin, ok := r.reconciler.(Finalizer); isLeader && ok { // Append the target method to the logger. logger = logger.With(zap.String("targetMethod", "FinalizeKind")) @@ -179,6 +246,12 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { if resource, err = r.clearFinalizer(ctx, resource, reconcileEvent); err != nil { return fmt.Errorf("failed to clear finalizers: %w", err) } + } else if !isLeader && isROF { + // Append the target method to the logger. + logger = logger.With(zap.String("targetMethod", "ObserveFinalizeKind")) + + // For finalizing reconcilers, just observe when we aren't the leader. + reconcileEvent = rof.ObserveFinalizeKind(ctx, resource) } // Synchronize the status. @@ -187,6 +260,9 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { // This is important because the copy we loaded from the injectionInformer's // cache may be stale and we don't want to overwrite a prior update // to status with this stale state. + } else if !isLeader { + logger.Warn("Saw status changes when we aren't the leader!") + // TODO: Consider logging the diff at Debug? } else if err = r.updateStatus(original, resource); err != nil { logger.Warnw("Failed to update resource status", zap.Error(err)) r.Recorder.Eventf(resource, v1.EventTypeWarning, "UpdateFailed", diff --git a/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/stub/reconciler.go b/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/stub/reconciler.go index be5bd027c6..fdd7f5a6a5 100644 --- a/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/stub/reconciler.go +++ b/client/injection/apiextensions/reconciler/apiextensions/v1beta1/customresourcedefinition/stub/reconciler.go @@ -46,6 +46,15 @@ var _ customresourcedefinition.Interface = (*Reconciler)(nil) // Optionally check that our Reconciler implements Finalizer //var _ customresourcedefinition.Finalizer = (*Reconciler)(nil) +// Optionally check that our Reconciler implements ReadOnlyInterface +// Implement this to observe resources even when we are not the leader. +//var _ customresourcedefinition.ReadOnlyInterface = (*Reconciler)(nil) + +// Optionally check that our Reconciler implements ReadOnlyFinalizer +// Implement this to observe tombstoned resources even when we are not +// the leader (best effort). +//var _ customresourcedefinition.ReadOnlyFinalizer = (*Reconciler)(nil) + // ReconcileKind implements Interface.ReconcileKind. func (r *Reconciler) ReconcileKind(ctx context.Context, o *v1beta1.CustomResourceDefinition) reconciler.Event { // TODO: use this if the resource implements InitializeConditions. @@ -64,3 +73,15 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, o *v1beta1.CustomResourc // // TODO: add custom finalization logic here. // return nil //} + +// Optionally, use ObserveKind to observe the resource when we are not the leader. +// func (r *Reconciler) ObserveKind(ctx context.Context, o *v1beta1.CustomResourceDefinition) reconciler.Event { +// // TODO: add custom observation logic here. +// return nil +// } + +// Optionally, use ObserveFinalizeKind to observe resources being finalized when we are no the leader. +//func (r *Reconciler) ObserveFinalizeKind(ctx context.Context, o *v1beta1.CustomResourceDefinition) reconciler.Event { +// // TODO: add custom observation logic here. +// return nil +//} diff --git a/client/injection/kube/reconciler/core/v1/namespace/controller.go b/client/injection/kube/reconciler/core/v1/namespace/controller.go index 4cee36b435..a21da03bb9 100644 --- a/client/injection/kube/reconciler/core/v1/namespace/controller.go +++ b/client/injection/kube/reconciler/core/v1/namespace/controller.go @@ -25,6 +25,8 @@ import ( strings "strings" corev1 "k8s.io/api/core/v1" + labels "k8s.io/apimachinery/pkg/labels" + types "k8s.io/apimachinery/pkg/types" watch "k8s.io/apimachinery/pkg/watch" scheme "k8s.io/client-go/kubernetes/scheme" v1 "k8s.io/client-go/kubernetes/typed/core/v1" @@ -33,6 +35,7 @@ import ( namespace "knative.dev/pkg/client/injection/kube/informers/core/v1/namespace" controller "knative.dev/pkg/controller" logging "knative.dev/pkg/logging" + reconciler "knative.dev/pkg/reconciler" ) const ( @@ -54,9 +57,27 @@ func NewImpl(ctx context.Context, r Interface, optionsFns ...controller.OptionsF namespaceInformer := namespace.Get(ctx) + lister := namespaceInformer.Lister() + rec := &reconcilerImpl{ + LeaderAwareFuncs: reconciler.LeaderAwareFuncs{ + PromoteFunc: func(bkt reconciler.Bucket, enq func(reconciler.Bucket, types.NamespacedName)) error { + all, err := lister.List(labels.Everything()) + if err != nil { + return err + } + for _, elt := range all { + // TODO: Consider letting users specify a filter in options. + enq(bkt, types.NamespacedName{ + Namespace: elt.GetNamespace(), + Name: elt.GetName(), + }) + } + return nil + }, + }, Client: client.Get(ctx), - Lister: namespaceInformer.Lister(), + Lister: lister, reconciler: r, finalizerName: defaultFinalizerName, } diff --git a/client/injection/kube/reconciler/core/v1/namespace/reconciler.go b/client/injection/kube/reconciler/core/v1/namespace/reconciler.go index 00c761a1d7..e7e27c0175 100644 --- a/client/injection/kube/reconciler/core/v1/namespace/reconciler.go +++ b/client/injection/kube/reconciler/core/v1/namespace/reconciler.go @@ -29,6 +29,7 @@ import ( equality "k8s.io/apimachinery/pkg/api/equality" errors "k8s.io/apimachinery/pkg/api/errors" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + labels "k8s.io/apimachinery/pkg/labels" types "k8s.io/apimachinery/pkg/types" sets "k8s.io/apimachinery/pkg/util/sets" kubernetes "k8s.io/client-go/kubernetes" @@ -63,8 +64,30 @@ type Finalizer interface { FinalizeKind(ctx context.Context, o *v1.Namespace) reconciler.Event } +// ReadOnlyInterface defines the strongly typed interfaces to be implemented by a +// controller reconciling v1.Namespace if they want to process resources for which +// they are not the leader. +type ReadOnlyInterface interface { + // ObserveKind implements logic to observe v1.Namespace. + // This method should not write to the API. + ObserveKind(ctx context.Context, o *v1.Namespace) reconciler.Event +} + +// ReadOnlyFinalizer defines the strongly typed interfaces to be implemented by a +// controller finalizing v1.Namespace if they want to process tombstoned resources +// even when they are not the leader. Due to the nature of how finalizers are handled +// there are no guarantees that this will be called. +type ReadOnlyFinalizer interface { + // ObserveFinalizeKind implements custom logic to observe the final state of v1.Namespace. + // This method should not write to the API. + ObserveFinalizeKind(ctx context.Context, o *v1.Namespace) reconciler.Event +} + // reconcilerImpl implements controller.Reconciler for v1.Namespace resources. type reconcilerImpl struct { + // LeaderAwareFuncs is inlined to help us implement reconciler.LeaderAware + reconciler.LeaderAwareFuncs + // Client is used to write back status updates. Client kubernetes.Interface @@ -89,13 +112,39 @@ type reconcilerImpl struct { // Check that our Reconciler implements controller.Reconciler var _ controller.Reconciler = (*reconcilerImpl)(nil) +// Check that our generated Reconciler is always LeaderAware. +var _ reconciler.LeaderAware = (*reconcilerImpl)(nil) + func NewReconciler(ctx context.Context, logger *zap.SugaredLogger, client kubernetes.Interface, lister corev1.NamespaceLister, recorder record.EventRecorder, r Interface, options ...controller.Options) controller.Reconciler { // Check the options function input. It should be 0 or 1. if len(options) > 1 { logger.Fatalf("up to one options struct is supported, found %d", len(options)) } + // Fail fast when users inadvertently implement the other LeaderAware interface. + // For the typed reconcilers, Promote shouldn't take any arguments. + if _, ok := r.(reconciler.LeaderAware); ok { + logger.Fatalf("%T implements the incorrect LeaderAware interface. Promote() should not take an argument as genreconciler handles the enqueuing automatically.", r) + } + // TODO: Consider validating when folks implement ReadOnlyFinalizer, but not Finalizer. + rec := &reconcilerImpl{ + LeaderAwareFuncs: reconciler.LeaderAwareFuncs{ + PromoteFunc: func(bkt reconciler.Bucket, enq func(reconciler.Bucket, types.NamespacedName)) error { + all, err := lister.List(labels.Everything()) + if err != nil { + return err + } + for _, elt := range all { + // TODO: Consider letting users specify a filter in options. + enq(bkt, types.NamespacedName{ + Namespace: elt.GetNamespace(), + Name: elt.GetName(), + }) + } + return nil + }, + }, Client: client, Lister: lister, Recorder: recorder, @@ -119,6 +168,25 @@ func NewReconciler(ctx context.Context, logger *zap.SugaredLogger, client kubern func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { logger := logging.FromContext(ctx) + // Convert the namespace/name string into a distinct namespace and name + namespace, name, err := cache.SplitMetaNamespaceKey(key) + if err != nil { + logger.Errorf("invalid resource key: %s", key) + return nil + } + // Establish whether we are the leader for use below. + isLeader := r.IsLeaderFor(types.NamespacedName{ + Namespace: namespace, + Name: name, + }) + roi, isROI := r.reconciler.(ReadOnlyInterface) + rof, isROF := r.reconciler.(ReadOnlyFinalizer) + if !isLeader && !isROI && !isROF { + // If we are not the leader, and we don't implement either ReadOnly + // interface, then take a fast-path out. + return nil + } + // If configStore is set, attach the frozen configuration to the context. if r.configStore != nil { ctx = r.configStore.ToContext(ctx) @@ -127,15 +195,6 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { // Add the recorder to context. ctx = controller.WithEventRecorder(ctx, r.Recorder) - // Convert the namespace/name string into a distinct namespace and name - - _, name, err := cache.SplitMetaNamespaceKey(key) - - if err != nil { - logger.Errorf("invalid resource key: %s", key) - return nil - } - // Get the resource with this namespace/name. getter := r.Lister @@ -155,20 +214,28 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { var reconcileEvent reconciler.Event if resource.GetDeletionTimestamp().IsZero() { - // Append the target method to the logger. - logger = logger.With(zap.String("targetMethod", "ReconcileKind")) + if isLeader { + // Append the target method to the logger. + logger = logger.With(zap.String("targetMethod", "ReconcileKind")) + + // Set and update the finalizer on resource if r.reconciler + // implements Finalizer. + if resource, err = r.setFinalizerIfFinalizer(ctx, resource); err != nil { + return fmt.Errorf("failed to set finalizers: %w", err) + } - // Set and update the finalizer on resource if r.reconciler - // implements Finalizer. - if resource, err = r.setFinalizerIfFinalizer(ctx, resource); err != nil { - return fmt.Errorf("failed to set finalizers: %w", err) - } + // Reconcile this copy of the resource and then write back any status + // updates regardless of whether the reconciliation errored out. + reconcileEvent = r.reconciler.ReconcileKind(ctx, resource) - // Reconcile this copy of the resource and then write back any status - // updates regardless of whether the reconciliation errored out. - reconcileEvent = r.reconciler.ReconcileKind(ctx, resource) + } else if isROI { + // Append the target method to the logger. + logger = logger.With(zap.String("targetMethod", "ObserveKind")) - } else if fin, ok := r.reconciler.(Finalizer); ok { + // Observe any changes to this resource, since we are not the leader. + reconcileEvent = roi.ObserveKind(ctx, resource) + } + } else if fin, ok := r.reconciler.(Finalizer); isLeader && ok { // Append the target method to the logger. logger = logger.With(zap.String("targetMethod", "FinalizeKind")) @@ -178,6 +245,12 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { if resource, err = r.clearFinalizer(ctx, resource, reconcileEvent); err != nil { return fmt.Errorf("failed to clear finalizers: %w", err) } + } else if !isLeader && isROF { + // Append the target method to the logger. + logger = logger.With(zap.String("targetMethod", "ObserveFinalizeKind")) + + // For finalizing reconcilers, just observe when we aren't the leader. + reconcileEvent = rof.ObserveFinalizeKind(ctx, resource) } // Synchronize the status. @@ -186,6 +259,9 @@ func (r *reconcilerImpl) Reconcile(ctx context.Context, key string) error { // This is important because the copy we loaded from the injectionInformer's // cache may be stale and we don't want to overwrite a prior update // to status with this stale state. + } else if !isLeader { + logger.Warn("Saw status changes when we aren't the leader!") + // TODO: Consider logging the diff at Debug? } else if err = r.updateStatus(original, resource); err != nil { logger.Warnw("Failed to update resource status", zap.Error(err)) r.Recorder.Eventf(resource, v1.EventTypeWarning, "UpdateFailed", diff --git a/client/injection/kube/reconciler/core/v1/namespace/stub/reconciler.go b/client/injection/kube/reconciler/core/v1/namespace/stub/reconciler.go index 21f7882872..570fe11225 100644 --- a/client/injection/kube/reconciler/core/v1/namespace/stub/reconciler.go +++ b/client/injection/kube/reconciler/core/v1/namespace/stub/reconciler.go @@ -45,6 +45,15 @@ var _ namespace.Interface = (*Reconciler)(nil) // Optionally check that our Reconciler implements Finalizer //var _ namespace.Finalizer = (*Reconciler)(nil) +// Optionally check that our Reconciler implements ReadOnlyInterface +// Implement this to observe resources even when we are not the leader. +//var _ namespace.ReadOnlyInterface = (*Reconciler)(nil) + +// Optionally check that our Reconciler implements ReadOnlyFinalizer +// Implement this to observe tombstoned resources even when we are not +// the leader (best effort). +//var _ namespace.ReadOnlyFinalizer = (*Reconciler)(nil) + // ReconcileKind implements Interface.ReconcileKind. func (r *Reconciler) ReconcileKind(ctx context.Context, o *v1.Namespace) reconciler.Event { // TODO: use this if the resource implements InitializeConditions. @@ -63,3 +72,15 @@ func (r *Reconciler) ReconcileKind(ctx context.Context, o *v1.Namespace) reconci // // TODO: add custom finalization logic here. // return nil //} + +// Optionally, use ObserveKind to observe the resource when we are not the leader. +// func (r *Reconciler) ObserveKind(ctx context.Context, o *v1.Namespace) reconciler.Event { +// // TODO: add custom observation logic here. +// return nil +// } + +// Optionally, use ObserveFinalizeKind to observe resources being finalized when we are no the leader. +//func (r *Reconciler) ObserveFinalizeKind(ctx context.Context, o *v1.Namespace) reconciler.Event { +// // TODO: add custom observation logic here. +// return nil +//} diff --git a/codegen/cmd/injection-gen/generators/reconciler_controller.go b/codegen/cmd/injection-gen/generators/reconciler_controller.go index bf3ca04fe8..ac1c9fac32 100644 --- a/codegen/cmd/injection-gen/generators/reconciler_controller.go +++ b/codegen/cmd/injection-gen/generators/reconciler_controller.go @@ -142,9 +142,21 @@ func (g *reconcilerControllerGenerator) GenerateType(c *generator.Context, t *ty Package: "context", Name: "Context", }), - "fmtSprintf": c.Universe.Function(types.Name{ - Package: "fmt", - Name: "Sprintf", + "reconcilerLeaderAwareFuncs": c.Universe.Type(types.Name{ + Package: "knative.dev/pkg/reconciler", + Name: "LeaderAwareFuncs", + }), + "reconcilerBucket": c.Universe.Type(types.Name{ + Package: "knative.dev/pkg/reconciler", + Name: "Bucket", + }), + "typesNamespacedName": c.Universe.Type(types.Name{ + Package: "k8s.io/apimachinery/pkg/types", + Name: "NamespacedName", + }), + "labelsEverything": c.Universe.Function(types.Name{ + Package: "k8s.io/apimachinery/pkg/labels", + Name: "Everything", }), "stringsReplaceAll": c.Universe.Function(types.Name{ Package: "strings", @@ -154,6 +166,10 @@ func (g *reconcilerControllerGenerator) GenerateType(c *generator.Context, t *ty Package: "reflect", Name: "TypeOf", }), + "fmtSprintf": c.Universe.Function(types.Name{ + Package: "fmt", + Name: "Sprintf", + }), } sw.Do(reconcilerControllerNewImpl, m) @@ -185,9 +201,27 @@ func NewImpl(ctx {{.contextContext|raw}}, r Interface{{if .hasClass}}, classValu {{.type|lowercaseSingular}}Informer := {{.informerGet|raw}}(ctx) + lister := {{.type|lowercaseSingular}}Informer.Lister() + rec := &reconcilerImpl{ + LeaderAwareFuncs: {{.reconcilerLeaderAwareFuncs|raw}}{ + PromoteFunc: func(bkt {{.reconcilerBucket|raw}}, enq func({{.reconcilerBucket|raw}}, {{.typesNamespacedName|raw}})) error { + all, err := lister.List({{.labelsEverything|raw}}()) + if err != nil { + return err + } + for _, elt := range all { + // TODO: Consider letting users specify a filter in options. + enq(bkt, {{.typesNamespacedName|raw}}{ + Namespace: elt.GetNamespace(), + Name: elt.GetName(), + }) + } + return nil + }, + }, Client: {{.clientGet|raw}}(ctx), - Lister: {{.type|lowercaseSingular}}Informer.Lister(), + Lister: lister, reconciler: r, finalizerName: defaultFinalizerName, {{if .hasClass}}classValue: classValue,{{end}} diff --git a/codegen/cmd/injection-gen/generators/reconciler_reconciler.go b/codegen/cmd/injection-gen/generators/reconciler_reconciler.go index df4d595ab2..b3d3596a15 100644 --- a/codegen/cmd/injection-gen/generators/reconciler_reconciler.go +++ b/codegen/cmd/injection-gen/generators/reconciler_reconciler.go @@ -152,6 +152,30 @@ func (g *reconcilerReconcilerGenerator) GenerateType(c *generator.Context, t *ty "equalitySemantic": c.Universe.Package("k8s.io/apimachinery/pkg/api/equality").Variable("Semantic"), "jsonMarshal": c.Universe.Package("encoding/json").Function("Marshal"), "typesMergePatchType": c.Universe.Package("k8s.io/apimachinery/pkg/types").Constant("MergePatchType"), + "syncRWMutex": c.Universe.Type(types.Name{ + Package: "sync", + Name: "RWMutex", + }), + "reconcilerLeaderAware": c.Universe.Type(types.Name{ + Package: "knative.dev/pkg/reconciler", + Name: "LeaderAware", + }), + "reconcilerLeaderAwareFuncs": c.Universe.Type(types.Name{ + Package: "knative.dev/pkg/reconciler", + Name: "LeaderAwareFuncs", + }), + "reconcilerBucket": c.Universe.Type(types.Name{ + Package: "knative.dev/pkg/reconciler", + Name: "Bucket", + }), + "typesNamespacedName": c.Universe.Type(types.Name{ + Package: "k8s.io/apimachinery/pkg/types", + Name: "NamespacedName", + }), + "labelsEverything": c.Universe.Function(types.Name{ + Package: "k8s.io/apimachinery/pkg/labels", + Name: "Everything", + }), } sw.Do(reconcilerInterfaceFactory, m) @@ -187,8 +211,30 @@ type Finalizer interface { FinalizeKind(ctx {{.contextContext|raw}}, o *{{.type|raw}}) {{.reconcilerEvent|raw}} } +// ReadOnlyInterface defines the strongly typed interfaces to be implemented by a +// controller reconciling {{.type|raw}} if they want to process resources for which +// they are not the leader. +type ReadOnlyInterface interface { + // ObserveKind implements logic to observe {{.type|raw}}. + // This method should not write to the API. + ObserveKind(ctx {{.contextContext|raw}}, o *{{.type|raw}}) {{.reconcilerEvent|raw}} +} + +// ReadOnlyFinalizer defines the strongly typed interfaces to be implemented by a +// controller finalizing {{.type|raw}} if they want to process tombstoned resources +// even when they are not the leader. Due to the nature of how finalizers are handled +// there are no guarantees that this will be called. +type ReadOnlyFinalizer interface { + // ObserveFinalizeKind implements custom logic to observe the final state of {{.type|raw}}. + // This method should not write to the API. + ObserveFinalizeKind(ctx {{.contextContext|raw}}, o *{{.type|raw}}) {{.reconcilerEvent|raw}} +} + // reconcilerImpl implements controller.Reconciler for {{.type|raw}} resources. type reconcilerImpl struct { + // LeaderAwareFuncs is inlined to help us implement {{.reconcilerLeaderAware|raw}} + {{.reconcilerLeaderAwareFuncs|raw}} + // Client is used to write back status updates. Client {{.clientsetInterface|raw}} @@ -217,6 +263,8 @@ type reconcilerImpl struct { // Check that our Reconciler implements controller.Reconciler var _ controller.Reconciler = (*reconcilerImpl)(nil) +// Check that our generated Reconciler is always LeaderAware. +var _ {{.reconcilerLeaderAware|raw}} = (*reconcilerImpl)(nil) ` @@ -227,7 +275,30 @@ func NewReconciler(ctx {{.contextContext|raw}}, logger *{{.zapSugaredLogger|raw} logger.Fatalf("up to one options struct is supported, found %d", len(options)) } + // Fail fast when users inadvertently implement the other LeaderAware interface. + // For the typed reconcilers, Promote shouldn't take any arguments. + if _, ok := r.({{.reconcilerLeaderAware|raw}}); ok { + logger.Fatalf("%T implements the incorrect LeaderAware interface. Promote() should not take an argument as genreconciler handles the enqueuing automatically.", r) + } + // TODO: Consider validating when folks implement ReadOnlyFinalizer, but not Finalizer. + rec := &reconcilerImpl{ + LeaderAwareFuncs: {{.reconcilerLeaderAwareFuncs|raw}}{ + PromoteFunc: func(bkt {{.reconcilerBucket|raw}}, enq func({{.reconcilerBucket|raw}}, {{.typesNamespacedName|raw}})) error { + all, err := lister.List({{.labelsEverything|raw}}()) + if err != nil { + return err + } + for _, elt := range all { + // TODO: Consider letting users specify a filter in options. + enq(bkt, {{.typesNamespacedName|raw}}{ + Namespace: elt.GetNamespace(), + Name: elt.GetName(), + }) + } + return nil + }, + }, Client: client, Lister: lister, Recorder: recorder, @@ -254,6 +325,25 @@ var reconcilerImplFactory = ` func (r *reconcilerImpl) Reconcile(ctx {{.contextContext|raw}}, key string) error { logger := {{.loggingFromContext|raw}}(ctx) + // Convert the namespace/name string into a distinct namespace and name + namespace, name, err := {{.cacheSplitMetaNamespaceKey|raw}}(key) + if err != nil { + logger.Errorf("invalid resource key: %s", key) + return nil + } + // Establish whether we are the leader for use below. + isLeader := r.IsLeaderFor({{.typesNamespacedName|raw}}{ + Namespace: namespace, + Name: name, + }) + roi, isROI := r.reconciler.(ReadOnlyInterface) + rof, isROF := r.reconciler.(ReadOnlyFinalizer); + if !isLeader && !isROI && !isROF { + // If we are not the leader, and we don't implement either ReadOnly + // interface, then take a fast-path out. + return nil + } + // If configStore is set, attach the frozen configuration to the context. if r.configStore != nil { ctx = r.configStore.ToContext(ctx) @@ -262,19 +352,7 @@ func (r *reconcilerImpl) Reconcile(ctx {{.contextContext|raw}}, key string) erro // Add the recorder to context. ctx = {{.controllerWithEventRecorder|raw}}(ctx, r.Recorder) - // Convert the namespace/name string into a distinct namespace and name - {{if .nonNamespaced}} - _, name, err := {{.cacheSplitMetaNamespaceKey|raw}}(key) - {{else}} - namespace, name, err := {{.cacheSplitMetaNamespaceKey|raw}}(key) - {{end}} - if err != nil { - logger.Errorf("invalid resource key: %s", key) - return nil - } - // Get the resource with this namespace/name. - {{if .nonNamespaced}} getter := r.Lister {{else}} @@ -303,27 +381,34 @@ func (r *reconcilerImpl) Reconcile(ctx {{.contextContext|raw}}, key string) erro var reconcileEvent {{.reconcilerEvent|raw}} if resource.GetDeletionTimestamp().IsZero() { - // Append the target method to the logger. - logger = logger.With(zap.String("targetMethod", "ReconcileKind")) - - // Set and update the finalizer on resource if r.reconciler - // implements Finalizer. - if resource, err = r.setFinalizerIfFinalizer(ctx, resource); err != nil { - return {{.fmtErrorf|raw}}("failed to set finalizers: %w", err) - } + if isLeader { + // Append the target method to the logger. + logger = logger.With(zap.String("targetMethod", "ReconcileKind")) + + // Set and update the finalizer on resource if r.reconciler + // implements Finalizer. + if resource, err = r.setFinalizerIfFinalizer(ctx, resource); err != nil { + return {{.fmtErrorf|raw}}("failed to set finalizers: %w", err) + } + {{if .isKRShaped}} + reconciler.PreProcessReconcile(ctx, resource) + {{end}} - {{if .isKRShaped}} - reconciler.PreProcessReconcile(ctx, resource) - {{end}} + // Reconcile this copy of the resource and then write back any status + // updates regardless of whether the reconciliation errored out. + reconcileEvent = r.reconciler.ReconcileKind(ctx, resource) - // Reconcile this copy of the resource and then write back any status - // updates regardless of whether the reconciliation errored out. - reconcileEvent = r.reconciler.ReconcileKind(ctx, resource) + {{if .isKRShaped}} + reconciler.PostProcessReconcile(ctx, resource, original) + {{end}} + } else if isROI { + // Append the target method to the logger. + logger = logger.With(zap.String("targetMethod", "ObserveKind")) - {{if .isKRShaped}} - reconciler.PostProcessReconcile(ctx, resource, original) - {{end}} - } else if fin, ok := r.reconciler.(Finalizer); ok { + // Observe any changes to this resource, since we are not the leader. + reconcileEvent = roi.ObserveKind(ctx, resource) + } + } else if fin, ok := r.reconciler.(Finalizer); isLeader && ok { // Append the target method to the logger. logger = logger.With(zap.String("targetMethod", "FinalizeKind")) @@ -333,6 +418,12 @@ func (r *reconcilerImpl) Reconcile(ctx {{.contextContext|raw}}, key string) erro if resource, err = r.clearFinalizer(ctx, resource, reconcileEvent); err != nil { return {{.fmtErrorf|raw}}("failed to clear finalizers: %w", err) } + } else if !isLeader && isROF { + // Append the target method to the logger. + logger = logger.With(zap.String("targetMethod", "ObserveFinalizeKind")) + + // For finalizing reconcilers, just observe when we aren't the leader. + reconcileEvent = rof.ObserveFinalizeKind(ctx, resource) } // Synchronize the status. @@ -341,6 +432,9 @@ func (r *reconcilerImpl) Reconcile(ctx {{.contextContext|raw}}, key string) erro // This is important because the copy we loaded from the injectionInformer's // cache may be stale and we don't want to overwrite a prior update // to status with this stale state. + } else if !isLeader { + logger.Warn("Saw status changes when we aren't the leader!") + // TODO: Consider logging the diff at Debug? } else if err = r.updateStatus(original, resource); err != nil { logger.Warnw("Failed to update resource status", zap.Error(err)) r.Recorder.Eventf(resource, {{.corev1EventTypeWarning|raw}}, "UpdateFailed", @@ -520,4 +614,5 @@ func (r *reconcilerImpl) clearFinalizer(ctx {{.contextContext|raw}}, resource *{ // Synchronize the finalizers filtered by r.finalizerName. return r.updateFinalizersFiltered(ctx, resource) } + ` diff --git a/codegen/cmd/injection-gen/generators/reconciler_reconciler_stub.go b/codegen/cmd/injection-gen/generators/reconciler_reconciler_stub.go index 3571ae2d1c..62805e3d2a 100644 --- a/codegen/cmd/injection-gen/generators/reconciler_reconciler_stub.go +++ b/codegen/cmd/injection-gen/generators/reconciler_reconciler_stub.go @@ -77,6 +77,14 @@ func (g *reconcilerReconcilerStubGenerator) GenerateType(c *generator.Context, t Package: g.reconcilerPkg, Name: "Finalizer", }), + "reconcilerReadOnlyInterface": c.Universe.Type(types.Name{ + Package: g.reconcilerPkg, + Name: "ReadOnlyInterface", + }), + "reconcilerReadOnlyFinalizer": c.Universe.Type(types.Name{ + Package: g.reconcilerPkg, + Name: "ReadOnlyFinalizer", + }), "corev1EventTypeNormal": c.Universe.Type(types.Name{ Package: "k8s.io/api/core/v1", Name: "EventTypeNormal", @@ -112,6 +120,14 @@ var _ {{.reconcilerInterface|raw}} = (*Reconciler)(nil) // Optionally check that our Reconciler implements Finalizer //var _ {{.reconcilerFinalizer|raw}} = (*Reconciler)(nil) +// Optionally check that our Reconciler implements ReadOnlyInterface +// Implement this to observe resources even when we are not the leader. +//var _ {{.reconcilerReadOnlyInterface|raw}} = (*Reconciler)(nil) + +// Optionally check that our Reconciler implements ReadOnlyFinalizer +// Implement this to observe tombstoned resources even when we are not +// the leader (best effort). +//var _ {{.reconcilerReadOnlyFinalizer|raw}} = (*Reconciler)(nil) // ReconcileKind implements Interface.ReconcileKind. func (r *Reconciler) ReconcileKind(ctx {{.contextContext|raw}}, o *{{.type|raw}}) {{.reconcilerEvent|raw}} { @@ -133,4 +149,16 @@ func (r *Reconciler) ReconcileKind(ctx {{.contextContext|raw}}, o *{{.type|raw}} // // TODO: add custom finalization logic here. // return nil //} + +// Optionally, use ObserveKind to observe the resource when we are not the leader. +// func (r *Reconciler) ObserveKind(ctx {{.contextContext|raw}}, o *{{.type|raw}}) {{.reconcilerEvent|raw}} { +// // TODO: add custom observation logic here. +// return nil +// } + +// Optionally, use ObserveFinalizeKind to observe resources being finalized when we are no the leader. +//func (r *Reconciler) ObserveFinalizeKind(ctx {{.contextContext|raw}}, o *{{.type|raw}}) {{.reconcilerEvent|raw}} { +// // TODO: add custom observation logic here. +// return nil +//} ` diff --git a/hack/update-codegen.sh b/hack/update-codegen.sh index 1337f81100..8a21983350 100755 --- a/hack/update-codegen.sh +++ b/hack/update-codegen.sh @@ -19,6 +19,12 @@ set -o nounset set -o pipefail export GO111MODULE=on +# If we run with -mod=vendor here, then generate-groups.sh looks for vendor files in the wrong place. +export GOFLAGS=-mod= + +if [ -z "${GOPATH:-}" ]; then + export GOPATH=$(go env GOPATH) +fi source $(dirname $0)/../vendor/knative.dev/test-infra/scripts/library.sh