Skip to content

Add an optional ForOptions field to the ResourceReconciler#673

Closed
annismckenzie wants to merge 1 commit intoreconcilerio:mainfrom
alfatraining:0-allow-passing-for-options
Closed

Add an optional ForOptions field to the ResourceReconciler#673
annismckenzie wants to merge 1 commit intoreconcilerio:mainfrom
alfatraining:0-allow-passing-for-options

Conversation

@annismckenzie
Copy link
Contributor

@annismckenzie annismckenzie commented Jan 20, 2026

In one of our projects we've used the predicates functionality that the controller runtime provides. This was easy to add but might cause issues if we keep finding things to tweak. Unfortunately, I didn't find another, less intrusive way to add this feature. There are tests missing which I'd add if you're willing to accept this PR.

@codecov
Copy link

codecov bot commented Jan 20, 2026

Codecov Report

❌ Patch coverage is 12.50000% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 59.88%. Comparing base (eaa676f) to head (6b9408e).
⚠️ Report is 4 commits behind head on main.

Files with missing lines Patch % Lines
reconcilers/resource.go 12.50% 6 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #673      +/-   ##
==========================================
- Coverage   59.96%   59.88%   -0.08%     
==========================================
  Files          39       39              
  Lines        3559     3565       +6     
==========================================
+ Hits         2134     2135       +1     
- Misses       1315     1319       +4     
- Partials      110      111       +1     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Daniel Lohse <info@asapdesign.de>
@annismckenzie annismckenzie force-pushed the 0-allow-passing-for-options branch from a6e1958 to 6b9408e Compare January 20, 2026 16:35
@scothis
Copy link
Member

scothis commented Jan 21, 2026

Thanks for opening. I agree these options are worth exposing. A few questions:

  1. Using a func is only really useful if the options are expected to vary run to run. I'd expect them to be stable, but maybe you have a use case?
  2. When would an error be returned?
  3. Should this pattern also be applied to the AggregateReconciler?

@scothis
Copy link
Member

scothis commented Feb 4, 2026

Implemented in #676. Thanks for the proposal @annismckenzie

@scothis scothis closed this Feb 4, 2026
@annismckenzie
Copy link
Contributor Author

So sorry, I just saw your comments above – your version will work nicely, thank you! 🤝

@annismckenzie annismckenzie deleted the 0-allow-passing-for-options branch February 4, 2026 19:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants