Skip to content

[Bug]: distinguish unsupported sandbox in-place resize - #933

Open
googs1025 wants to merge 1 commit into
openkruise:masterfrom
googs1025:fix-sandboxclaim-inplace-waitready
Open

[Bug]: distinguish unsupported sandbox in-place resize#933
googs1025 wants to merge 1 commit into
openkruise:masterfrom
googs1025:fix-sandboxclaim-inplace-waitready

Conversation

@googs1025

@googs1025 googs1025 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Ⅰ. Describe what this PR does

This PR refines Sandbox in-place resource resize failure reporting for clusters where the Pod resize path is not supported.

Before this change, a resize capability mismatch was reported with the same generic condition reason used by other in-place update failures:

InplaceUpdate=False
Reason=Failed
Message=in-place pod resize not supported

That message is visible to operators, but the reason is not machine-readable enough to distinguish an unsupported cluster/runtime resize path from other generic in-place update failures.

This PR adds one structured InplaceUpdate reason:

  • UnsupportedResize: the cluster/runtime resize path is not supported.

Other terminal resource resize failures, such as kubelet reporting an infeasible resize after the Pod has moved to the target revision, continue to use the existing Failed reason.

The claim wait-ready behavior intentionally remains compatibility-preserving. NewSandboxWaitReadyTask only waits while the Sandbox reports InplaceUpdating; terminal in-place update failure reasons, including UnsupportedResize, fall through to the existing usable-Sandbox readiness check. As a result, this PR improves status classification without changing the current best-effort claim semantics.

Broader API semantics, such as a future spec.inplaceUpdate.failurePolicy with BestEffort, Fail, or CreateNew, are tracked in #934 and should be designed separately.

Ⅱ. Does this pull request fix one issue?

Related to #934

Ⅲ. Describe how to verify it

go test ./api/v1alpha1 -count=1
go test ./pkg/cache -run 'TestNewSandboxWaitReadyTask|TestCache_CountActiveSandboxes' -count=1
go test ./pkg/controller/sandbox/core -run 'TestHandleInPlaceUpdateCommon|TestIsInplaceUpdateTerminal' -count=1
go test ./pkg/controller/sandboxclaim/core -run 'Test.*Claim|Test.*Inplace|Test.*buildClaimOptions' -count=1
make generate manifests

Ⅳ. Special notes for reviews

This PR does not introduce a fallback policy and does not make UnsupportedResize directly control SandboxClaim success. It only makes the unsupported resize path explicit while preserving the existing best-effort claim behavior.

@kruise-bot

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign zmberg for approval by writing /assign @zmberg in a comment. For more information see:The Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@googs1025 googs1025 changed the title fix: fail sandbox claim wait on inplace update failure [Bug]: fail sandbox claim wait on inplace update failure Sep 3, 2026
@codecov

codecov Bot commented Sep 3, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.15%. Comparing base (5b89c11) to head (c04ef17).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #933      +/-   ##
==========================================
+ Coverage   83.10%   83.15%   +0.04%     
==========================================
  Files         257      259       +2     
  Lines       22536    22548      +12     
==========================================
+ Hits        18728    18749      +21     
+ Misses       3094     3079      -15     
- Partials      714      720       +6     
Flag Coverage Δ
unittests 83.15% <100.00%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread pkg/cache/tasks.go Outdated
)
}

func isTerminalResourceResizeFailure(message string) bool {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is not good way to fix.
will disscus in issue

@googs1025
googs1025 force-pushed the fix-sandboxclaim-inplace-waitready branch 2 times, most recently from ca94a9b to b58a9c7 Compare September 4, 2026 08:11
@googs1025 googs1025 changed the title [Bug]: fail sandbox claim wait on inplace update failure [Bug]: distinguish sandbox in-place resource resize failures Sep 4, 2026
@googs1025
googs1025 force-pushed the fix-sandboxclaim-inplace-waitready branch from b58a9c7 to b8b2f2e Compare September 4, 2026 08:14
@googs1025 googs1025 changed the title [Bug]: distinguish sandbox in-place resource resize failures [Bug]: distinguish unsupported sandbox in-place resize Sep 4, 2026
@googs1025
googs1025 force-pushed the fix-sandboxclaim-inplace-waitready branch from b8b2f2e to 15ac25a Compare September 4, 2026 08:32
Signed-off-by: CYJiang <googs1025@gmail.com>
@googs1025
googs1025 force-pushed the fix-sandboxclaim-inplace-waitready branch from 15ac25a to c04ef17 Compare September 4, 2026 09:07
@kruise-bot kruise-bot added size/L and removed size/M labels Sep 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants