fix(controller): keep pool scale/update/status running on schedule fa… - #1618
fix(controller): keep pool scale/update/status running on schedule fa…#1618Spground wants to merge 1 commit into
Conversation
…ilure scheduleSandbox now returns a best-effort result built from the last persisted allocation on failure, so update/scale/status proceed instead of aborting with a nil dereference. Failure events are recorded and conflict errors are returned first to preserve RetryOnConflict retries.
github-actions
Bot
added
component/k8s
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d3cefed1b8
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if errors.IsNotFound(err) { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
When a Pool disappears between the initial Reconcile fetch and this retry-closure fetch, returning success bypasses the allocation and scale-expectation cleanup performed by the outer not-found path. Both stores are keyed only by namespace/name, so if a Pool with the same name is recreated before the queued deletion request runs, that request fetches the new object and the old state is never cleared; stale creation expectations can then block the replacement Pool's scaling until timeout, while stale allocations are exposed to its first scheduling pass. Perform the same cleanup here before returning.
Useful? React with 👍 / 👎.
| allocation, err := r.Allocator.GetPoolAllocation(ctx, pool) | ||
| if err != nil { | ||
| logf.FromContext(ctx).Error(err, "Failed to read pool allocation for best-effort schedule result", "pool", pool.Name) | ||
| allocation = map[string]string{} |
| toDeletePods, err := r.doRelease(ctx, pool, batchSandboxes, pods, allocAction.ToRelease) | ||
| if err != nil { | ||
| return nil, err | ||
| return r.bestEffortScheduleResult(ctx, pool, pods), err |
There was a problem hiding this comment.
前面的allocation/release 如果存在部分成功/失败的情况，这里返回的状态会不会导致后面的scale出现问题？
2 participants
Summary
scheduleSandbox now returns a best-effort result built from the last persisted allocation on failure, so update/scale/status proceed instead of aborting with a nil dereference. Failure events are recorded and conflict errors are returned first to preserve RetryOnConflict retries.
Testing
Breaking Changes
Checklist