diff --git a/README.md b/README.md index bd2d5c3..bb9e544 100644 --- a/README.md +++ b/README.md @@ -16,6 +16,7 @@ - [Higher-order Reconcilers](#higher-order-reconcilers) - [CastParent](#castparent) - [Sequence](#sequence) + - [WithConfig](#withconfig) - [Testing](#testing) - [ReconcilerTestSuite](#reconcilertestsuite) - [SubReconcilerTestSuite](#subreconcilertestsuite) @@ -93,8 +94,6 @@ func FunctionTargetImageReconciler(c reconcilers.Config) reconcilers.SubReconcil parent.Status.TargetImage = targetImage return nil }, - - Config: c, } } ``` @@ -192,8 +191,6 @@ func FunctionChildImageReconciler(c reconcilers.Config) reconcilers.SubReconcile // up in our logs return child.Spec }, - - Config: c, } } ``` @@ -224,7 +221,6 @@ func FunctionReconciler(c reconcilers.Config) *reconcilers.ParentReconciler { // do something with the duckv1alpha1.ImageRef instead of a buildv1alpha1.Function return nil }, - Config: c, }, }, FunctionChildImageReconciler(c), @@ -259,6 +255,31 @@ func FunctionReconciler(c reconcilers.Config) *reconcilers.ParentReconciler { ``` [full source](https://github.com/projectriff/system/blob/4c3b75327bf99cc37b57ba14df4c65d21dc79d28/pkg/controllers/build/function_reconciler.go#L39-L51) +#### WithConfig + +[`WithConfig`](https://pkg.go.dev/github.com/vmware-labs/reconciler-runtime/reconcilers#WithConfig) overrides the config that nested reconcilers consume. The config can be retrieved from the context via [`RetrieveConfig`](https://pkg.go.dev/github.com/vmware-labs/reconciler-runtime/reconcilers#RetrieveConfig). The config used to load the parent resource should be used for interactions with the parent resource, which can be retrieved from the context via [`RetrieveParentConfig`](https://pkg.go.dev/github.com/vmware-labs/reconciler-runtime/reconcilers#RetrieveParentConfig). + +**Example:** + +`WithConfig` can be used to change the REST Config backing the clients. This could be to make requests to the same cluster with a user defined service account, or target an entirely different Kubernetes cluster. + +```go +func SwapRESTConfig(c reconciler.Config, rc *rest.Config) (*reconcilers.SubReconciler, error) { + cl, err := clusters.New(rc) + if err != nil { + return nil, err + } + + return &reconcilers.WithConfig{ + Reconciler: reconcilers.Sequence{ + LookupReferenceDataReconciler(), + DoSomethingChildReconciler(), + }, + + Config: c.WithCluster(cl), + }, nil +} +``` ## Testing @@ -395,8 +416,6 @@ func StashExampleSubReconciler(c reconcilers.Config) reconcilers.SubReconciler { reconcilers.StashValue(ctx, exampleStashKey, *value) return nil }, - - Config: c, } } @@ -411,8 +430,6 @@ func StashExampleSubReconciler(c reconcilers.Config) reconcilers.SubReconciler { } ... // do something with the value }, - - Config: c, } } ``` @@ -431,6 +448,7 @@ func InMemoryGatewaySyncConfigReconciler(c reconcilers.Config, namespace string) Name: "SyncConfig", Sync: func(ctx context.Context, parent *streamingv1alpha1.InMemoryGateway) error { log := logr.FromContextOrDiscard(ctx) + c := reconciler.RetrieveConfig(ctx) var config corev1.ConfigMap key := types.NamespacedName{Namespace: namespace, Name: inmemoryGatewayImages} @@ -451,14 +469,13 @@ func InMemoryGatewaySyncConfigReconciler(c reconcilers.Config, namespace string) return nil }, - Config: c, Setup: func(ctx context.Context, mgr reconcilers.Manager, bldr *reconcilers.Builder) error { // enqueue the tracking resource for reconciliation from changes to // tracked ConfigMaps. Internally `EnqueueTracked` sets up an // Informer to watch to changes of the target resource. When the // informer emits an event, the tracking resources are looked up // from the tracker and enqueded for reconciliation. - bldr.Watches(&source.Kind{Type: &corev1.ConfigMap{}}, reconcilers.EnqueueTracked(ctx, &corev1.ConfigMap{}, c.Tracker, c.Scheme)) + bldr.Watches(&source.Kind{Type: &corev1.ConfigMap{}}, reconcilers.EnqueueTracked(ctx, &corev1.ConfigMap{})) return nil }, } diff --git a/reconcilers/enqueuer.go b/reconcilers/enqueuer.go index 5a7b171..24971ab 100644 --- a/reconcilers/enqueuer.go +++ b/reconcilers/enqueuer.go @@ -8,7 +8,6 @@ package reconcilers import ( "context" - "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/types" "sigs.k8s.io/controller-runtime/pkg/client" "sigs.k8s.io/controller-runtime/pkg/handler" @@ -17,12 +16,13 @@ import ( "github.com/vmware-labs/reconciler-runtime/tracker" ) -func EnqueueTracked(ctx context.Context, by client.Object, t tracker.Tracker, s *runtime.Scheme) handler.EventHandler { +func EnqueueTracked(ctx context.Context, by client.Object) handler.EventHandler { + c := RetrieveConfigOrDie(ctx) return handler.EnqueueRequestsFromMapFunc( func(a client.Object) []reconcile.Request { var requests []reconcile.Request - gvks, _, err := s.ObjectKinds(by) + gvks, _, err := c.Scheme().ObjectKinds(by) if err != nil { panic(err) } @@ -31,7 +31,7 @@ func EnqueueTracked(ctx context.Context, by client.Object, t tracker.Tracker, s gvks[0], types.NamespacedName{Namespace: a.GetNamespace(), Name: a.GetName()}, ) - for _, item := range t.Lookup(ctx, key) { + for _, item := range c.Tracker.Lookup(ctx, key) { requests = append(requests, reconcile.Request{NamespacedName: item}) } diff --git a/reconcilers/patch.go b/reconcilers/patch.go index 674ae91..f37eefa 100644 --- a/reconcilers/patch.go +++ b/reconcilers/patch.go @@ -62,5 +62,7 @@ func (p *Patch) Apply(rebase client.Object) error { if err != nil { return err } + // reset rebase to its empty value before unmarshaling into it + replaceWithEmpty(rebase) return json.Unmarshal(patchedBytes, rebase) } diff --git a/reconcilers/reconcilers.go b/reconcilers/reconcilers.go index 2b126a0..29b563b 100644 --- a/reconcilers/reconcilers.go +++ b/reconcilers/reconcilers.go @@ -27,6 +27,7 @@ import ( ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/builder" "sigs.k8s.io/controller-runtime/pkg/client" + "sigs.k8s.io/controller-runtime/pkg/cluster" "sigs.k8s.io/controller-runtime/pkg/reconcile" "sigs.k8s.io/controller-runtime/pkg/webhook" @@ -49,6 +50,21 @@ type Config struct { Log logr.Logger } +func (c Config) IsEmpty() bool { + return c == Config{} +} + +// WithConfig extends the config to access a new cluster. +func (c Config) WithCluster(cluster cluster.Cluster) Config { + return Config{ + Client: cluster.GetClient(), + APIReader: cluster.GetAPIReader(), + Recorder: cluster.GetEventRecorderFor("controller"), + Log: c.Log, + Tracker: c.Tracker, + } +} + // NewConfig creates a Config for a specific API type. Typically passed into a // reconciler. func NewConfig(mgr ctrl.Manager, apiType client.Object, syncPeriod time.Duration) Config { @@ -57,7 +73,7 @@ func NewConfig(mgr ctrl.Manager, apiType client.Object, syncPeriod time.Duration return Config{ Client: mgr.GetClient(), APIReader: mgr.GetAPIReader(), - Recorder: mgr.GetEventRecorderFor(name), + Recorder: mgr.GetEventRecorderFor("controller"), Log: log, Tracker: tracker.New(syncPeriod), } @@ -82,21 +98,24 @@ type ParentReconciler struct { // multiple SubReconcilers. Reconciler SubReconciler - Config + Config Config } func (r *ParentReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager) error { - if r.Name == "" { - r.Name = fmt.Sprintf("%sParentReconciler", typeName(r.Type)) - } - log := logr.FromContextOrDiscard(ctx). WithName(r.Name). WithValues("parentType", gvk(r.Type, r.Config.Scheme())) ctx = logr.NewContext(ctx, log) + ctx = StashConfig(ctx, r.Config) + ctx = StashParentConfig(ctx, r.Config) ctx = StashParentType(ctx, r.Type) ctx = StashCastParentType(ctx, r.Type) + + if r.Name == "" { + r.Name = fmt.Sprintf("%sParentReconciler", typeName(r.Type)) + } + bldr := ctrl.NewControllerManagedBy(mgr).For(r.Type) if err := r.Reconciler.SetupWithManager(ctx, mgr, bldr); err != nil { return err @@ -106,16 +125,21 @@ func (r *ParentReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manage func (r *ParentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Result, error) { ctx = WithStash(ctx) + + c := r.Config + log := logr.FromContextOrDiscard(ctx). WithName(r.Name). - WithValues("parentType", gvk(r.Type, r.Config.Scheme())) + WithValues("parentType", gvk(r.Type, c.Scheme())) ctx = logr.NewContext(ctx, log) + ctx = StashConfig(ctx, c) + ctx = StashParentConfig(ctx, c) ctx = StashParentType(ctx, r.Type) ctx = StashCastParentType(ctx, r.Type) originalParent := r.Type.DeepCopyObject().(client.Object) - if err := r.Get(ctx, req.NamespacedName, originalParent); err != nil { + if err := c.Get(ctx, req.NamespacedName, originalParent); err != nil { if apierrs.IsNotFound(err) { // we'll ignore not-found errors, since they can't be fixed by an immediate // requeue (we'll need to wait for a new notification), and we can get them @@ -148,13 +172,13 @@ func (r *ParentReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctr if !equality.Semantic.DeepEqual(r.status(parent), r.status(originalParent)) && parent.GetDeletionTimestamp() == nil { // update status log.Info("updating status", "diff", cmp.Diff(r.status(originalParent), r.status(parent))) - if updateErr := r.Status().Update(ctx, parent); updateErr != nil { + if updateErr := c.Status().Update(ctx, parent); updateErr != nil { log.Error(updateErr, "unable to update status") - r.Recorder.Eventf(parent, corev1.EventTypeWarning, "StatusUpdateFailed", + c.Recorder.Eventf(parent, corev1.EventTypeWarning, "StatusUpdateFailed", "Failed to update status: %v", updateErr) return ctrl.Result{}, updateErr } - r.Recorder.Eventf(parent, corev1.EventTypeNormal, "StatusUpdated", + c.Recorder.Eventf(parent, corev1.EventTypeNormal, "StatusUpdated", "Updated status") } } @@ -226,9 +250,19 @@ func (r *ParentReconciler) syncLastTransitionTime(proposed, original []metav1.Co } } +const configStashKey StashKey = "reconciler-runtime:config" +const parentConfigStashKey StashKey = "reconciler-runtime:parentConfig" const parentTypeStashKey StashKey = "reconciler-runtime:parentType" const castParentTypeStashKey StashKey = "reconciler-runtime:castParentType" +func StashConfig(ctx context.Context, config Config) context.Context { + return context.WithValue(ctx, configStashKey, config) +} + +func StashParentConfig(ctx context.Context, parentConfig Config) context.Context { + return context.WithValue(ctx, parentConfigStashKey, parentConfig) +} + func StashParentType(ctx context.Context, parentType client.Object) context.Context { return context.WithValue(ctx, parentTypeStashKey, parentType) } @@ -237,6 +271,38 @@ func StashCastParentType(ctx context.Context, currentType client.Object) context return context.WithValue(ctx, castParentTypeStashKey, currentType) } +func RetrieveConfig(ctx context.Context) Config { + value := ctx.Value(configStashKey) + if config, ok := value.(Config); ok { + return config + } + return Config{} +} + +func RetrieveConfigOrDie(ctx context.Context) Config { + config := RetrieveConfig(ctx) + if config.IsEmpty() { + panic(fmt.Errorf("config must exist on the context. Check that the context is from a ParentReconciler or WithConfig")) + } + return config +} + +func RetrieveParentConfig(ctx context.Context) Config { + value := ctx.Value(parentConfigStashKey) + if parentConfig, ok := value.(Config); ok { + return parentConfig + } + return Config{} +} + +func RetrieveParentConfigOrDie(ctx context.Context) Config { + config := RetrieveParentConfig(ctx) + if config.IsEmpty() { + panic(fmt.Errorf("parent config must exist on the context. Check that the context is from a ParentReconciler")) + } + return config +} + func RetrieveParentType(ctx context.Context) client.Object { value := ctx.Value(parentTypeStashKey) if parentType, ok := value.(client.Object); ok { @@ -288,8 +354,6 @@ type SyncReconciler struct { // func(ctx context.Context, parent client.Object) error // func(ctx context.Context, parent client.Object) (ctrl.Result, error) Sync interface{} - - Config } func (r *SyncReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, bldr *builder.Builder) error { @@ -487,8 +551,6 @@ type ChildReconciler struct { // +optional Sanitize interface{} - Config - // mutationCache holds patches received from updates to a resource made by // mutation webhooks. This cache is used to avoid unnecessary update calls // that would actually have no effect. @@ -496,15 +558,17 @@ type ChildReconciler struct { } func (r *ChildReconciler) SetupWithManager(ctx context.Context, mgr ctrl.Manager, bldr *builder.Builder) error { - if r.Name == "" { - r.Name = fmt.Sprintf("%sChildReconciler", typeName(r.ChildType)) - } + c := RetrieveConfigOrDie(ctx) log := logr.FromContextOrDiscard(ctx). WithName(r.Name). - WithValues("childType", gvk(r.ChildType, r.Config.Scheme())) + WithValues("childType", gvk(r.ChildType, c.Scheme())) ctx = logr.NewContext(ctx, log) + if r.Name == "" { + r.Name = fmt.Sprintf("%sChildReconciler", typeName(r.ChildType)) + } + if err := r.validate(ctx); err != nil { return err } @@ -625,9 +689,11 @@ func (r *ChildReconciler) validate(ctx context.Context) error { } func (r *ChildReconciler) Reconcile(ctx context.Context, parent client.Object) (ctrl.Result, error) { + c := RetrieveConfigOrDie(ctx) + log := logr.FromContextOrDiscard(ctx). WithName(r.Name). - WithValues("childType", gvk(r.ChildType, r.Config.Scheme())) + WithValues("childType", gvk(r.ChildType, c.Scheme())) ctx = logr.NewContext(ctx, log) if r.mutationCache == nil { @@ -642,8 +708,8 @@ func (r *ChildReconciler) Reconcile(ctx context.Context, parent client.Object) ( // on the parent as being not ready. apierr := err.(apierrs.APIStatus) conflicted := r.ChildType.DeepCopyObject().(client.Object) - _ = r.APIReader.Get(ctx, types.NamespacedName{Namespace: parent.GetNamespace(), Name: apierr.Status().Details.Name}, conflicted) - if metav1.IsControlledBy(conflicted, parent) { + _ = c.APIReader.Get(ctx, types.NamespacedName{Namespace: parent.GetNamespace(), Name: apierr.Status().Details.Name}, conflicted) + if r.ourChild(parent, conflicted) { // skip updating the parent's status, fail and try again return ctrl.Result{}, err } @@ -661,10 +727,12 @@ func (r *ChildReconciler) Reconcile(ctx context.Context, parent client.Object) ( func (r *ChildReconciler) reconcile(ctx context.Context, parent client.Object) (client.Object, error) { log := logr.FromContextOrDiscard(ctx) + pc := RetrieveParentConfigOrDie(ctx) + c := RetrieveConfigOrDie(ctx) actual := r.ChildType.DeepCopyObject().(client.Object) children := r.ChildListType.DeepCopyObject().(client.ObjectList) - if err := r.List(ctx, children, client.InNamespace(parent.GetNamespace())); err != nil { + if err := c.List(ctx, children, client.InNamespace(parent.GetNamespace())); err != nil { return nil, err } items := r.filterChildren(parent, children) @@ -674,12 +742,12 @@ func (r *ChildReconciler) reconcile(ctx context.Context, parent client.Object) ( // this shouldn't happen, delete everything to a clean slate for _, extra := range items { log.Info("deleting extra child", "child", namespaceName(extra)) - if err := r.Delete(ctx, extra); err != nil { - r.Recorder.Eventf(parent, corev1.EventTypeWarning, "DeleteFailed", + if err := c.Delete(ctx, extra); err != nil { + pc.Recorder.Eventf(parent, corev1.EventTypeWarning, "DeleteFailed", "Failed to delete %s %q: %v", typeName(r.ChildType), extra.GetName(), err) return nil, err } - r.Recorder.Eventf(parent, corev1.EventTypeNormal, "Deleted", + pc.Recorder.Eventf(parent, corev1.EventTypeNormal, "Deleted", "Deleted %s %q", typeName(r.ChildType), extra.GetName()) } } @@ -692,7 +760,7 @@ func (r *ChildReconciler) reconcile(ctx context.Context, parent client.Object) ( return nil, err } if desired != nil { - if err := ctrl.SetControllerReference(parent, desired, r.Scheme()); err != nil { + if err := ctrl.SetControllerReference(parent, desired, c.Scheme()); err != nil { return nil, err } if !r.ourChild(parent, desired) { @@ -704,13 +772,13 @@ func (r *ChildReconciler) reconcile(ctx context.Context, parent client.Object) ( if desired == nil { if !actual.GetCreationTimestamp().Time.IsZero() { log.Info("deleting unwanted child", "child", namespaceName(actual)) - if err := r.Delete(ctx, actual); err != nil { + if err := c.Delete(ctx, actual); err != nil { log.Error(err, "unable to delete unwanted child", "child", namespaceName(actual)) - r.Recorder.Eventf(parent, corev1.EventTypeWarning, "DeleteFailed", + pc.Recorder.Eventf(parent, corev1.EventTypeWarning, "DeleteFailed", "Failed to delete %s %q: %v", typeName(r.ChildType), actual.GetName(), err) return nil, err } - r.Recorder.Eventf(parent, corev1.EventTypeNormal, "Deleted", + pc.Recorder.Eventf(parent, corev1.EventTypeNormal, "Deleted", "Deleted %s %q", typeName(r.ChildType), actual.GetName()) } return nil, nil @@ -719,13 +787,13 @@ func (r *ChildReconciler) reconcile(ctx context.Context, parent client.Object) ( // create child if it doesn't exist if actual.GetName() == "" { log.Info("creating child", "child", r.sanitize(desired)) - if err := r.Create(ctx, desired); err != nil { + if err := c.Create(ctx, desired); err != nil { log.Error(err, "unable to create child", "child", namespaceName(desired)) - r.Recorder.Eventf(parent, corev1.EventTypeWarning, "CreationFailed", + pc.Recorder.Eventf(parent, corev1.EventTypeWarning, "CreationFailed", "Failed to create %s %q: %v", typeName(r.ChildType), desired.GetName(), err) return nil, err } - r.Recorder.Eventf(parent, corev1.EventTypeNormal, "Created", + pc.Recorder.Eventf(parent, corev1.EventTypeNormal, "Created", "Created %s %q", typeName(r.ChildType), desired.GetName()) return desired, nil } @@ -753,9 +821,9 @@ func (r *ChildReconciler) reconcile(ctx context.Context, parent client.Object) ( current := actual.DeepCopyObject().(client.Object) r.mergeBeforeUpdate(current, desiredPatched) log.Info("updating child", "diff", cmp.Diff(r.sanitize(actual), r.sanitize(current))) - if err := r.Update(ctx, current); err != nil { + if err := c.Update(ctx, current); err != nil { log.Error(err, "unable to update child", "child", namespaceName(current)) - r.Recorder.Eventf(parent, corev1.EventTypeWarning, "UpdateFailed", + pc.Recorder.Eventf(parent, corev1.EventTypeWarning, "UpdateFailed", "Failed to update %s %q: %v", typeName(r.ChildType), current.GetName(), err) return nil, err } @@ -772,7 +840,7 @@ func (r *ChildReconciler) reconcile(ctx context.Context, parent client.Object) ( } } log.Info("updated child") - r.Recorder.Eventf(parent, corev1.EventTypeNormal, "Updated", + pc.Recorder.Eventf(parent, corev1.EventTypeNormal, "Updated", "Updated %s %q", typeName(r.ChildType), current.GetName()) return current, nil @@ -955,15 +1023,15 @@ type CastParent struct { } func (r *CastParent) SetupWithManager(ctx context.Context, mgr ctrl.Manager, bldr *builder.Builder) error { - if r.Name == "" { - r.Name = fmt.Sprintf("%sCastParent", typeName(r.Type)) - } - log := logr.FromContextOrDiscard(ctx). WithName(r.Name). WithValues("castParentType", typeName(r.Type)) ctx = logr.NewContext(ctx, log) + if r.Name == "" { + r.Name = fmt.Sprintf("%sCastParent", typeName(r.Type)) + } + if err := r.validate(ctx); err != nil { return err } @@ -1019,7 +1087,7 @@ func (r *CastParent) cast(ctx context.Context, parent client.Object) (context.Co if err != nil { return nil, nil, err } - castParent := r.Type.DeepCopyObject().(client.Object) + castParent := newEmpty(r.Type).(client.Object) err = json.Unmarshal(data, castParent) if err != nil { return nil, nil, err @@ -1028,6 +1096,49 @@ func (r *CastParent) cast(ctx context.Context, parent client.Object) (context.Co return ctx, castParent, nil } +// WithConfig injects the provided config into the reconcilers nested under it. For example, the +// client can be swapped to use a service account with different permissions, or to target an +// entirely different cluster. +// +// The specified config can be accessed with `RetrieveConfig(ctx)`, the original config used to +// load the parent resource can be accessed with `RetrieveParentConfig(ctx)`. +type WithConfig struct { + // Config to use for this portion of the reconciler hierarchy + Config Config + + // Reconciler is called for each reconciler request with the parent + // resource being reconciled. Typically a Sequence is used to compose + // multiple SubReconcilers. + Reconciler SubReconciler +} + +func (r *WithConfig) SetupWithManager(ctx context.Context, mgr ctrl.Manager, bldr *builder.Builder) error { + if err := r.validate(ctx); err != nil { + return err + } + ctx = StashConfig(ctx, r.Config) + return r.Reconciler.SetupWithManager(ctx, mgr, bldr) +} + +func (r *WithConfig) validate(ctx context.Context) error { + // validate Config value + if r.Config.IsEmpty() { + return fmt.Errorf("Config must be defined") + } + + // validate Reconciler value + if r.Reconciler == nil { + return fmt.Errorf("Reconciler must be defined") + } + + return nil +} + +func (r *WithConfig) Reconcile(ctx context.Context, parent client.Object) (ctrl.Result, error) { + ctx = StashConfig(ctx, r.Config) + return r.Reconciler.Reconcile(ctx, parent) +} + func typeName(i interface{}) string { t := reflect.TypeOf(i) // TODO do we need this? @@ -1063,3 +1174,15 @@ func MergeMaps(maps ...map[string]string) map[string]string { } return out } + +// replaceWithEmpty overwrite the underlying value with it's empty value +func replaceWithEmpty(x interface{}) { + v := reflect.ValueOf(x).Elem() + v.Set(reflect.Zero(v.Type())) +} + +// newEmpty returns a new empty value of the same underlying type, preserving the existing value +func newEmpty(x interface{}) interface{} { + t := reflect.TypeOf(x).Elem() + return reflect.New(t).Interface() +} diff --git a/reconcilers/reconcilers_test.go b/reconcilers/reconcilers_test.go index a0eceea..c7964b5 100644 --- a/reconcilers/reconcilers_test.go +++ b/reconcilers/reconcilers_test.go @@ -19,6 +19,7 @@ import ( "github.com/vmware-labs/reconciler-runtime/internal/resources/dies" "github.com/vmware-labs/reconciler-runtime/reconcilers" rtesting "github.com/vmware-labs/reconciler-runtime/testing" + "github.com/vmware-labs/reconciler-runtime/tracker" appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/api/equality" @@ -58,7 +59,6 @@ func TestParentReconcilerWithNoStatus(t *testing.T) { Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResourceNoStatus) error { return nil }, @@ -102,7 +102,6 @@ func TestParentReconciler(t *testing.T) { Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) error { t.Error("should not be called") return nil @@ -121,7 +120,6 @@ func TestParentReconciler(t *testing.T) { Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) error { t.Error("should not be called") return nil @@ -143,7 +141,6 @@ func TestParentReconciler(t *testing.T) { Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) error { t.Error("should not be called") return nil @@ -161,7 +158,6 @@ func TestParentReconciler(t *testing.T) { Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) error { if expected, actual := "ran", parent.Spec.Fields["Defaulter"]; expected != actual { t.Errorf("unexpected default value, actually = %v, expected = %v", expected, actual) @@ -182,7 +178,6 @@ func TestParentReconciler(t *testing.T) { Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) error { expected := []metav1.Condition{ {Type: apis.ConditionReady, Status: metav1.ConditionUnknown, Reason: "Initializing"}, @@ -211,7 +206,6 @@ func TestParentReconciler(t *testing.T) { Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) error { if parent.Status.Fields == nil { parent.Status.Fields = map[string]string{} @@ -240,7 +234,6 @@ func TestParentReconciler(t *testing.T) { Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) error { return fmt.Errorf("reconciler error") }, @@ -262,7 +255,6 @@ func TestParentReconciler(t *testing.T) { Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) error { if parent.Status.Fields == nil { parent.Status.Fields = map[string]string{} @@ -292,7 +284,6 @@ func TestParentReconciler(t *testing.T) { Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) error { var key reconcilers.StashKey = "foo" // StashValue will panic if the context is not setup correctly @@ -302,6 +293,48 @@ func TestParentReconciler(t *testing.T) { } }, }, + }, { + Name: "context has config", + Key: testKey, + GivenObjects: []client.Object{ + resource, + }, + Metadata: map[string]interface{}{ + "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { + return &reconcilers.SyncReconciler{ + Sync: func(ctx context.Context, parent *resources.TestResource) error { + if config := reconcilers.RetrieveConfig(ctx); config != c { + t.Errorf("expected config in context, found %#v", config) + } + if parentConfig := reconcilers.RetrieveParentConfig(ctx); parentConfig != c { + t.Errorf("expected parent config in context, found %#v", parentConfig) + } + return nil + }, + } + }, + }, + }, { + Name: "context has parent type", + Key: testKey, + GivenObjects: []client.Object{ + resource, + }, + Metadata: map[string]interface{}{ + "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { + return &reconcilers.SyncReconciler{ + Sync: func(ctx context.Context, parent *resources.TestResource) error { + if parentType, ok := reconcilers.RetrieveParentType(ctx).(*resources.TestResource); !ok { + t.Errorf("expected parent type not in context, found %#v", parentType) + } + if castParentType, ok := reconcilers.RetrieveCastParentType(ctx).(*resources.TestResource); !ok { + t.Errorf("expected cast parent type not in context, found %#v", castParentType) + } + return nil + }, + } + }, + }, }} rts.Test(t, scheme, func(t *testing.T, rtc *rtesting.ReconcilerTestCase, c reconcilers.Config) reconcile.Reconciler { @@ -338,7 +371,6 @@ func TestSyncReconciler(t *testing.T) { Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) error { return nil }, @@ -351,7 +383,6 @@ func TestSyncReconciler(t *testing.T) { Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) error { return fmt.Errorf("syncreconciler error") }, @@ -365,8 +396,7 @@ func TestSyncReconciler(t *testing.T) { Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return &reconcilers.SyncReconciler{ - Config: c, - Sync: nil, + Sync: nil, } }, }, @@ -377,7 +407,6 @@ func TestSyncReconciler(t *testing.T) { Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent string) error { return nil }, @@ -470,8 +499,6 @@ func TestChildReconciler(t *testing.T) { SemanticEquals: func(r1, r2 *corev1.ConfigMap) bool { return equality.Semantic.DeepEqual(r1.Data, r2.Data) }, - - Config: c, } } @@ -931,7 +958,6 @@ func TestSequence(t *testing.T) { "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return reconcilers.Sequence{ &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) error { return fmt.Errorf("reconciler error") }, @@ -946,7 +972,6 @@ func TestSequence(t *testing.T) { Metadata: map[string]interface{}{ "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) (ctrl.Result, error) { return ctrl.Result{Requeue: true}, nil }, @@ -961,7 +986,6 @@ func TestSequence(t *testing.T) { "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return reconcilers.Sequence{ &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) (ctrl.Result, error) { return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil }, @@ -977,7 +1001,6 @@ func TestSequence(t *testing.T) { "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return reconcilers.Sequence{ &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) (ctrl.Result, error) { return ctrl.Result{Requeue: true}, fmt.Errorf("test error") }, @@ -994,13 +1017,11 @@ func TestSequence(t *testing.T) { "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return reconcilers.Sequence{ &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) (ctrl.Result, error) { return ctrl.Result{Requeue: true}, nil }, }, &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) (ctrl.Result, error) { return ctrl.Result{}, nil }, @@ -1016,13 +1037,11 @@ func TestSequence(t *testing.T) { "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return reconcilers.Sequence{ &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) (ctrl.Result, error) { return ctrl.Result{}, nil }, }, &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) (ctrl.Result, error) { return ctrl.Result{Requeue: true}, nil }, @@ -1038,13 +1057,11 @@ func TestSequence(t *testing.T) { "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return reconcilers.Sequence{ &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) (ctrl.Result, error) { return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil }, }, &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) (ctrl.Result, error) { return ctrl.Result{}, nil }, @@ -1060,13 +1077,11 @@ func TestSequence(t *testing.T) { "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return reconcilers.Sequence{ &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) (ctrl.Result, error) { return ctrl.Result{}, nil }, }, &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) (ctrl.Result, error) { return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil }, @@ -1082,13 +1097,11 @@ func TestSequence(t *testing.T) { "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return reconcilers.Sequence{ &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) (ctrl.Result, error) { return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil }, }, &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) (ctrl.Result, error) { return ctrl.Result{Requeue: true}, nil }, @@ -1104,13 +1117,11 @@ func TestSequence(t *testing.T) { "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return reconcilers.Sequence{ &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) (ctrl.Result, error) { return ctrl.Result{Requeue: true}, nil }, }, &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) (ctrl.Result, error) { return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil }, @@ -1126,13 +1137,11 @@ func TestSequence(t *testing.T) { "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return reconcilers.Sequence{ &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) (ctrl.Result, error) { return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil }, }, &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) (ctrl.Result, error) { return ctrl.Result{RequeueAfter: 2 * time.Minute}, nil }, @@ -1148,13 +1157,11 @@ func TestSequence(t *testing.T) { "SubReconciler": func(t *testing.T, c reconcilers.Config) reconcilers.SubReconciler { return reconcilers.Sequence{ &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) (ctrl.Result, error) { return ctrl.Result{RequeueAfter: 2 * time.Minute}, nil }, }, &reconcilers.SyncReconciler{ - Config: c, Sync: func(ctx context.Context, parent *resources.TestResource) (ctrl.Result, error) { return ctrl.Result{RequeueAfter: 1 * time.Minute}, nil }, @@ -1209,7 +1216,6 @@ func TestCastParent(t *testing.T) { parent.Spec.Template.Spec.Containers[0].Name) return nil }, - Config: c, }, } }, @@ -1240,7 +1246,6 @@ func TestCastParent(t *testing.T) { parent.Spec.Paused = true return nil }, - Config: c, }, } }, @@ -1256,7 +1261,6 @@ func TestCastParent(t *testing.T) { Sync: func(ctx context.Context, parent *appsv1.Deployment) (ctrl.Result, error) { return ctrl.Result{Requeue: true}, nil }, - Config: c, }, } }, @@ -1273,7 +1277,6 @@ func TestCastParent(t *testing.T) { Sync: func(ctx context.Context, parent *appsv1.Deployment) error { return fmt.Errorf("subreconciler error") }, - Config: c, }, } }, @@ -1297,7 +1300,6 @@ func TestCastParent(t *testing.T) { Sync: func(ctx context.Context, parent *resources.TestResource) error { return nil }, - Config: c, }, } }, @@ -1321,7 +1323,6 @@ func TestCastParent(t *testing.T) { Sync: func(ctx context.Context, parent *resources.TestResource) error { return nil }, - Config: c, }, } }, @@ -1342,7 +1343,6 @@ func TestCastParent(t *testing.T) { c.Recorder.Event(resource, corev1.EventTypeNormal, "Test", parent.Name) return nil }, - Config: c, }, } }, @@ -1363,7 +1363,6 @@ func TestCastParent(t *testing.T) { c.Recorder.Event(resource, corev1.EventTypeNormal, "Test", parent.Name) return nil }, - Config: c, }, } }, @@ -1375,3 +1374,58 @@ func TestCastParent(t *testing.T) { return rtc.Metadata["SubReconciler"].(func(*testing.T, reconcilers.Config) reconcilers.SubReconciler)(t, c) }) } + +func TestWithConfig(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) + d.CreationTimestamp(metav1.NewTime(time.UnixMilli(1000))) + }) + + rts := rtesting.SubReconcilerTestSuite{{ + Name: "with config", + Parent: resource, + Metadata: map[string]interface{}{ + "SubReconciler": func(t *testing.T, pc reconcilers.Config) reconcilers.SubReconciler { + c := reconcilers.Config{ + Tracker: tracker.New(0), + } + + return &reconcilers.WithConfig{ + Config: c, + Reconciler: &reconcilers.SyncReconciler{ + Sync: func(ctx context.Context, parent *resources.TestResource) error { + ac := reconcilers.RetrieveConfig(ctx) + apc := reconcilers.RetrieveParentConfig(ctx) + + if ac != c { + t.Errorf("unexpected config") + } + if apc != pc { + t.Errorf("unexpected parent config") + } + + pc.Recorder.Event(resource, corev1.EventTypeNormal, "AllGood", "") + + return nil + }, + }, + } + }, + }, + ExpectEvents: []rtesting.Event{ + rtesting.NewEvent(resource, scheme, corev1.EventTypeNormal, "AllGood", ""), + }, + }} + + rts.Test(t, scheme, func(t *testing.T, rtc *rtesting.SubReconcilerTestCase, c reconcilers.Config) reconcilers.SubReconciler { + return rtc.Metadata["SubReconciler"].(func(*testing.T, reconcilers.Config) reconcilers.SubReconciler)(t, c) + }) +} diff --git a/reconcilers/reconcilers_validate_test.go b/reconcilers/reconcilers_validate_test.go index d6db56d..b4a802d 100644 --- a/reconcilers/reconcilers_validate_test.go +++ b/reconcilers/reconcilers_validate_test.go @@ -9,6 +9,7 @@ import ( "context" "testing" + "github.com/vmware-labs/reconciler-runtime/tracker" corev1 "k8s.io/api/core/v1" ctrl "sigs.k8s.io/controller-runtime" "sigs.k8s.io/controller-runtime/pkg/client" @@ -735,3 +736,66 @@ func TestCastParent_validate(t *testing.T) { }) } } + +func TestWithConfig_validate(t *testing.T) { + config := Config{ + Tracker: tracker.New(0), + } + + tests := []struct { + name string + parent client.Object + reconciler *WithConfig + shouldErr string + }{ + { + name: "empty", + parent: &corev1.ConfigMap{}, + reconciler: &WithConfig{}, + shouldErr: "Config must be defined", + }, + { + name: "valid", + parent: &corev1.ConfigMap{}, + reconciler: &WithConfig{ + Config: config, + Reconciler: &SyncReconciler{ + Sync: func(ctx context.Context, parent *corev1.Secret) error { + return nil + }, + }, + }, + }, + { + name: "missing type", + parent: &corev1.ConfigMap{}, + reconciler: &WithConfig{ + Reconciler: &SyncReconciler{ + Sync: func(ctx context.Context, parent *corev1.Secret) error { + return nil + }, + }, + }, + shouldErr: "Config must be defined", + }, + { + name: "missing reconciler", + parent: &corev1.ConfigMap{}, + reconciler: &WithConfig{ + Config: config, + Reconciler: nil, + }, + shouldErr: "Reconciler must be defined", + }, + } + + for _, c := range tests { + t.Run(c.name, func(t *testing.T) { + ctx := context.TODO() + 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) + } + }) + } +} diff --git a/testing/reconciler.go b/testing/reconciler.go index 7dc8075..04bcceb 100644 --- a/testing/reconciler.go +++ b/testing/reconciler.go @@ -164,7 +164,7 @@ func (tc *ReconcilerTestCase) Run(t *testing.T, scheme *runtime.Scheme, factory }) if (err != nil) != tc.ShouldErr { - t.Errorf("Reconcile() error = %v, ExpectErr %v", err, tc.ShouldErr) + t.Errorf("Reconcile() error = %v, ShouldErr %v", err, tc.ShouldErr) } if err == nil { // result is only significant if there wasn't an error diff --git a/testing/subreconciler.go b/testing/subreconciler.go index 4c06999..d794808 100644 --- a/testing/subreconciler.go +++ b/testing/subreconciler.go @@ -132,13 +132,14 @@ func (tc *SubReconcilerTestCase) Run(t *testing.T, scheme *runtime.Scheme, facto scheme: scheme, } log := logrtesting.NewTestLogger(t) - c := factory(t, tc, reconcilers.Config{ + c := reconcilers.Config{ Client: clientWrapper, APIReader: apiReader, Tracker: tracker, Recorder: recorder, Log: log, - }) + } + r := factory(t, tc, c) if tc.CleanUp != nil { defer func() { @@ -161,6 +162,9 @@ func (tc *SubReconcilerTestCase) Run(t *testing.T, scheme *runtime.Scheme, facto reconcilers.StashValue(ctx, k, v) } + ctx = reconcilers.StashConfig(ctx, c) + ctx = reconcilers.StashParentConfig(ctx, c) + parent := tc.Parent.DeepCopyObject().(client.Object) ctx = reconcilers.StashParentType(ctx, parent.DeepCopyObject().(client.Object)) ctx = reconcilers.StashCastParentType(ctx, parent.DeepCopyObject().(client.Object)) @@ -175,11 +179,11 @@ func (tc *SubReconcilerTestCase) Run(t *testing.T, scheme *runtime.Scheme, facto }() } - return c.Reconcile(ctx, parent) + return r.Reconcile(ctx, parent) }(ctx, parent) if (err != nil) != tc.ShouldErr { - t.Errorf("Reconcile() error = %v, ExpectErr %v", err, tc.ShouldErr) + t.Errorf("Reconcile() error = %v, ShouldErr %v", err, tc.ShouldErr) } if err == nil { // result is only significant if there wasn't an error