Skip to content

fix(deepswe): Fix leaked sandboxes on trajectory completion and failure - #2312

Open
andytwigg wants to merge 1 commit into
mainfrom
fix-sandbox-cleanup
Open

andytwigg wants to merge 1 commit into
mainfrom
fix-sandbox-cleanup

Conversation

@andytwigg

Copy link
Copy Markdown
Collaborator

Summary

This PR resolves an issue where sandboxes from failed, completed, or cancelled trajectories accumulate in Kubernetes clusters as stale SandboxClaim / Sandbox CRDs and PodFailed / restart-looping pods.


Root Causes

  1. Missing Controller Lifecycle Policy on SandboxClaim (shutdownPolicy: Retain default):
    When SandboxClaim custom resources were created by the agent-sandbox runtime, they defaulted to shutdownPolicy: Retain without ttlSecondsAfterFinished. When backing pods completed or terminated with exit code 137 (PodFailed), the Kubernetes agent-sandbox controller kept the claims and sandboxes alive in etcd and attempted to restart the pod or pool.
  2. Fragile and Incomplete Environment Cleanup in SWEEnv.close():
    If the underlying execution environment (self.env.close()) threw an exception (e.g. network timeout or dead Docker container), the remainder of close() was aborted before fleet.release(handle) was called. Furthermore, fleet.release() only untracked the handle in memory if it was found in fleet._handles; if fleet was None or untracked, no teardown of the claim/sandbox occurred.
  3. Trajectory Engine Collect Loop Bypassing Environment Cleanup on Error:
    In tunix/rl/agentic/trajectory/trajectory_collect_engine.py, TrajectoryCollectEngine.collect() only had try ... finally: await self._close() wrapping the final reward calculation. If an exception occurred earlier during await self._reset() or inside the rollout stepping loop (await self._one_step()), _close() was never reached, causing any allocated environment/sandbox to leak.
  4. Kubernetes Controller ReconcilerError from Resource Spec Mismatch:
    In examples/deepswe/template.py, default or configured CPU requests (SANDBOX_CPU=2500m) could exceed CPU limits (SANDBOX_CPU_LIMIT=2), triggering admission failure ReconcilerError: cpu.requests must not exceed cpu.limits on warm pools. Additionally, pod templates lacked an activeDeadlineSeconds safeguard to automatically reap hung pods.

Key Changes

  1. Configured Claim Lifecycle & Defensiveness in examples/deepswe/swe_env.py:
    • Added configure_claim_lifecycle(handle, ttl_seconds) to automatically patch spec.lifecycle.shutdownPolicy: "Delete" and ttlSecondsAfterFinished: 60 on claim acquisition. Even if the client process abruptly crashes or is OOM-killed, Kubernetes garbage collects the claim and sandbox.
    • Added cleanup_k8s_sandbox_handle(handle) to defensively terminate the sandbox instance, call delete_claim / delete_sandbox via cluster resources, and delete remaining sandboxclaims and sandboxes custom objects via CustomObjectsApi.
    • Updated SWEEnv._init_agent_sandbox_env() to wrap workspace/env initialization in try ... except that calls self.close() on error before re-raising.
    • Hardened SWEEnv.close():
      • Wrapped self.env.close() and self.workspace.cleanup() in try ... except.
      • Added fallback from fleet.release(handle) to handle.release().
      • Invokes cleanup_k8s_sandbox_handle(handle).
    • Implemented context manager (__enter__, __exit__) and destructor (__del__) on SWEEnv to guarantee resource cleanup.
  2. Guaranteed Episode Cleanup in tunix/rl/agentic/trajectory/trajectory_collect_engine.py:
    • Wrapped the entire episode lifecycle (_reset(), the while True: stepping loop, and reward computation) inside try ... finally: await self._close().
    • Added defensive checks in _close() to handle None env and log any unexpected exceptions during cleanup without masking rollout exceptions.
  3. Resource Specification & Guardrails in examples/deepswe/template.py:
    • Added parse_cpu_to_millicores() and parse_memory_to_bytes() utilities.
    • Validated and ensured requests <= limits (adjusting limits if requests exceed limits) in get_openhands_pod_template() and get_r2egym_pod_template().
    • Added activeDeadlineSeconds support (default 7200s, configurable via SANDBOX_ACTIVE_DEADLINE_SECONDS).
    • Added lightweight fallback for TemplateSpec / ResourceSpec when agent_sandbox_rl is not pre-installed.
  4. Unit Tests:
    • Added TemplateAndLifecycleTest and TrajectoryCollectEngineLifecycleTest in examples/deepswe/sandbox_utils_test.py covering resource parsing, template normalization, claim lifecycle patching, defensive k8s handle cleanup, SWEEnv.close() resilience to exceptions, and TrajectoryCollectEngine cleanup on reset and step errors.
    • Added exception cleanup tests in tests/rl/agentic/trajectory/trajectory_collect_engine_test.py.
    • Verified that sandbox_k8s_e2e_test.py and sandbox_utils_test.py pass cleanly.

…tion

Prevent accumulation of stale SandboxClaims, Sandboxes, and PodFailed pods:
1. Configure lifecycle shutdownPolicy: Delete and ttlSecondsAfterFinished on
   SandboxClaims upon creation, ensuring automatic Kubernetes controller GC even
   if client processes fail or terminate abruptly.
2. Defensively clean up Sandbox and SandboxClaim custom resources on SWEEnv.close(),
   including terminate calls and Kubernetes CRD deletion even when the underlying
   env.close() raises an exception.
3. Add context manager (__enter__, __exit__) and destructor (__del__) to SWEEnv
   to ensure sandboxes are always released back to the fleet or deleted.
4. In TrajectoryCollectEngine, wrap the entire episode lifecycle (reset, rollout loop,
   and reward computation) in try ... finally: await self._close() to guarantee
   environment cleanup on exceptions during reset or rollout steps.
5. In template.py, validate and normalize CPU and memory requests against limits
   (requests <= limits) and add activeDeadlineSeconds to prevent admission ReconcilerErrors
   and indefinitely running leaked pods.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants