AdviceReconciler

[scothis]

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

[scothis]

cc @squeedee

[codecov]

Codecov Report

Attention: Patch coverage is 43.61702% with 53 lines in your changes are missing coverage. Please review.

Project coverage is 60.43%. Comparing base (a63803b) to head (daa0cac).
Report is 12 commits behind head on main.

Files	Patch %	Lines
reconcilers/advice.go	0.00%	44 Missing ⚠️
reconcilers/null.go	0.00%	4 Missing ⚠️
reconcilers/aggregate.go	83.33%	2 Missing and 1 partial ⚠️
reconcilers/resource.go	90.00%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #502      +/-   ##
==========================================
- Coverage   61.13%   60.43%   -0.70%     
==========================================
  Files          28       30       +2     
  Lines        2583     2669      +86     
==========================================
+ Hits         1579     1613      +34     
- Misses        917      967      +50     
- Partials       87       89       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[mamachanko]

[question]: Is this for testing?

[scothis]

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.

[squeedee]

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]

@scothis I was trying AdviceReconciler and identified a small bug, line 312 in reconcilers/resource.go should be

r.AfterReconcile(ctx, req, result, nil)

[scothis]

@scothis I was trying AdviceReconciler and identified a small bug, line 312 in reconcilers/resource.go should be

r.AfterReconcile(ctx, req, result, nil)

@yharish991 good catch, I refactored a bit to make sure this can't happen in the future.

[squeedee]

@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.