fix(kstatus): fine-grained context options for waiting#31735
fix(kstatus): fine-grained context options for waiting#31735
Conversation
720ce1b to
e63e95a
Compare
banjoh
left a comment
There was a problem hiding this comment.
Change LGTM. Thanks for the extensive tests
e63e95a to
6ffc17f
Compare
There was a problem hiding this comment.
Pull request overview
This PR introduces fine-grained context control for different waiting methods in Helm's kstatus integration. It enables consumers like Flux to pass different contexts to specific wait methods (e.g., WatchUntilReady, Wait, WaitWithJobs, WaitForDelete) without affecting others. This is critical for Flux to avoid canceling Helm hooks that may be running important operations like database migrations when only the general wait operation should be cancelled.
Changes:
- Added method-specific context fields to
statusWaiterstruct andwaitOptionstype - Introduced four new
WaitOptionfunctions to set method-specific contexts - Modified context resolution logic to use method-specific context with fallback to general context
- Added comprehensive unit tests covering option setting, context cancellation, fallback behavior, and override scenarios
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| pkg/kube/statuswait.go | Added method-specific context fields to statusWaiter and updated contextWithTimeout to support method-specific contexts with fallback |
| pkg/kube/options.go | Added four new WaitOption functions and corresponding fields in waitOptions for method-specific contexts |
| pkg/kube/client.go | Updated newStatusWatcher to pass method-specific contexts from options to statusWaiter |
| pkg/kube/statuswait_test.go | Added comprehensive tests for option functions, context cancellation, fallback, and override behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
6ffc17f to
748887e
Compare
Signed-off-by: Matheus Pimenta <matheuscscp@gmail.com>
748887e to
b0b35f1
Compare
What this PR does / why we need it / Special notes for your reviewer:
This is a critical fix for the feature previously introduced here:
https://github.com/helm/helm/pull/31435/files (see file
pkg/kube/statuswait.go)Basically, Flux cannot pass the custom context to all the waiter methods. Specifically for
WatchUntilReady, we can never pass the custom context to it because it means cancelling Helm hooks, which can be very bad, as users use hooks to run e.g. database migrations. Please see this issue for more context: fluxcd/helm-controller#1380 (comment)If applicable:
docs neededlabel should be applied if so)