Improve error context for controller startup failures#2341
Improve error context for controller startup failures#2341Kunal1522 wants to merge 2 commits intoopenkruise:masterfrom
Conversation
Add error wrapping with context to controller initialization errors to improve debugging of startup failures. This helps operators quickly identify which controller failed during initialization without grepping through code. Changes: - Add error context to controller.New() failures - Add error context to Watch() setup failures for all resource types - Add error context to webhook controller initialization This follows best practices from kubernetes/kubernetes and other CNCF projects which wrap startup errors but not runtime reconciliation errors (which are already logged with context by defer statements). Signed-off-by: Kunal1522 <kunalkashyap8024@gmail.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR enhances error messages during controller and webhook initialization to include controller-specific context, making startup failures easier to diagnose.
Changes:
- Wrap
webhookcontroller.Newinitialization errors inpkg/webhook/server.gowith contextualfmt.Errorf("failed to create webhook controller: %w", err). - Wrap
controller.NewandWatchsetup failures inpkg/controller/cloneset/cloneset_controller.gowith descriptive, controller- and resource-specific error messages. - Wrap
controller.Newand selectedWatchsetup failures inpkg/controller/resourcedistribution/resourcedistribution_controller.gowith more informative error messages.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| pkg/webhook/server.go | Adds contextual wrapping around webhook controller creation errors to clarify which component failed at startup. |
| pkg/controller/resourcedistribution/resourcedistribution_controller.go | Adds contextual error wrapping for controller creation and some watch registrations to improve diagnostics for ResourceDistribution startup failures. |
| pkg/controller/cloneset/cloneset_controller.go | Adds contextual error messages for CloneSet controller creation and all relevant watch registrations, aligning startup errors with the requested format. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return fmt.Errorf("failed to create resourcedistribution controller: %w", err) | ||
| } |
There was a problem hiding this comment.
The PR description and linked issue state that Watch() setup failures should be wrapped with controller-specific context for all resource types, but in this controller only the Secret and ConfigMap watches now add context while the ResourceDistribution and Namespace watches above still return the bare err. This leaves some startup failures for this controller still emitting generic errors, which partially defeats the goal of making it obvious which watch failed; consider wrapping the earlier c.Watch error returns in this function with contextual fmt.Errorf("...: %w", err) messages as well.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2341 +/- ##
==========================================
- Coverage 48.72% 48.67% -0.06%
==========================================
Files 324 324
Lines 27920 27920
==========================================
- Hits 13604 13590 -14
- Misses 12779 12791 +12
- Partials 1537 1539 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Address Copilot review comment: wrap ResourceDistribution and Namespace watch errors with context messages for better debugging. Signed-off-by: Kunal1522 <kunalkashyap8024@gmail.com>
c439f91 to
0d3dc86
Compare
Fixes #2340
Add error wrapping with context to controller initialization errors to improve debugging of startup failures. This helps operators quickly identify which controller failed during initialization without grepping through code.
Changes: