reconciler-runtime
reconciler-runtime copied to clipboard
Automatically watch tracked resources
Both the ParentReconciler and ChildReconcilers properly watch resources for when the resource changes, the resource is enqueued for reconciliation. This doesn't happen automatically with the SyncReconciler when manually looking up and tracking a resource. Currently the watches are configured when the reconciler is setup, which requires knowledge of which resources to watch.
Duck types enable consuming a resource without requiring advanced knowledge of the concrete resource in advance, often we won't know the concrete resource until see an ObjectReference that has the address coordinates for the concrete type. We should enhance (or wrap) the tracker so that when a reference is tracked, there is a corresponding informer for the gvk started so we can enqueue the object.
A few thoughts that may be useful, or a distraction:
- the version of tracked resources is irrelevant as versions are a projection of an underlying resource, yet informers must be configured for a specific version
- metadata only informers contain everything we need to track a resource
- informers should be started lazily, we don't need to wait for the cache to sync
- gvks that are no longer tracked should have their informers stopped and cache released
- reusing existing informers may be useful to reduce load, but may make cleanup harder
- RBAC issues accessing a new gvk should be handled gracefully. The RBAC policies may change at runtime granting or revoking access
- new apiGroups, versions and kinds can be added and removed at runtime
- Knative is able to dynamically spin up informers for references resources, so there's precedent that this is possible (although controller-runtime may throw a wrench at us)
An example of SyncReconciler that has foreknowledge of the resource that are being tracked:
https://github.com/projectriff/system/blob/1fcdb7a090565d6750f9284a176eb00a3fe14663/pkg/controllers/core/deployer_reconciler.go#L66-L148
The Setup method should be able to go away if this issue is resolved:
Setup: func(mgr reconcilers.Manager, bldr *reconcilers.Builder) error {
bldr.Watches(&source.Kind{Type: &buildv1alpha1.Application{}}, reconcilers.EnqueueTracked(&buildv1alpha1.Application{}, c.Tracker, c.Scheme))
bldr.Watches(&source.Kind{Type: &buildv1alpha1.Container{}}, reconcilers.EnqueueTracked(&buildv1alpha1.Container{}, c.Tracker, c.Scheme))
bldr.Watches(&source.Kind{Type: &buildv1alpha1.Function{}}, reconcilers.EnqueueTracked(&buildv1alpha1.Function{}, c.Tracker, c.Scheme))
return nil
},
- gvks that are no longer tracked should have their informers stopped and cache released
It looks like controller-runtime's Controller interface doesn't allow watches to be removed. How hard a requirement is this?
- gvks that are no longer tracked should have their informers stopped and cache released
Also, a different issue, I'm wondering how often in practice the tracker Lookup method is called. If it's infrequently or never called, then we may need to add time-based cleanup of expired trackers so they don't "pin" the corresponding informers.
It seems that Lookup is called by the informer callback, so I guess that's sufficient because if a watch is causing overhead, then it's informer callback will be driven, any expired trackers will be deleted, and the corresponding informers will no longer be pinned.
We don't need to get hung up on removing informers on the first pass. Let's get it working and then figure out what we need to optimize. If we have to push support upstream or create a different informer stack we can cross that bridge when we get there.