Skip to content

feat(cli): add --prune option to sandbox delete command - #3378

Open
engelmi wants to merge 1 commit into
NVIDIA:mainfrom
engelmi:add-delete-prune-option
Open

feat(cli): add --prune option to sandbox delete command#3378
engelmi wants to merge 1 commit into
NVIDIA:mainfrom
engelmi:add-delete-prune-option

Conversation

@engelmi

@engelmi engelmi commented Sep 16, 2026 •
edited by sjenning
Loading

Copy link
Copy Markdown

Summary

Add a --prune flag to openshell sandbox delete that deletes only inactive sandboxes. This allows users to clean up terminated, stopped, or errored sandboxes while preserving active and provisioning ones.

The prune filter targets sandboxes in these (inactive) phases:

  • Error
  • Completed

Sandboxes in unconfirmed or non-terminal states are preserved.

The flag conflicts with both --all and named sandbox arguments, ensuring clear deletion intent.

Related Issue

Fixes: #2594

Changes

Testing

  • mise run pre-commit passes
  • Unit tests added/updated
    - [ ] E2E tests added/updated (if applicable) (not applicable)

Checklist

  • Follows Conventional Commits
  • Commits are signed off (DCO)
    - [] Architecture docs updated (if applicable) (not applicable)

@copy-pr-bot

copy-pr-bot Bot commented Sep 16, 2026

Copy link
Copy Markdown

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

@engelmi engelmi mentioned this pull request Sep 16, 2026
Open
2 tasks done
@engelmi
engelmi force-pushed the add-delete-prune-option branch from 5e9dcbf to e0c79cd Compare September 17, 2026 06:34
@sjenning

sjenning commented Sep 17, 2026

Copy link
Copy Markdown
Collaborator

I think we just want to prune sandboxes in confirmed terminal states. That is Errror or Completed. All the other states could be transient and/or non-terminal (e.g. stopped sandboxes can be restarted).

@engelmi
engelmi force-pushed the add-delete-prune-option branch from e0c79cd to d0ef037 Compare September 18, 2026 06:41
@engelmi

engelmi commented Sep 18, 2026

Copy link
Copy Markdown
Author

I think we just want to prune sandboxes in confirmed terminal states. That is Errror or Completed. All the other states could be transient and/or non-terminal (e.g. stopped sandboxes can be restarted).

Agreed. I pushed the changes. PTAL @sjenning

@sjenning

sjenning commented Sep 18, 2026

Copy link
Copy Markdown
Collaborator

/ok to test d0ef037

@engelmi
engelmi force-pushed the add-delete-prune-option branch 2 times, most recently from 9362ae8 to c2ed632 Compare September 22, 2026 07:33
@engelmi

engelmi commented Sep 22, 2026

Copy link
Copy Markdown
Author

@sjenning I had to rebase. Could you trigger the CI again?

@engelmi
engelmi force-pushed the add-delete-prune-option branch from c2ed632 to a2579c7 Compare September 29, 2026 07:15
ericcurtin
ericcurtin approved these changes Sep 29, 2026
View reviewed changes

@ericcurtin ericcurtin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW (not that it's worth much), LGTM

@engelmi
engelmi force-pushed the add-delete-prune-option branch from a2579c7 to 9f33ea4 Compare September 30, 2026 15:05
@johntmyers johntmyers added the test:e2e Requires end-to-end coverage label Oct 5, 2026
@johntmyers

johntmyers commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator

/ok to test 9f33ea4

@github-actions

github-actions Bot commented Oct 5, 2026

Copy link
Copy Markdown

Label test:e2e applied for 9f33ea4. Open the existing run and click Re-run all jobs to execute with the label set. The run will execute the standard E2E suite after building the required gateway, sandbox, and supervisor images once. The matching required CI gate status on this PR will flip green automatically once the run finishes.

johntmyers
johntmyers reviewed Oct 5, 2026
View reviewed changes

@johntmyers johntmyers left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gator-agent

PR Review Status

No blocking code findings were found. @sjenning, I checked your request to prune only confirmed terminal states: the filter deletes Error and Completed sandboxes while preserving Stopped and every other phase. @engelmi, I checked your update and request to restart CI after rebasing; I applied test:e2e and posted /ok to test for the current head.

Action required: @engelmi, document openshell sandbox delete --prune in the Delete Sandboxes section of docs/how-it-works/sandboxes/overview.mdx, explaining that only Error and Completed are deleted and that the flag conflicts with names and --all; alternatively, a maintainer can explain why published docs are intentionally unnecessary.

Blocking findings: No blocking code findings.
Carried findings: None.

The man page and public CLI skill were updated, but the published Fern docs were not. Gator will remain in review until this docs requirement is addressed. Test dispatch has been requested; the E2E Label Help workflow is queued, and runtime test execution is not yet confirmed.

Gator metadata
  • Validation: Concentrated CLI cleanup feature linked to #2594 and directed by verified maintainer @sjenning.
  • Docs: Published docs update or explicit maintainer explanation required for the new CLI flag.
  • Checks: DCO and Trivy pass; Branch Checks and Helm Lint awaiting current-head mirror.
  • E2E: test:e2e applied; current-head /ok to test posted; runtime workflow dispatch not yet confirmed.
  • Head SHA: 9f33ea42f2f08efcecf33079e1c7df9c2ff9ca54
  • Base SHA: 5acaaba19281cb6b30171afeb8602ce83edd9e45
  • Merge base SHA: 5acaaba19281cb6b30171afeb8602ce83edd9e45
  • Patch ID: 9e59f3f882916d91de44f5cdd8863637f63b06dc
  • Gator payload: 10
  • Review mode: initial
  • Previous reviewed SHA: none
  • Review budget exhausted: no
  • Maintainer decision required: no
  • Next state: gator:in-review

@johntmyers johntmyers added gator:in-review Gator is reviewing or awaiting PR review feedback gator:blocked Gator is blocked by process or repository gates and removed gator:in-review Gator is reviewing or awaiting PR review feedback gator:blocked Gator is blocked by process or repository gates labels Oct 5, 2026
@engelmi
feat(cli): add --prune option to sandbox delete command
ead164b 
Add a --prune flag to `openshell sandbox delete` that deletes only
inactive sandboxes. This allows users to clean up terminated, or
errored sandboxes while preserving active and provisioning ones.

The prune filter targets sandboxes in these phases:
- Error
- Completed

Sandboxes in other phases (e.g. Ready, Provisioning) are preserved.

The flag conflicts with both --all and named sandbox arguments,
ensuring clear deletion intent.

Fixes: NVIDIA#2594

Signed-off-by: Michael Engel <mengel@redhat.com>
@engelmi
engelmi force-pushed the add-delete-prune-option branch from 9f33ea4 to ead164b Compare October 6, 2026 08:40
@engelmi

engelmi commented Oct 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

@johntmyers Updated the docs/how-it-works/sandboxes/overview.mdx to include an explanation of --prune and --all. Also, fixed the out-dated commit message to list the correct sandbox phases deleted by --prune.
PTAL

Edit: The previous e2e test failures seemed to be due to infrastructure issues (based on copilots assessment).

@johntmyers

johntmyers commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

/ok to test ead164b

@johntmyers

johntmyers commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

gator-agent

PR Review Status

@engelmi, I checked your Fern docs update and corrected commit description against the author-only changes after rebasing. The docs now explain that --prune deletes only Error and Completed sandboxes and is mutually exclusive with --all; the published-docs requirement is addressed. @sjenning's terminal-state guidance remains preserved, and the follow-up contains no executable code changes or new blocking findings.

No blocking findings or carried obligations remain. The docs could also mention that these flags cannot be combined with sandbox names; that wording gap is non-blocking.

I posted /ok to test for the current head and confirmed that Branch Checks, Helm Lint, and Branch E2E Checks are queued for it. The previous E2E failure belongs to the old head and does not establish this head's result. Gator will monitor the new runs; maintainer approval remains outstanding.

Gator metadata
  • Validation: Concentrated CLI cleanup feature linked to feat(cli): add sandbox prune command to delete ERROR-phase sandboxes #2594 and directed by verified maintainer @sjenning.
  • Docs: Relevant Fern page updated; navigation unchanged because no new page was added.
  • Checks: DCO passes; current-head Branch Checks and Helm Lint queued. Trivy Changes requires workflow authorization and has no successful current-head result yet.
  • E2E: test:e2e already applied; current-head Branch E2E Checks run 37438735044 queued. The old-head bot rerun instruction does not apply to this new run.
  • Head SHA: ead164bf719e54f5aee7693c7fe16eb4af8c872a
  • Base SHA: 12cec59bf4c36c305032143eb90870bd16d8820b
  • Merge base SHA: 12cec59bf4c36c305032143eb90870bd16d8820b
  • Patch ID: 3ea5b1f4591e84eb648701a46805079bbb29a4a9
  • Gator payload: 10
  • Review mode: follow_up
  • Previous reviewed SHA: 9f33ea42f2f08efcecf33079e1c7df9c2ff9ca54
  • Review budget exhausted: no
  • Maintainer decision required: no
  • Next state: gator:watch-pipeline

@johntmyers johntmyers added gator:watch-pipeline Gator is monitoring PR CI/CD status gator:blocked Gator is blocked by process or repository gates and removed gator:in-review Gator is reviewing or awaiting PR review feedback gator:watch-pipeline Gator is monitoring PR CI/CD status labels Oct 6, 2026
@engelmi

engelmi commented Oct 6, 2026

Copy link
Copy Markdown
Author

Requested changes have been made and CI (incl. e2e tests) passes. Is it ready to be merged? @sjenning

This branch has not been deployed

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

Reviewers

@johntmyers johntmyers johntmyers left review comments
@mrunalp mrunalp Awaiting requested review from mrunalp mrunalp is a code owner
@sjenning sjenning Awaiting requested review from sjenning sjenning is a code owner
@derekwaynecarr derekwaynecarr Awaiting requested review from derekwaynecarr derekwaynecarr is a code owner
+1 more reviewer
@ericcurtin ericcurtin ericcurtin approved these changes
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

gator:blocked Gator is blocked by process or repository gates test:e2e Requires end-to-end coverage

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat(cli): add sandbox prune command to delete ERROR-phase sandboxes

4 participants

@engelmi @sjenning @johntmyers @ericcurtin