fix: correct warm pool sandbox deletion when policy is retain

[alimx07]

Fix: delete warm-pool-adopted Sandboxes when SandboxClaim expires with Policy=Retain

Problem

SandboxClaimReconciler.reconcileExpired looks up the Sandbox to delete using client.ObjectKeyFromObject(claim), i.e. {claim.Namespace, claim.Name}. This works for Sandboxes created directly from a claim (they are named after the claim), but fails for Sandboxes adopted from a SandboxWarmPool.

Adopted Sandboxes keep the name the warm pool gave (e.g. basic-warmpool-a1b2c3). When the claim expires under ShutdownPolicy=Retain, the controller's lookup by claim.Name returns NotFound, the function returns nil, nil

Ofc, GC does not save us here: as the sandbox is still referenced with the claim. Once we delete the claim, everything is good.

Fix

Look up the Sandbox by claim.Status.SandboxStatus.Name when populated. This field is maintained by computeAndSetStatus to track the actual Sandbox name (direct or adopted). Fall back to claim.Name when status is unset, which preserves existing behavior for direct-created Sandboxes whose names are equal to the claim name anyway.

	statusName := claim.Name
	if claim.Status.SandboxStatus.Name != "" {
		statusName = claim.Status.SandboxStatus.Name
	}

	sandbox := &v1alpha1.Sandbox{}
	if err := r.Get(ctx, client.ObjectKey{Namespace: claim.Namespace, Name: statusName}, sandbox); err != nil { ... }

I also add a verfication to ensure the ownership between claim & sandbox before deletion using !metav1.IsControlledBy(sandbox, claim)

Tests

• Added Warm-Pool Scenario: Expanded TestSandboxClaimCleanupPolicy by adding a new test case (isWarmPool: true) to specifically simulate an adopted warm-pool sandbox.
• Fixed Assertions for Adopted Sandboxes: Decoupled the test verification logic by ensuring the test explicitly looks up the Sandbox using its actual initialized name (which can differ from the claim).

[netlify]

✅ Deploy Preview for agent-sandbox canceled.

Name	Link
🔨 Latest commit	79cc61f
🔍 Latest deploy log	https://app.netlify.com/projects/agent-sandbox/deploys/69e82280680abf0008e20b1b

[k8s-ci-robot]

Welcome @alimx07!

It looks like this is your first PR to kubernetes-sigs/agent-sandbox 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-sigs/agent-sandbox has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

[k8s-ci-robot]

Hi @alimx07. Thanks for your PR.

I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

[Copilot]

Pull request overview

Fixes Sandbox deletion for expired SandboxClaims with ShutdownPolicy=Retain when the Sandbox was adopted from a SandboxWarmPool (and therefore doesn’t share the claim’s name). This aligns cleanup behavior across direct-created and warm-pool-adopted Sandboxes.

Changes:

• Update reconcileExpired to look up the Sandbox by claim.Status.SandboxStatus.Name (fallback to claim.Name).
• Add an ownership guard (metav1.IsControlledBy) before deleting a Sandbox on claim expiration.
• Extend TestSandboxClaimCleanupPolicy with a warm-pool-adoption scenario and adjust sandbox lookup assertions accordingly.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
extensions/controllers/sandboxclaim_controller.go	Fixes expired-claim sandbox lookup for warm-pool-adopted sandboxes and adds an ownership guard before deletion.
extensions/controllers/sandboxclaim_controller_test.go	Adds/adjusts test coverage to validate deletion behavior when the adopted sandbox name differs from the claim name.

[barney-s]

/ok-to-test

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[barney-s]

/ok-to-test

[barney-s]

/lgtm
/approve

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: alimx07, barney-s

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

The pull request process is described here

Details

Needs approval from an approver in each of these files:

• extensions/controllers/OWNERS [barney-s]

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