NVIDIA / NVIDIA/OpenShell

bug(e2e): sandbox pods not cleaned up during test runs — SandboxGuard::Drop uses detached threads

Open
#2,922 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

state:accepted
Dominant language
Rust
Stars
8.7k
Forks
1.3k
Avg merge
2d 11h
Merged PRs (30d)
253

Description

User Story

As an operator running E2E tests on a Kubernetes or OpenShift cluster,
I want sandbox pods created by tests to be reliably deleted when each test finishes,
so that the cluster does not accumulate orphaned pods that consume resources until the entire test run completes and the namespace is deleted.

Problem Statement

E2E tests create sandbox pods that are not reliably cleaned up after each test. Two issues contribute:

  1. SandboxGuard::Drop uses a detached thread (std::thread::spawn) to run openshell sandbox delete. When the test process exits or nextest terminates it, these threads are killed before the delete command completes. This affects all tests that use SandboxGuard.

    // e2e/rust/src/harness/sandbox.rs, line 654
    std::thread::spawn(move || {
        let rt = tokio::runtime::Runtime::new().expect("create cleanup runtime");
        rt.block_on(async {
            // ...
            let mut cmd = openshell_cmd();
            cmd.arg("sandbox").arg("delete").arg(&name);
            let _ = cmd.status().await;
        });
    });
    
  2. Two tests use manual delete_sandbox() without RAII guards, so a panic before the cleanup call leaks sandboxes:

    • e2e/rust/tests/sandbox_labels.rs — creates 4 sandboxes via create_sandbox_with_labels(), manual delete_sandbox() at lines 230–233.
    • e2e/rust/tests/sandbox_lifecycle.rs (canonical_main_exit_transitions_persistent_sandbox_to_error) — creates a sandbox via raw CLI, manual delete_sandbox() at line 267.

Impact / Why This Matters

When running the full E2E suite with no test filter, 9+ sandbox CRDs accumulate in the openshell namespace and remain for the duration of the test run. On resource-constrained clusters (CI, shared OpenShift environments), this can exhaust node resources, slow down subsequent tests, or hit pod quotas. The sandboxes are eventually cleaned up when with-kube-gateway.sh's EXIT trap deletes the namespace, but only after all tests complete — which can take several minutes with a full suite.

The ManagedCleanup pattern used in workspace_namespace_managed.rs already solves this correctly with synchronous std::process::Command in Drop, proving the fix is straightforward.

Acceptance Criteria

  • SandboxGuard::Drop uses synchronous cleanup (e.g. std::process::Command) instead of a detached thread, ensuring sandbox deletion completes before the process exits
  • sandbox_labels.rs wraps its sandboxes in SandboxGuard or equivalent RAII cleanup
  • sandbox_lifecycle.rs (canonical_main_exit_transitions_persistent_sandbox_to_error) wraps its sandbox in SandboxGuard or equivalent RAII cleanup
  • Running the full E2E suite shows no lingering sandbox pods between test cases (verified by inspecting the namespace mid-run)

Reproduction Steps

  1. Set up a Kind cluster (or use an existing Kubernetes/OpenShift cluster):

    kind create cluster --name openshell-e2e-test
    
  2. Run the full Kubernetes E2E suite using mise (no test filter):

    OPENSHELL_E2E_KUBE_CONTEXT=kind-openshell-e2e-test mise run e2e:kubernetes
    

    For OpenShift clusters, apply the SCC binding and overlay first:

    oc adm policy add-scc-to-user privileged -z openshell-sandbox -n openshell
    OPENSHELL_E2E_KUBE_CONTEXT=$(oc config current-context) \
      OPENSHELL_E2E_KUBE_EXTRA_VALUES=deploy/helm/openshell/ci/values-openshift-e2e.yaml \
      mise run e2e:kubernetes
    
  3. While tests are running, observe sandbox CRDs and pods accumulating in the openshell namespace:

    kubectl get sandbox -n openshell
    kubectl get pods -n openshell
    
  4. Example output mid-run (observed on OpenShift, ~28 minutes into a full suite run):

    NAME                          AGE
    default--e2e-sparse-enrich    4m48s
    default--engaged-bird         18m      # stale — test completed long ago
    default--foremost-staghound   8m1s
    default--hale-oxpecker        23m      # stale
    default--immense-drake        4m48s
    default--legendary-dunlin     28m      # stale
    default--lightened-woodcock   23m      # stale
    default--patient-hornet       13m      # stale
    default--visionary-vireo      4m48s
    

    9 sandbox CRDs accumulated, with the oldest never cleaned up by the test harness. They are only removed when the namespace is deleted at the end of the full test run.

Environment

  • OpenShell: main branch (commit 7fc61389)
  • Rust E2E harness: e2e/rust/src/harness/sandbox.rs
  • Test runner: cargo test via mise run e2e:kubernetes
  • Tested on: Kind (vanilla Kubernetes) and OpenShift clusters

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in e2e/rust/src/harness/sandbox.rs at SandboxGuard::Drop, then inspect the ManagedCleanup pattern in workspace_namespace_managed.rs. Review e2e/rust/tests/sandbox_labels.rs and the canonical_main_exit_transitions_persistent_sandbox_to_error test in sandbox_lifecycle.rs. Run the full suite with mise run e2e:kubernetes and verify sandbox CRDs and pods are cleaned up between tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
kubernetes, rust
Domain
infrastructure, testing
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
78/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.