fix: infinite retry loop for too large request(#28703) - #28704
Draft
Julian-Chu wants to merge 8 commits into
Draft
fix: infinite retry loop for too large request(#28703)#28704Julian-Chu wants to merge 8 commits into
Julian-Chu wants to merge 8 commits into
Conversation
… limit Signed-off-by: Julian-Chu <yulang.chu@gmail.com>
…te on patch size limit When setOperationState's patch is rejected as too large, write a minimal OperationState (phase=Error) instead of an application condition. This surfaces the failure in the UI as a terminal sync error rather than a status condition, and avoids re-sending the same oversized payload. Also: - Fix logCtx.WithError passing `err` (out of scope) instead of nonRetryableError - Return early after json.Marshal failure to avoid sending a nil patch body - Add nil guard to isOperationStatePayloadTooLargeError - Add TestSetOperationStateTooLargeRequest for nil/non-nil prior OperationState Signed-off-by: Julian-Chu <yulang.chu@gmail.com>
❗ Preview Environment stop on Bunnyshell failedSee: Environment Details | Pipeline Logs Available commands (reply to this comment):
|
…request Signed-off-by: Julian-Chu <yulang.chu@gmail.com>
Bundle ReportBundle size has no change ✅ |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #28704 +/- ##
==========================================
+ Coverage 65.59% 65.63% +0.03%
==========================================
Files 427 427
Lines 60565 60613 +48
==========================================
+ Hits 39728 39781 +53
+ Misses 17204 17198 -6
- Partials 3633 3634 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
agaudreault
requested changes
Jul 22, 2026
| if err != nil { | ||
| logCtx.WithError(err).Error("Error marshaling fallback patch") | ||
| return | ||
| } |
Member
There was a problem hiding this comment.
The overall logic looks good and follow what is done in persistAppStatus. But the Patch here should decide to
- Call Terminate
- Clear the Operation
- Set phase to Error + message
- Clear the syncResult (probably just clearing the successful one is enough)
When persisting operation state fails because the patch exceeds the Kubernetes/etcd size limit, request termination first (mirroring the "Terminate" action) instead of jumping straight to an error, ensuring in-flight hooks get cleaned up properly. Key changes: * Fall back to OperationError only if the operation was already terminating and remains too large. * Clear the pending operation once finalized to stop the controller from re-attempting the same doomed sync. * Drop SyncResult.Resources from the fallback patch so it avoids hitting the same size limit itself. * Detect the "etcdserver: request is too large" error string directly instead of relying on the unused rpctypes sentinel. Signed-off-by: Julian-Chu <yulang.chu@gmail.com>
When resuming an operation already in the Terminating phase, the recorded state.Message describing why it's terminating was being dropped instead of being surfaced as the termination cause on the final Failed/Error state. Add a test covering this case, and fix an existing test that relied on a stale fixture message which the new behavior now exposes. Signed-off-by: Julian-Chu <yulang.chu@gmail.com>
Contributor
Author
|
some update: etcdserver issue is wrapped in the how it looks on the UI 
|
Signed-off-by: Julian-Chu <yulang.chu@gmail.com>
Signed-off-by: Julian-Chu <yulang.chu@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #28703
Checklist: