AdviceReconciler #502
AdviceReconciler #502
Conversation
|
cc @squeedee |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #502 +/- ##
==========================================
+ Coverage 59.73% 60.46% +0.73%
==========================================
Files 31 32 +1
Lines 2779 2861 +82
==========================================
+ Hits 1660 1730 +70
- Misses 1027 1039 +12
Partials 92 92 ☔ View full report in Codecov by Sentry. |
reconcilers/null.go
Outdated
| _ SubReconciler[client.Object] = (*NullReconciler[client.Object])(nil) | ||
| ) | ||
|
|
||
| // NullReconciler does nothing |
There was a problem hiding this comment.
[question]: Is this for testing?
There was a problem hiding this comment.
Not just for testing. Since the Reconciler field is required, but Around can choose not to invoke a reconciler it's an easy way to define something that implements the interface, but does nothing.
|
This is really great. I started at the Top level reconciler so it didn't occur to me that composable subreconcilers will work for everything else.. I guess a delegate with the same interface for the top level? |
@yharish991 good catch, I refactored a bit to make sure this can't happen in the future. |
|
@scothis @yharish991 has been using this directly. We're finding this quite useful so far. Do you plan on moving it from "sketch" status? Personally. I think it's a good approach and will serve users well. |
scothis
force-pushed
the
aspect-oriented-reconciler
branch
2 times, most recently
from
0317a8e to
efa6d61
Compare
|
Renamed |
README.md
Outdated
| @@ -618,6 +647,10 @@ func SyncExternalState() *reconcilers.SubReconciler[*resources.MyResource] { | |||
| } | |||
| ``` | |||
|
|
|||
| #### Null | |||
|
|
|||
| [`Null`](https://pkg.go.dev/reconciler.io/runtime/reconcilers#Null) implements the SubReconciler interface, while performing no action. It can be used where a sub reconciler is required, but no behavior is necessary. | |||
There was a problem hiding this comment.
[question]: Is there a conceivable example like we've got for the others?
There was a problem hiding this comment.
Dropping null support for now. It's not essential to this PR.
Add hooks to ResourceReconciler, AggregateReconciler and AdmissionWebhookAdapter before and after the reconciler is called. The Advice SubReconciler can wrap any other SubReconciler to provide similar hooks. Signed-off-by: Scott Andrews <[email protected]>
Co-authored-by: Max Brauer <[email protected]> Signed-off-by: Scott Andrews <[email protected]>
scothis
force-pushed
the
aspect-oriented-reconciler
branch
from
9ffef94 to
ec7561c
Compare
A sketch of a potential method for defining advice for a SubReconciler as a SubReconciler. It can be composed manually into existing SubReconciler hierarchies. Other composition strategies could be explored in the future.
This does not address advice for ResourceReconciler, AggregateReconciler, or AdmissionWebhookAdapter.
Refs #500