Skip to content

fix: remove UnsafeDisableDeepCopy in groupAllSandboxes to prevent informer cache corruption#387

Open
oindrilakha12-ui wants to merge 1 commit into
openkruise:masterfrom
oindrilakha12-ui:fix/unsafe-disable-deepcopy-cache-corruption
Open

fix: remove UnsafeDisableDeepCopy in groupAllSandboxes to prevent informer cache corruption#387
oindrilakha12-ui wants to merge 1 commit into
openkruise:masterfrom
oindrilakha12-ui:fix/unsafe-disable-deepcopy-cache-corruption

Conversation

@oindrilakha12-ui
Copy link
Copy Markdown

@oindrilakha12-ui oindrilakha12-ui commented May 14, 2026

What

Remove client.UnsafeDisableDeepCopy from groupAllSandboxes and fix slice aliasing in scaleDown to prevent informer cache corruption during concurrent scale-down and rolling update operations.

Why

  1. Cache Corruption: groupAllSandboxes lists sandboxes with client.UnsafeDisableDeepCopy, returning raw informer cache pointers. Downstream code mutates these pointers, silently corrupting the shared informer cache.
  2. Slice Aliasing: scaleDown used �ppend(groups.Creating, groups.Available...) which is a Go slice aliasing trap — when Creating has spare capacity, Available elements silently overwrite its memory.

Changes

  • Removed client.UnsafeDisableDeepCopy from the groupAllSandboxes List call.
  • Fixed slice aliasing by replacing the buggy �ppend with explicit make + dual appends.

Fixes #386
Fixes #388

…ormer cache corruption

groupAllSandboxes lists sandboxes with client.UnsafeDisableDeepCopy,
returning raw informer cache pointers. Downstream code (scaleDownSandbox,
deleteSandboxForUpdate) mutates annotations on these pointers via
LockSandbox, silently corrupting the informer cache and causing
non-deterministic controller behavior under concurrent load.

Additionally, scaleDown uses append(groups.Creating, groups.Available...)
which is a Go slice aliasing trap — when Creating has spare backing-array
capacity, Available elements silently overwrite Creating's memory.

Fix:
1. Remove client.UnsafeDisableDeepCopy from the List call so objects are
   deep-copied and safe for downstream mutation.
2. Replace the aliasing append with explicit make + two appends.

Fixes openkruise#386
@kruise-bot kruise-bot requested review from AiRanthem and zmberg May 14, 2026 10:18
@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 airanthem for approval by writing /assign @airanthem 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

@kruise-bot
Copy link
Copy Markdown

Welcome @oindrilakha12-ui! It looks like this is your first PR to openkruise/agents 🎉

@codecov
Copy link
Copy Markdown

codecov Bot commented May 14, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 75.91%. Comparing base (5d3212b) to head (f873d88).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #387   +/-   ##
=======================================
  Coverage   75.90%   75.91%           
=======================================
  Files         145      145           
  Lines       10626    10627    +1     
=======================================
+ Hits         8066     8067    +1     
  Misses       2212     2212           
  Partials      348      348           
Flag Coverage Δ
unittests 75.91% <100.00%> (+<0.01%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

2 participants