diff --git a/README.md b/README.md index 7b82833..103aa37 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,7 @@ Within an existing Kubebuilder or controller-runtime project, reconcilers.io may - [Higher-order Reconcilers](#higher-order-reconcilers) - [CastResource](#castresource) - [Sequence](#sequence) + - [Advice](#advice) - [IfThen](#ifthen) - [While](#while) - [TryCatch](#trycatch) @@ -457,6 +458,33 @@ func FunctionReconciler(c reconcilers.Config) *reconcilers.ResourceReconciler[*b ``` [full source](https://github.com/projectriff/system/blob/4c3b75327bf99cc37b57ba14df4c65d21dc79d28/pkg/controllers/build/function_reconciler.go#L39-L51) +#### Advice + +[`Advice`](https://pkg.go.dev/reconciler.io/runtime/reconcilers#Advice) is a sub reconciler for advising the lifecycle of another sub reconciler in an aspect oriented programming (AOP) style. `Before` is called before the delegated reconciler and `After` afterward. `Around` is used between Before and After to have full control over how the delegated reconciler is called, including suppressing the call, modifying the input or result, or calling the reconciler multiple times. + +**Example:** + +Advice can be used to control calls to a reconciler at a lower level. In this case the reconciler is called twice aggregating the results while returning immediately on error. + +```go +func CallTwice(reconciler reconciler.SubReconciler[*buildv1alpha1.Function]) *reconcilers.SubReconciler[*buildv1alpha1.Function] { + return &reconcilers.Advice[*buildv1alpha1.Function]{ + Reconciler: reconciler, + Around: func(ctx context.Context, resource *resources.TestResource, reconciler reconcilers.SubReconciler[*resources.TestResource]) (reconcile.Result, error) { + result := reconcilers.Result{} + for i := 0; i < 2; i++ { + if r, err := reconciler.Reconcile(ctx, resource); true { + result = reconcilers.AggregateResults(result, r) + } else if err != nil { + return result, err + } + } + return result, nil + }, + } +} +``` + #### IfThen An [`IfThen`](https://pkg.go.dev/reconciler.io/runtime/reconcilers#IfThen) branches execution of the current reconcile request based on a condition. The false `Else` branch is optional and ignored if not defined. diff --git a/reconcilers/advice.go b/reconcilers/advice.go new file mode 100644 index 0000000..7f3bda3 --- /dev/null +++ b/reconcilers/advice.go @@ -0,0 +1,155 @@ +/* +Copyright 2024 the original author or authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package reconcilers + +import ( + "context" + "fmt" + "sync" + + "github.com/go-logr/logr" + ctrl "sigs.k8s.io/controller-runtime" + "sigs.k8s.io/controller-runtime/pkg/builder" + "sigs.k8s.io/controller-runtime/pkg/client" +) + +var ( + _ SubReconciler[client.Object] = (*Advice[client.Object])(nil) +) + +// Advice is a sub reconciler for advising the lifecycle of another sub reconciler in an aspect +// oriented programming style. +type Advice[Type client.Object] struct { + // Name used to identify this reconciler. Defaults to `Advice`. Ideally unique, but + // not required to be so. + // + // +optional + Name string + + // Setup performs initialization on the manager and builder this reconciler + // will run with. It's common to setup field indexes and watch resources. + // + // +optional + Setup func(ctx context.Context, mgr ctrl.Manager, bldr *builder.Builder) error + + // Reconciler being advised + Reconciler SubReconciler[Type] + + // Before is called preceding Around. A modified context may be returned. Errors are returned + // immediately. + // + // If Before is not defined, there is no effect. + // + // +optional + Before func(ctx context.Context, resource Type) (context.Context, Result, error) + + // Around is responsible for invoking the reconciler and returning the result. Implementations + // may choose to not invoke the reconciler, invoke a different reconciler or invoke the + // reconciler multiple times. + // + // If Around is not defined, the Reconciler is invoked once. + // + // +optional + Around func(ctx context.Context, resource Type, reconciler SubReconciler[Type]) (Result, error) + + // After is called following Around. The result and error from Around are provided and may be + // modified before returning. + // + // If After is not defined, the result and error are returned directly. + // + // +optional + After func(ctx context.Context, resource Type, result Result, err error) (Result, error) + + lazyInit sync.Once +} + +func (r *Advice[T]) init() { + r.lazyInit.Do(func() { + if r.Name == "" { + r.Name = "Advice" + } + if r.Before == nil { + r.Before = func(ctx context.Context, resource T) (context.Context, Result, error) { + return nil, Result{}, nil + } + } + if r.Around == nil { + r.Around = func(ctx context.Context, resource T, reconciler SubReconciler[T]) (Result, error) { + return reconciler.Reconcile(ctx, resource) + } + } + if r.After == nil { + r.After = func(ctx context.Context, resource T, result Result, err error) (Result, error) { + return result, err + } + } + }) +} + +func (r *Advice[T]) SetupWithManager(ctx context.Context, mgr ctrl.Manager, bldr *builder.Builder) error { + r.init() + + log := logr.FromContextOrDiscard(ctx). + WithName(r.Name) + ctx = logr.NewContext(ctx, log) + + if r.Setup == nil { + return nil + } + if err := r.validate(ctx); err != nil { + return err + } + if err := r.Setup(ctx, mgr, bldr); err != nil { + return err + } + return r.Reconciler.SetupWithManager(ctx, mgr, bldr) +} + +func (r *Advice[T]) validate(ctx context.Context) error { + if r.Reconciler == nil { + return fmt.Errorf("Advice %q must implement Reconciler", r.Name) + } + if r.Before == nil && r.Around == nil && r.After == nil { + return fmt.Errorf("Advice %q must implement at least one of Before, Around or After", r.Name) + } + + return nil +} + +func (r *Advice[T]) Reconcile(ctx context.Context, resource T) (Result, error) { + r.init() + + log := logr.FromContextOrDiscard(ctx). + WithName(r.Name) + ctx = logr.NewContext(ctx, log) + + // before phase + beforeCtx, result, err := r.Before(ctx, resource) + if err != nil { + return result, err + } + if beforeCtx != nil { + ctx = beforeCtx + } + + // around phase + aroundResult, err := r.Around(ctx, resource, r.Reconciler) + result = AggregateResults(result, aroundResult) + + // after phase + return r.After(ctx, resource, result, err) +} diff --git a/reconcilers/advice_test.go b/reconcilers/advice_test.go new file mode 100644 index 0000000..a527f0c --- /dev/null +++ b/reconcilers/advice_test.go @@ -0,0 +1,227 @@ +/* +Copyright 2024 the original author or authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package reconcilers_test + +import ( + "context" + "errors" + "testing" + + corev1 "k8s.io/api/core/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" + diemetav1 "reconciler.io/dies/apis/meta/v1" + "reconciler.io/runtime/apis" + "reconciler.io/runtime/internal/resources" + "reconciler.io/runtime/internal/resources/dies" + "reconciler.io/runtime/reconcilers" + rtesting "reconciler.io/runtime/testing" + "sigs.k8s.io/controller-runtime/pkg/reconcile" +) + +func TestAdvice(t *testing.T) { + testNamespace := "test-namespace" + testName := "test-resource" + + scheme := runtime.NewScheme() + _ = resources.AddToScheme(scheme) + + resource := dies.TestResourceBlank. + MetadataDie(func(d *diemetav1.ObjectMetaDie) { + d.Namespace(testNamespace) + d.Name(testName) + }). + StatusDie(func(d *dies.TestResourceStatusDie) { + d.ConditionsDie( + diemetav1.ConditionBlank.Type(apis.ConditionReady).Status(metav1.ConditionUnknown).Reason("Initializing"), + ) + }) + + rts := rtesting.SubReconcilerTests[*resources.TestResource]{ + "before can replace the context": { + Resource: resource.DieReleasePtr(), + Metadata: map[string]interface{}{ + "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { + return &reconcilers.Advice[*resources.TestResource]{ + Before: func(ctx context.Context, resource *resources.TestResource) (context.Context, reconcile.Result, error) { + ctx = context.WithValue(ctx, "message", "hello world") + return ctx, reconcile.Result{}, nil + }, + Reconciler: &reconcilers.SyncReconciler[*resources.TestResource]{ + Sync: func(ctx context.Context, resource *resources.TestResource) error { + if resource.Status.Fields == nil { + resource.Status.Fields = map[string]string{} + } + resource.Status.Fields["message"] = ctx.Value("message").(string) + return nil + }, + }, + } + }, + }, + ExpectResource: resource. + StatusDie(func(d *dies.TestResourceStatusDie) { + d.AddField("message", "hello world") + }). + DieReleasePtr(), + }, + "before can augment the result": { + Resource: resource.DieReleasePtr(), + Metadata: map[string]interface{}{ + "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { + return &reconcilers.Advice[*resources.TestResource]{ + Before: func(ctx context.Context, resource *resources.TestResource) (context.Context, reconcile.Result, error) { + return nil, reconcile.Result{Requeue: true}, nil + }, + Reconciler: &reconcilers.SyncReconciler[*resources.TestResource]{ + Sync: func(ctx context.Context, resource *resources.TestResource) error { + if resource.Status.Fields == nil { + resource.Status.Fields = map[string]string{} + } + resource.Status.Fields["message"] = "reconciler called" + return nil + }, + }, + } + }, + }, + ExpectedResult: reconcile.Result{ + Requeue: true, + }, + ExpectResource: resource. + StatusDie(func(d *dies.TestResourceStatusDie) { + d.AddField("message", "reconciler called") + }). + DieReleasePtr(), + }, + "before errors return immediately": { + Resource: resource.DieReleasePtr(), + Metadata: map[string]interface{}{ + "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { + return &reconcilers.Advice[*resources.TestResource]{ + Before: func(ctx context.Context, resource *resources.TestResource) (context.Context, reconcile.Result, error) { + return nil, reconcile.Result{}, errors.New("test") + }, + Reconciler: &reconcilers.SyncReconciler[*resources.TestResource]{ + Sync: func(ctx context.Context, resource *resources.TestResource) error { + t.Errorf("unreachable") + return nil + }, + }, + } + }, + }, + ShouldErr: true, + }, + "around calls the reconciler by default": { + Resource: resource.DieReleasePtr(), + Metadata: map[string]interface{}{ + "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { + // this reconciler would fail validation, but more directly expresses the desired behavior + return &reconcilers.Advice[*resources.TestResource]{ + Reconciler: &reconcilers.SyncReconciler[*resources.TestResource]{ + Sync: func(ctx context.Context, resource *resources.TestResource) error { + c := reconcilers.RetrieveConfigOrDie(ctx) + c.Recorder.Event(resource, corev1.EventTypeNormal, "Called", "reconciler called") + return nil + }, + }, + } + }, + }, + ExpectEvents: []rtesting.Event{ + rtesting.NewEvent(resource, scheme, corev1.EventTypeNormal, "Called", "reconciler called"), + }, + }, + "around can skip the reconciler": { + Resource: resource.DieReleasePtr(), + Metadata: map[string]interface{}{ + "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { + return &reconcilers.Advice[*resources.TestResource]{ + Reconciler: &reconcilers.SyncReconciler[*resources.TestResource]{ + Sync: func(ctx context.Context, resource *resources.TestResource) error { + t.Errorf("unreachable") + return nil + }, + }, + Around: func(ctx context.Context, resource *resources.TestResource, reconciler reconcilers.SubReconciler[*resources.TestResource]) (reconcile.Result, error) { + return reconcilers.Result{}, nil + }, + } + }, + }, + }, + "around can call into the reconciler multiple times": { + Resource: resource.DieReleasePtr(), + Metadata: map[string]interface{}{ + "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { + return &reconcilers.Advice[*resources.TestResource]{ + Reconciler: &reconcilers.SyncReconciler[*resources.TestResource]{ + Sync: func(ctx context.Context, resource *resources.TestResource) error { + c := reconcilers.RetrieveConfigOrDie(ctx) + c.Recorder.Event(resource, corev1.EventTypeNormal, "Called", "reconciler called") + return nil + }, + }, + Around: func(ctx context.Context, resource *resources.TestResource, reconciler reconcilers.SubReconciler[*resources.TestResource]) (reconcile.Result, error) { + result := reconcilers.Result{} + for i := 0; i < 2; i++ { + if r, err := reconciler.Reconcile(ctx, resource); true { + result = reconcilers.AggregateResults(result, r) + } else if err != nil { + return result, err + } + } + return result, nil + }, + } + }, + }, + ExpectEvents: []rtesting.Event{ + rtesting.NewEvent(resource, scheme, corev1.EventTypeNormal, "Called", "reconciler called"), + rtesting.NewEvent(resource, scheme, corev1.EventTypeNormal, "Called", "reconciler called"), + }, + }, + "after can rewrite the result": { + Resource: resource.DieReleasePtr(), + Metadata: map[string]interface{}{ + "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { + return &reconcilers.Advice[*resources.TestResource]{ + Reconciler: &reconcilers.SyncReconciler[*resources.TestResource]{ + Sync: func(ctx context.Context, resource *resources.TestResource) error { + return errors.New("test") + }, + }, + After: func(ctx context.Context, resource *resources.TestResource, result reconcile.Result, err error) (reconcile.Result, error) { + if err == nil { + t.Errorf("expected error") + } + return reconcile.Result{Requeue: true}, nil + }, + } + }, + }, + ExpectedResult: reconcile.Result{ + Requeue: true, + }, + }, + } + + rts.Run(t, scheme, func(t *testing.T, rtc *rtesting.SubReconcilerTestCase[*resources.TestResource], c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { + return rtc.Metadata["SubReconciler"].(func(*testing.T, reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource])(t, c) + }) +} diff --git a/reconcilers/aggregate.go b/reconcilers/aggregate.go index dbcc148..cb9d5ec 100644 --- a/reconcilers/aggregate.go +++ b/reconcilers/aggregate.go @@ -104,6 +104,22 @@ type AggregateReconciler[Type client.Object] struct { // +optional Sanitize func(resource Type) interface{} + // BeforeReconcile is called first thing for each reconcile request. A modified context may be + // returned. Errors are returned immediately. + // + // If BeforeReconcile is not defined, there is no effect. + // + // +optional + BeforeReconcile func(ctx context.Context, req Request) (context.Context, Result, error) + + // AfterReconcile is called following all work for the reconcile request. The result and error + // are provided and may be modified before returning. + // + // If AfterReconcile is not defined, the result and error are returned directly. + // + // +optional + AfterReconcile func(ctx context.Context, req Request, res Result, err error) (Result, error) + Config Config // stamp manages the lifecycle of the aggregated resource. @@ -128,6 +144,16 @@ func (r *AggregateReconciler[T]) init() { return resource, nil } } + if r.BeforeReconcile == nil { + r.BeforeReconcile = func(ctx context.Context, req reconcile.Request) (context.Context, reconcile.Result, error) { + return ctx, Result{}, nil + } + } + if r.AfterReconcile == nil { + r.AfterReconcile = func(ctx context.Context, req reconcile.Request, res reconcile.Result, err error) (reconcile.Result, error) { + return res, err + } + } r.stamp = &ResourceManager[T]{ Name: r.Name, @@ -231,6 +257,23 @@ func (r *AggregateReconciler[T]) Reconcile(ctx context.Context, req Request) (Re ctx = StashOriginalResourceType(ctx, r.Type) ctx = StashResourceType(ctx, r.Type) + beforeCtx, beforeResult, err := r.BeforeReconcile(ctx, req) + if err != nil { + return beforeResult, err + } + if beforeCtx != nil { + ctx = beforeCtx + } + + reconcileResult, err := r.reconcile(ctx, req) + + return r.AfterReconcile(ctx, req, AggregateResults(beforeResult, reconcileResult), err) +} + +func (r *AggregateReconciler[T]) reconcile(ctx context.Context, req Request) (Result, error) { + log := logr.FromContextOrDiscard(ctx) + c := RetrieveConfigOrDie(ctx) + resource := r.Type.DeepCopyObject().(T) if err := c.Get(ctx, req.NamespacedName, resource); err != nil { if apierrs.IsNotFound(err) { @@ -264,13 +307,10 @@ func (r *AggregateReconciler[T]) Reconcile(ctx context.Context, req Request) (Re }) desired, err := r.desiredResource(ctx, resource) if err != nil { - return Result{}, err + return result, err } _, err = r.stamp.Manage(ctx, resource, resource, desired) - if err != nil { - return Result{}, err - } - return result, nil + return result, err } func (r *AggregateReconciler[T]) desiredResource(ctx context.Context, resource T) (T, error) { diff --git a/reconcilers/aggregate_test.go b/reconcilers/aggregate_test.go index f962cf6..9ce12c8 100644 --- a/reconcilers/aggregate_test.go +++ b/reconcilers/aggregate_test.go @@ -18,6 +18,7 @@ package reconcilers_test import ( "context" + "errors" "fmt" "testing" "time" @@ -32,6 +33,7 @@ import ( "reconciler.io/runtime/internal/resources" "reconciler.io/runtime/reconcilers" rtesting "reconciler.io/runtime/testing" + rtime "reconciler.io/runtime/time" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -506,6 +508,136 @@ func TestAggregateReconciler(t *testing.T) { return ctx, nil }, }, + "before reconcile is called before reconcile and after the context is populated": { + Request: request, + Metadata: map[string]interface{}{ + "Reconciler": func(t *testing.T, c reconcilers.Config) reconcile.Reconciler { + r := defaultAggregateReconciler(c) + r.BeforeReconcile = func(ctx context.Context, req reconcile.Request) (context.Context, reconcile.Result, error) { + c := reconcilers.RetrieveConfigOrDie(ctx) + // create the object manually rather than as a given + r := configMapCreate. + MetadataDie(func(d *diemetav1.ObjectMetaDie) { + d.CreationTimestamp(metav1.NewTime(rtime.RetrieveNow(ctx))) + }). + DieReleasePtr() + err := c.Create(ctx, r) + return nil, reconcile.Result{}, err + } + return r + }, + }, + ExpectEvents: []rtesting.Event{ + rtesting.NewEvent(configMapGiven, scheme, corev1.EventTypeNormal, "Updated", + `Updated ConfigMap %q`, testName), + }, + ExpectCreates: []client.Object{ + configMapCreate, + }, + ExpectUpdates: []client.Object{ + configMapGiven. + AddData("foo", "bar"), + }, + }, + "before reconcile can influence the result": { + Request: request, + GivenObjects: []client.Object{ + configMapGiven, + }, + Metadata: map[string]interface{}{ + "Reconciler": func(t *testing.T, c reconcilers.Config) reconcile.Reconciler { + r := defaultAggregateReconciler(c) + r.BeforeReconcile = func(ctx context.Context, req reconcile.Request) (context.Context, reconcile.Result, error) { + return nil, reconcile.Result{Requeue: true}, nil + } + return r + }, + }, + ExpectedResult: reconcile.Result{ + Requeue: true, + }, + ExpectEvents: []rtesting.Event{ + rtesting.NewEvent(configMapGiven, scheme, corev1.EventTypeNormal, "Updated", + `Updated ConfigMap %q`, testName), + }, + ExpectUpdates: []client.Object{ + configMapGiven. + AddData("foo", "bar"), + }, + }, + "before reconcile can replace the context": { + Request: request, + GivenObjects: []client.Object{ + configMapGiven, + }, + Metadata: map[string]interface{}{ + "Reconciler": func(t *testing.T, c reconcilers.Config) reconcile.Reconciler { + r := defaultAggregateReconciler(c) + r.BeforeReconcile = func(ctx context.Context, req reconcile.Request) (context.Context, reconcile.Result, error) { + ctx = context.WithValue(ctx, "message", "hello world") + return ctx, reconcile.Result{}, nil + } + r.DesiredResource = func(ctx context.Context, resource *corev1.ConfigMap) (*corev1.ConfigMap, error) { + resource.Data = map[string]string{ + "message": ctx.Value("message").(string), + } + return resource, nil + } + return r + }, + }, + ExpectEvents: []rtesting.Event{ + rtesting.NewEvent(configMapGiven, scheme, corev1.EventTypeNormal, "Updated", + `Updated ConfigMap %q`, testName), + }, + ExpectUpdates: []client.Object{ + configMapGiven. + AddData("message", "hello world"), + }, + }, + "before reconcile errors shortcut execution": { + Request: request, + GivenObjects: []client.Object{ + configMapGiven, + }, + Metadata: map[string]interface{}{ + "Reconciler": func(t *testing.T, c reconcilers.Config) reconcile.Reconciler { + r := defaultAggregateReconciler(c) + r.BeforeReconcile = func(ctx context.Context, req reconcile.Request) (context.Context, reconcile.Result, error) { + return nil, reconcile.Result{}, errors.New("test") + } + return r + }, + }, + ShouldErr: true, + }, + "after reconcile can overwrite the result": { + Request: request, + GivenObjects: []client.Object{ + configMapGiven, + }, + WithReactors: []rtesting.ReactionFunc{ + rtesting.InduceFailure("update", "ConfigMap"), + }, + Metadata: map[string]interface{}{ + "Reconciler": func(t *testing.T, c reconcilers.Config) reconcile.Reconciler { + r := defaultAggregateReconciler(c) + r.AfterReconcile = func(ctx context.Context, req reconcile.Request, res reconcile.Result, err error) (reconcile.Result, error) { + // suppress error + return reconcile.Result{}, nil + } + return r + }, + }, + ExpectEvents: []rtesting.Event{ + rtesting.NewEvent(configMapGiven, scheme, corev1.EventTypeWarning, "UpdateFailed", + `Failed to update ConfigMap %q: inducing failure for update ConfigMap`, testName), + }, + ExpectUpdates: []client.Object{ + configMapGiven. + AddData("foo", "bar"), + }, + }, } rts.Run(t, scheme, func(t *testing.T, rtc *rtesting.ReconcilerTestCase, c reconcilers.Config) reconcile.Reconciler { diff --git a/reconcilers/resource.go b/reconcilers/resource.go index 956fd2e..6325bca 100644 --- a/reconcilers/resource.go +++ b/reconcilers/resource.go @@ -83,6 +83,22 @@ type ResourceReconciler[Type client.Object] struct { // returned. Reconciler SubReconciler[Type] + // BeforeReconcile is called first thing for each reconcile request. A modified context may be + // returned. Errors are returned immediately. + // + // If BeforeReconcile is not defined, there is no effect. + // + // +optional + BeforeReconcile func(ctx context.Context, req Request) (context.Context, Result, error) + + // AfterReconcile is called following all work for the reconcile request. The result and error + // are provided and may be modified before returning. + // + // If AfterReconcile is not defined, the result and error are returned directly. + // + // +optional + AfterReconcile func(ctx context.Context, req Request, res Result, err error) (Result, error) + Config Config lazyInit sync.Once @@ -97,6 +113,16 @@ func (r *ResourceReconciler[T]) init() { if r.Name == "" { r.Name = fmt.Sprintf("%sResourceReconciler", typeName(r.Type)) } + if r.BeforeReconcile == nil { + r.BeforeReconcile = func(ctx context.Context, req reconcile.Request) (context.Context, reconcile.Result, error) { + return ctx, Result{}, nil + } + } + if r.AfterReconcile == nil { + r.AfterReconcile = func(ctx context.Context, req reconcile.Request, res reconcile.Result, err error) (reconcile.Result, error) { + return res, err + } + } }) } @@ -208,6 +234,24 @@ func (r *ResourceReconciler[T]) Reconcile(ctx context.Context, req Request) (Res ctx = StashOriginalConfig(ctx, c) ctx = StashOriginalResourceType(ctx, r.Type) ctx = StashResourceType(ctx, r.Type) + + beforeCtx, beforeResult, err := r.BeforeReconcile(ctx, req) + if err != nil { + return beforeResult, err + } + if beforeCtx != nil { + ctx = beforeCtx + } + + reconcileResult, err := r.reconcileOuter(ctx, req) + + return r.AfterReconcile(ctx, req, AggregateResults(beforeResult, reconcileResult), err) +} + +func (r *ResourceReconciler[T]) reconcileOuter(ctx context.Context, req Request) (Result, error) { + log := logr.FromContextOrDiscard(ctx) + c := RetrieveOriginalConfigOrDie(ctx) + originalResource := r.Type.DeepCopyObject().(T) if err := c.Get(ctx, req.NamespacedName, originalResource); err != nil { @@ -220,6 +264,7 @@ func (r *ResourceReconciler[T]) Reconcile(ctx context.Context, req Request) (Res if !errors.Is(err, ErrQuiet) { log.Error(err, "unable to fetch resource") } + return Result{}, err } resource := originalResource.DeepCopyObject().(T) @@ -230,7 +275,7 @@ func (r *ResourceReconciler[T]) Reconcile(ctx context.Context, req Request) (Res } r.initializeConditions(ctx, resource) - result, err := r.reconcile(ctx, resource) + result, err := r.reconcileInner(ctx, resource) if r.SkipStatusUpdate { return result, err @@ -251,7 +296,8 @@ func (r *ResourceReconciler[T]) Reconcile(ctx context.Context, req Request) (Res c.Recorder.Eventf(resource, corev1.EventTypeWarning, "StatusPatchFailed", "Failed to patch status: %v", patchErr) } - return Result{}, patchErr + + return result, patchErr } c.Recorder.Eventf(resource, corev1.EventTypeNormal, "StatusPatched", "Patched status") @@ -264,7 +310,7 @@ func (r *ResourceReconciler[T]) Reconcile(ctx context.Context, req Request) (Res c.Recorder.Eventf(resource, corev1.EventTypeWarning, "StatusUpdateFailed", "Failed to update status: %v", updateErr) } - return Result{}, updateErr + return result, updateErr } c.Recorder.Eventf(resource, corev1.EventTypeNormal, "StatusUpdated", "Updated status") @@ -275,7 +321,7 @@ func (r *ResourceReconciler[T]) Reconcile(ctx context.Context, req Request) (Res return result, err } -func (r *ResourceReconciler[T]) reconcile(ctx context.Context, resource T) (Result, error) { +func (r *ResourceReconciler[T]) reconcileInner(ctx context.Context, resource T) (Result, error) { if resource.GetDeletionTimestamp() != nil && len(resource.GetFinalizers()) == 0 { // resource is being deleted and has no pending finalizers, nothing to do return Result{}, nil diff --git a/reconcilers/resource_test.go b/reconcilers/resource_test.go index 137b3d9..1d1a543 100644 --- a/reconcilers/resource_test.go +++ b/reconcilers/resource_test.go @@ -18,6 +18,7 @@ package reconcilers_test import ( "context" + "errors" "fmt" "testing" "time" @@ -34,6 +35,7 @@ import ( "reconciler.io/runtime/internal/resources/dies" "reconciler.io/runtime/reconcilers" rtesting "reconciler.io/runtime/testing" + rtime "reconciler.io/runtime/time" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/reconcile" ) @@ -861,11 +863,12 @@ func TestResourceReconciler(t *testing.T) { scheme := runtime.NewScheme() _ = resources.AddToScheme(scheme) - resource := dies.TestResourceBlank. + createResource := dies.TestResourceBlank. MetadataDie(func(d *diemetav1.ObjectMetaDie) { d.Namespace(testNamespace) d.Name(testName) - }). + }) + givenResource := createResource. StatusDie(func(d *dies.TestResourceStatusDie) { d.ConditionsDie( diemetav1.ConditionBlank.Type(apis.ConditionReady).Status(metav1.ConditionUnknown).Reason("Initializing"), @@ -896,7 +899,7 @@ func TestResourceReconciler(t *testing.T) { &resources.TestResource{}, }, GivenObjects: []client.Object{ - resource.MetadataDie(func(d *diemetav1.ObjectMetaDie) { + givenResource.MetadataDie(func(d *diemetav1.ObjectMetaDie) { d.DeletionTimestamp(&deletedAt) d.Finalizers(testFinalizer) }), @@ -918,7 +921,7 @@ func TestResourceReconciler(t *testing.T) { &resources.TestResource{}, }, GivenObjects: []client.Object{ - resource, + givenResource, }, WithReactors: []rtesting.ReactionFunc{ rtesting.InduceFailure("get", "TestResource"), @@ -941,7 +944,7 @@ func TestResourceReconciler(t *testing.T) { &resources.TestResource{}, }, GivenObjects: []client.Object{ - resource, + givenResource, }, Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { @@ -962,7 +965,7 @@ func TestResourceReconciler(t *testing.T) { &resources.TestResource{}, }, GivenObjects: []client.Object{ - resource.StatusDie(func(d *dies.TestResourceStatusDie) { + givenResource.StatusDie(func(d *dies.TestResourceStatusDie) { d.ConditionsDie() }), }, @@ -982,11 +985,11 @@ func TestResourceReconciler(t *testing.T) { }, }, ExpectEvents: []rtesting.Event{ - rtesting.NewEvent(resource, scheme, corev1.EventTypeNormal, "StatusUpdated", + rtesting.NewEvent(givenResource, scheme, corev1.EventTypeNormal, "StatusUpdated", `Updated status`), }, ExpectStatusUpdates: []client.Object{ - resource, + givenResource, }, }, "reconciler mutated status": { @@ -995,7 +998,7 @@ func TestResourceReconciler(t *testing.T) { &resources.TestResource{}, }, GivenObjects: []client.Object{ - resource, + givenResource, }, Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { @@ -1011,11 +1014,11 @@ func TestResourceReconciler(t *testing.T) { }, }, ExpectEvents: []rtesting.Event{ - rtesting.NewEvent(resource, scheme, corev1.EventTypeNormal, "StatusUpdated", + rtesting.NewEvent(givenResource, scheme, corev1.EventTypeNormal, "StatusUpdated", `Updated status`), }, ExpectStatusUpdates: []client.Object{ - resource.StatusDie(func(d *dies.TestResourceStatusDie) { + givenResource.StatusDie(func(d *dies.TestResourceStatusDie) { d.AddField("Reconciler", "ran") }), }, @@ -1023,7 +1026,7 @@ func TestResourceReconciler(t *testing.T) { "skip status updates": { Request: testRequest, GivenObjects: []client.Object{ - resource, + givenResource, }, Metadata: map[string]interface{}{ "SkipStatusUpdate": true, @@ -1046,7 +1049,7 @@ func TestResourceReconciler(t *testing.T) { &resources.TestResource{}, }, GivenObjects: []client.Object{ - resource, + givenResource, }, Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { @@ -1065,7 +1068,7 @@ func TestResourceReconciler(t *testing.T) { &resources.TestResource{}, }, GivenObjects: []client.Object{ - resource, + givenResource, }, Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { @@ -1090,11 +1093,11 @@ func TestResourceReconciler(t *testing.T) { }, }, ExpectEvents: []rtesting.Event{ - rtesting.NewEvent(resource, scheme, corev1.EventTypeNormal, "StatusUpdated", + rtesting.NewEvent(givenResource, scheme, corev1.EventTypeNormal, "StatusUpdated", `Updated status`), }, ExpectStatusUpdates: []client.Object{ - resource.StatusDie(func(d *dies.TestResourceStatusDie) { + givenResource.StatusDie(func(d *dies.TestResourceStatusDie) { d.AddField("want", "this to run") }), }, @@ -1105,7 +1108,7 @@ func TestResourceReconciler(t *testing.T) { &resources.TestResource{}, }, GivenObjects: []client.Object{ - resource, + givenResource, }, Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { @@ -1130,11 +1133,11 @@ func TestResourceReconciler(t *testing.T) { }, }, ExpectEvents: []rtesting.Event{ - rtesting.NewEvent(resource, scheme, corev1.EventTypeNormal, "StatusUpdated", + rtesting.NewEvent(givenResource, scheme, corev1.EventTypeNormal, "StatusUpdated", `Updated status`), }, ExpectStatusUpdates: []client.Object{ - resource.StatusDie(func(d *dies.TestResourceStatusDie) { + givenResource.StatusDie(func(d *dies.TestResourceStatusDie) { d.AddField("want", "this to run") }), }, @@ -1146,7 +1149,7 @@ func TestResourceReconciler(t *testing.T) { &resources.TestResource{}, }, GivenObjects: []client.Object{ - resource, + givenResource, }, WithReactors: []rtesting.ReactionFunc{ rtesting.InduceFailure("update", "TestResource", rtesting.InduceFailureOpts{ @@ -1167,11 +1170,11 @@ func TestResourceReconciler(t *testing.T) { }, }, ExpectEvents: []rtesting.Event{ - rtesting.NewEvent(resource, scheme, corev1.EventTypeWarning, "StatusUpdateFailed", + rtesting.NewEvent(givenResource, scheme, corev1.EventTypeWarning, "StatusUpdateFailed", `Failed to update status: inducing failure for update TestResource`), }, ExpectStatusUpdates: []client.Object{ - resource.StatusDie(func(d *dies.TestResourceStatusDie) { + givenResource.StatusDie(func(d *dies.TestResourceStatusDie) { d.AddField("Reconciler", "ran") }), }, @@ -1183,7 +1186,7 @@ func TestResourceReconciler(t *testing.T) { &resources.TestResource{}, }, GivenObjects: []client.Object{ - resource, + givenResource, }, Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { @@ -1204,7 +1207,7 @@ func TestResourceReconciler(t *testing.T) { &resources.TestResource{}, }, GivenObjects: []client.Object{ - resource, + givenResource, }, Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { @@ -1228,7 +1231,7 @@ func TestResourceReconciler(t *testing.T) { &resources.TestResource{}, }, GivenObjects: []client.Object{ - resource, + givenResource, }, Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { @@ -1252,7 +1255,7 @@ func TestResourceReconciler(t *testing.T) { &resources.TestResource{}, }, GivenObjects: []client.Object{ - resource, + givenResource, }, Prepare: func(t *testing.T, ctx context.Context, tc *rtesting.ReconcilerTestCase) (context.Context, error) { key := "test-key" @@ -1279,6 +1282,149 @@ func TestResourceReconciler(t *testing.T) { return ctx, nil }, }, + "before reconcile is called before reconcile and after the context is populated": { + Request: testRequest, + StatusSubResourceTypes: []client.Object{ + &resources.TestResource{}, + }, + Metadata: map[string]interface{}{ + "BeforeReconcile": func(ctx context.Context, req reconcilers.Request) (context.Context, reconcilers.Result, error) { + c := reconcilers.RetrieveConfigOrDie(ctx) + // create the object manually rather than as a given + r := createResource. + MetadataDie(func(d *diemetav1.ObjectMetaDie) { + d.CreationTimestamp(metav1.NewTime(rtime.RetrieveNow(ctx))) + }). + DieReleasePtr() + err := c.Create(ctx, r) + return nil, reconcile.Result{}, err + }, + "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { + return &reconcilers.SyncReconciler[*resources.TestResource]{ + Sync: func(ctx context.Context, resource *resources.TestResource) error { + if resource.Status.Fields == nil { + resource.Status.Fields = map[string]string{} + } + resource.Status.Fields["Reconciler"] = "ran" + return nil + }, + } + }, + }, + ExpectEvents: []rtesting.Event{ + rtesting.NewEvent(givenResource, scheme, corev1.EventTypeNormal, "StatusUpdated", + `Updated status`), + }, + ExpectCreates: []client.Object{ + createResource, + }, + ExpectStatusUpdates: []client.Object{ + givenResource.StatusDie(func(d *dies.TestResourceStatusDie) { + d.AddField("Reconciler", "ran") + }), + }, + }, + "before reconcile can influence the result": { + Request: testRequest, + StatusSubResourceTypes: []client.Object{ + &resources.TestResource{}, + }, + GivenObjects: []client.Object{ + givenResource, + }, + Metadata: map[string]interface{}{ + "BeforeReconcile": func(ctx context.Context, req reconcilers.Request) (context.Context, reconcilers.Result, error) { + return nil, reconcile.Result{Requeue: true}, nil + }, + "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { + return &reconcilers.SyncReconciler[*resources.TestResource]{ + Sync: func(ctx context.Context, resource *resources.TestResource) error { + return nil + }, + } + }, + }, + ExpectedResult: reconcile.Result{ + Requeue: true, + }, + }, + "before reconcile can replace the context": { + Request: testRequest, + StatusSubResourceTypes: []client.Object{ + &resources.TestResource{}, + }, + GivenObjects: []client.Object{ + givenResource, + }, + Metadata: map[string]interface{}{ + "BeforeReconcile": func(ctx context.Context, req reconcilers.Request) (context.Context, reconcilers.Result, error) { + ctx = context.WithValue(ctx, "message", "hello world") + return ctx, reconcile.Result{}, nil + }, + "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { + return &reconcilers.SyncReconciler[*resources.TestResource]{ + Sync: func(ctx context.Context, resource *resources.TestResource) error { + if resource.Status.Fields == nil { + resource.Status.Fields = map[string]string{} + } + resource.Status.Fields["message"] = ctx.Value("message").(string) + return nil + }, + } + }, + }, + ExpectEvents: []rtesting.Event{ + rtesting.NewEvent(givenResource, scheme, corev1.EventTypeNormal, "StatusUpdated", + `Updated status`), + }, + ExpectStatusUpdates: []client.Object{ + givenResource.StatusDie(func(d *dies.TestResourceStatusDie) { + d.AddField("message", "hello world") + }), + }, + }, + "before reconcile errors shortcut execution": { + Request: testRequest, + StatusSubResourceTypes: []client.Object{ + &resources.TestResource{}, + }, + Metadata: map[string]interface{}{ + "BeforeReconcile": func(ctx context.Context, req reconcilers.Request) (context.Context, reconcilers.Result, error) { + return nil, reconcile.Result{}, errors.New("test") + }, + "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { + return &reconcilers.SyncReconciler[*resources.TestResource]{ + Sync: func(ctx context.Context, resource *resources.TestResource) error { + t.Error("should not be called") + return nil + }, + } + }, + }, + ShouldErr: true, + }, + "after reconcile can overwrite the result": { + Request: testRequest, + StatusSubResourceTypes: []client.Object{ + &resources.TestResource{}, + }, + GivenObjects: []client.Object{ + givenResource, + }, + Metadata: map[string]interface{}{ + "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource] { + return &reconcilers.SyncReconciler[*resources.TestResource]{ + Sync: func(ctx context.Context, resource *resources.TestResource) error { + return errors.New("test") + }, + } + }, + "AfterReconcile": func(ctx context.Context, req reconcile.Request, res reconcile.Result, err error) (reconcile.Result, error) { + // suppress error + return reconcile.Result{}, nil + }, + }, + }, } rts.Run(t, scheme, func(t *testing.T, rtc *rtesting.ReconcilerTestCase, c reconcilers.Config) reconcile.Reconciler { @@ -1286,9 +1432,19 @@ func TestResourceReconciler(t *testing.T) { if skip, ok := rtc.Metadata["SkipStatusUpdate"].(bool); ok { skipStatusUpdate = skip } + var beforeReconcile func(context.Context, reconcilers.Request) (context.Context, reconcilers.Result, error) + if before, ok := rtc.Metadata["BeforeReconcile"].(func(context.Context, reconcilers.Request) (context.Context, reconcilers.Result, error)); ok { + beforeReconcile = before + } + var afterReconcile func(context.Context, reconcilers.Request, reconcilers.Result, error) (reconcilers.Result, error) + if after, ok := rtc.Metadata["AfterReconcile"].(func(context.Context, reconcilers.Request, reconcilers.Result, error) (reconcilers.Result, error)); ok { + afterReconcile = after + } return &reconcilers.ResourceReconciler[*resources.TestResource]{ Reconciler: rtc.Metadata["SubReconciler"].(func(*testing.T, reconcilers.Config) reconcilers.SubReconciler[*resources.TestResource])(t, c), SkipStatusUpdate: skipStatusUpdate, + BeforeReconcile: beforeReconcile, + AfterReconcile: afterReconcile, Config: c, } }) diff --git a/reconcilers/validate_test.go b/reconcilers/validate_test.go index c992743..031f8b4 100644 --- a/reconcilers/validate_test.go +++ b/reconcilers/validate_test.go @@ -1182,6 +1182,86 @@ func TestOverrideSetup_validate(t *testing.T) { } } +func TestAdvice_validate(t *testing.T) { + tests := []struct { + name string + resource client.Object + reconciler *Advice[*corev1.ConfigMap] + shouldErr string + }{ + { + name: "empty", + resource: &corev1.ConfigMap{}, + reconciler: &Advice[*corev1.ConfigMap]{}, + shouldErr: `Advice "" must implement Reconciler`, + }, + { + name: "reconciler only", + resource: &corev1.ConfigMap{}, + reconciler: &Advice[*corev1.ConfigMap]{ + Reconciler: &SyncReconciler[*corev1.ConfigMap]{}, + }, + shouldErr: `Advice "" must implement at least one of Before, Around or After`, + }, + { + name: "valid", + resource: &corev1.ConfigMap{}, + reconciler: &Advice[*corev1.ConfigMap]{ + Reconciler: &SyncReconciler[*corev1.ConfigMap]{}, + Before: func(ctx context.Context, resource *corev1.ConfigMap) (context.Context, reconcile.Result, error) { + return nil, reconcile.Result{}, nil + }, + Around: func(ctx context.Context, resource *corev1.ConfigMap, reconciler SubReconciler[*corev1.ConfigMap]) (reconcile.Result, error) { + return reconcile.Result{}, nil + }, + After: func(ctx context.Context, resource *corev1.ConfigMap, result reconcile.Result, err error) (reconcile.Result, error) { + return reconcile.Result{}, nil + }, + }, + }, + { + name: "valid before", + resource: &corev1.ConfigMap{}, + reconciler: &Advice[*corev1.ConfigMap]{ + Reconciler: &SyncReconciler[*corev1.ConfigMap]{}, + Before: func(ctx context.Context, resource *corev1.ConfigMap) (context.Context, reconcile.Result, error) { + return nil, reconcile.Result{}, nil + }, + }, + }, + { + name: "valid around", + resource: &corev1.ConfigMap{}, + reconciler: &Advice[*corev1.ConfigMap]{ + Reconciler: &SyncReconciler[*corev1.ConfigMap]{}, + Around: func(ctx context.Context, resource *corev1.ConfigMap, reconciler SubReconciler[*corev1.ConfigMap]) (reconcile.Result, error) { + return reconcile.Result{}, nil + }, + }, + }, + { + name: "valid after", + resource: &corev1.ConfigMap{}, + reconciler: &Advice[*corev1.ConfigMap]{ + Reconciler: &SyncReconciler[*corev1.ConfigMap]{}, + After: func(ctx context.Context, resource *corev1.ConfigMap, result reconcile.Result, err error) (reconcile.Result, error) { + return reconcile.Result{}, nil + }, + }, + }, + } + + for _, c := range tests { + t.Run(c.name, func(t *testing.T) { + ctx := StashResourceType(context.TODO(), c.resource) + err := c.reconciler.validate(ctx) + if (err != nil) != (c.shouldErr != "") || (c.shouldErr != "" && c.shouldErr != err.Error()) { + t.Errorf("validate() error = %q, shouldErr %q", err, c.shouldErr) + } + }) + } +} + var _ logr.LogSink = &bufferedSink{} type bufferedSink struct { diff --git a/reconcilers/webhook.go b/reconcilers/webhook.go index 843e5ed..900f6fe 100644 --- a/reconcilers/webhook.go +++ b/reconcilers/webhook.go @@ -74,6 +74,22 @@ type AdmissionWebhookAdapter[Type client.Object] struct { // Typically, Reconciler is a Sequence of multiple SubReconcilers. Reconciler SubReconciler[Type] + // BeforeHandle is called first thing for each admission request. A modified context may be + // returned. + // + // If BeforeHandle is not defined, there is no effect. + // + // +optional + BeforeHandle func(ctx context.Context, req admission.Request, resp *admission.Response) context.Context + + // AfterHandle is called following all work for the admission request. The response is provided + // and may be modified before returning. + // + // If AfterHandle is not defined, the response is returned directly. + // + // +optional + AfterHandle func(ctx context.Context, req admission.Request, resp *admission.Response) + Config Config lazyInit sync.Once @@ -88,6 +104,14 @@ func (r *AdmissionWebhookAdapter[T]) init() { if r.Name == "" { r.Name = fmt.Sprintf("%sAdmissionWebhookAdapter", typeName(r.Type)) } + if r.BeforeHandle == nil { + r.BeforeHandle = func(ctx context.Context, req admission.Request, resp *admission.Response) context.Context { + return ctx + } + } + if r.AfterHandle == nil { + r.AfterHandle = func(ctx context.Context, req admission.Request, resp *admission.Response) {} + } }) } @@ -142,6 +166,10 @@ func (r *AdmissionWebhookAdapter[T]) Handle(ctx context.Context, req admission.R ctx = StashAdmissionRequest(ctx, req) ctx = StashAdmissionResponse(ctx, resp) + if beforeCtx := r.BeforeHandle(ctx, req, resp); beforeCtx != nil { + ctx = beforeCtx + } + // defined for compatibility since this is not a reconciler ctx = StashRequest(ctx, Request{ NamespacedName: types.NamespacedName{ @@ -163,6 +191,7 @@ func (r *AdmissionWebhookAdapter[T]) Handle(ctx context.Context, req admission.R } } + r.AfterHandle(ctx, req, resp) return *resp }