Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
oindrilakha12-ui wants to merge 1 commit into
base: master
Choose a base branch
from
+10 −2
Open

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions pkg/controller/sandboxset/sandboxset_controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,12 @@ func (r *Reconciler) scaleDown(ctx context.Context, count int, sbs *agentsv1alph
log.Info("scale down", "count", count)

// Separate candidates into old revision and updated revision.
candidates := append(groups.Creating, groups.Available...)
// Allocate a new slice to avoid aliasing the backing arrays of Creating
// and Available. Using append(Creating, Available...) would mutate
// Creating's backing array when it has spare capacity.
candidates := make([]*agentsv1alpha1.Sandbox, 0, len(groups.Creating)+len(groups.Available))
candidates = append(candidates, groups.Creating...)
candidates = append(candidates, groups.Available...)
var oldCandidates, updatedCandidates []*agentsv1alpha1.Sandbox
for _, sbx := range candidates {
if sbx.Labels[agentsv1alpha1.LabelTemplateHash] != updateRevision {
Expand Down Expand Up @@ -425,10 +430,13 @@ func (r *Reconciler) updateSandboxSetStatus(ctx context.Context, newStatus agent
func (r *Reconciler) groupAllSandboxes(ctx context.Context, sbs *agentsv1alpha1.SandboxSet) (GroupedSandboxes, error) {
log := logf.FromContext(ctx)
sandboxList := &agentsv1alpha1.SandboxList{}
// NOTE: Do NOT use client.UnsafeDisableDeepCopy here. Downstream code
// (scaleDownSandbox, deleteSandboxForUpdate) mutates sandbox annotations
// (e.g. LockSandbox), so the returned objects must be independent copies
// to avoid corrupting the informer cache.
if err := r.List(ctx, sandboxList,
client.InNamespace(sbs.Namespace),
client.MatchingFields{fieldindex.IndexNameForOwnerRefUID: string(sbs.UID)},
client.UnsafeDisableDeepCopy,
); err != nil {
return GroupedSandboxes{}, err
}
Expand Down
Loading