Repository navigation
fix(soak): size an autoscaler step after the status shows its target - #318
Merged
Merged
Conversation
… step The operator writes a step to its state first. The status shows the limit of the pods, and the StatefulSet gets the step limit one reconcile later. Thus the first status of a step can show the limit before the step, and the judge reported a correct step from 128Mi to 192Mi as a step to 128Mi. The size check waits until effectiveMemory moves in Applying, or the step is Done or Held. A revert, and a user change of the spec limit or of memoryMax in the step, give no check. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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
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.
Changes proposed on the PR:
Applying, the newlastStep), and the status still shows the oldeffectiveMemory(or none, when it equals the spec limit). The template and the status get the target one reconcile later. The soak run of main740f2c9eraised a falseautoscale_step_sizefinding at every step ("the step from 128Mi to 128Mi, want 192Mi"), although the operator steps were correct.memoryMax. It checks the size inApplyingas soon aseffectiveMemorydiffers from that limit, and atDoneorHeldfor a step that is first seen late. ARevertedstep gets no check. A change of the spec limit or ofmemoryMaxduring the step drops the check, because the limit then moves for another reason.Donestep that still shows the old limit). Each step is counted once. The "started" log line no longer quotes a target limit.effectiveMemoryin the first round of a step (cooldown and budget uselastStep; shrink, maxmemory, restart and handover use the pod limits).Possible operator change, not in this PR: the status pairs the new
lastStepandApplyingwith the oldeffectiveMemoryfor one reconcile. Showing the target duringApplyingwould change the meaning ofeffectiveMemory(the limit of the StatefulSet), so it needs a decision.Tests:
TestStepSizeAfterTheFirstRound(11 cases: the production status sequence, a wrong size, a fast step first seen asDone,Held,Reverted, a spec ormemoryMaxchange during the step). 8 of the 11 fail on main. Not tested: a kind or e2e run.🤖 Generated with Claude Code
https://claude.ai/code/session_01G4mmyo3sqLwf23m5FE2nBE
Generated by Claude Code