Skip to content

fix(e2e): use --name flag in landlock sandbox exec (#1851) - #1934

Merged
ericksoa merged 2 commits into
NVIDIA:mainfrom
jyaunches:fix/landlock-exec-name-flag
Apr 16, 2026
Merged

fix(e2e): use --name flag in landlock sandbox exec (#1851)#1934
ericksoa merged 2 commits into
NVIDIA:mainfrom
jyaunches:fix/landlock-exec-name-flag

Conversation

@jyaunches

@jyaunches jyaunches commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

Summary

04-landlock-readonly.sh has exited 127 on every nightly since April 8 when it was added in PR #1121. This is one of the three root causes keeping the nightly E2E red for 24+ consecutive days.

Root Cause

The sandbox_exec() helper calls:

openshell sandbox exec "$SANDBOX_NAME" -- bash -c "..."

But openshell 0.0.26 requires the --name flag:

openshell sandbox exec --name "$SANDBOX_NAME" -- bash -c "..."

Without --name, the sandbox name (e2e-cloud-experimental) is parsed as the command to execute, producing command not found → exit 127.

Fix

One-line change in sandbox_exec():

-  openshell sandbox exec "$SANDBOX_NAME" -- bash -c "$1" 2>&1
+  openshell sandbox exec --name "$SANDBOX_NAME" -- bash -c "$1" 2>&1

Verification

Reproduced and verified the fix on a DGX Spark running the E2E suite:

  • Before fix: 04-landlock-readonly: EXIT CODE: 127
  • After fix: openshell sandbox exec --name e2e-debug -- echo "hello from sandbox"hello from sandbox

Closes #1851

Summary by CodeRabbit

  • Chores
    • Standardized how sandbox instances are invoked in end-to-end tests to ensure consistent execution and verification across checks and rebuild scenarios.
    • Internal test-only changes; no user-facing behavior, API, or public interface modifications.

@coderabbitai

coderabbitai Bot commented Apr 16, 2026

Copy link
Copy Markdown
Contributor

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: a3aeebba-082f-4e23-ae2d-d040383808fb

📥 Commits

Reviewing files that changed from the base of the PR and between e003759 and 74f2585.

📒 Files selected for processing (2)
  • test/e2e/e2e-cloud-experimental/checks/04-landlock-readonly.sh
  • test/e2e/test-sandbox-rebuild.sh
✅ Files skipped from review due to trivial changes (1)
  • test/e2e/test-sandbox-rebuild.sh

📝 Walkthrough

Walkthrough

Replaced positional sandbox name arguments with an explicit --name "$SANDBOX_NAME" option in openshell sandbox exec invocations across two test scripts: the helper in 04-landlock-readonly.sh and three direct calls in test-sandbox-rebuild.sh. No other logic was changed.

Changes

Cohort / File(s) Summary
Sandbox exec flag updates
test/e2e/e2e-cloud-experimental/checks/04-landlock-readonly.sh, test/e2e/test-sandbox-rebuild.sh
Changed openshell sandbox exec "$SANDBOX_NAME" ... to openshell sandbox exec --name "$SANDBOX_NAME" ... in the helper and three direct calls. Command payloads and I/O handling unchanged.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I hopped through scripts at night,
Swapped a name from left to right,
One little flag, three tiny taps,
Sandbox listens, no more gaps 🌙

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: updating E2E tests to use the --name flag in openshell sandbox exec calls, matching the primary objective of fixing the failing nightly E2E tests.
Linked Issues check ✅ Passed The changes directly address issue #1851 by fixing the openshell sandbox exec invocation that caused exit code 127 in nightly E2E tests, restoring the expected functionality.
Out of Scope Changes check ✅ Passed All changes are scoped to fixing the openshell sandbox exec syntax in E2E test files as required by the linked issue, with no extraneous modifications.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@ericksoa ericksoa 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.

Nice fix — root cause analysis and verification are really thorough, thank you!

While looking at this, I noticed test/e2e/test-sandbox-rebuild.sh also calls openshell sandbox exec "$SANDBOX_NAME" (lines 99, 104, 147) without --name. Do you think those would hit the same exit-127 on openshell 0.0.26? Might be worth a quick check — would be great to sweep them all in one pass if so.

@wscurran

Copy link
Copy Markdown
Contributor

@jyaunches

Copy link
Copy Markdown
Contributor Author

Good catch @ericksoa — you were right, test-sandbox-rebuild.sh had the same positional-arg pattern at all three callsites (lines 99, 104, 147). Swept them in e003759. All three now use --name to match OpenShell 0.0.26's interface.

04-landlock-readonly.sh has exited 127 on every nightly since it was
added (April 8). The sandbox_exec() helper calls:

  openshell sandbox exec "$SANDBOX_NAME" -- bash -c "..."

but openshell 0.0.26 requires the --name flag for positional sandbox
identification. Without it, the sandbox name is parsed as the command
to execute, producing 'command not found' (exit 127).

Fix: openshell sandbox exec --name "$SANDBOX_NAME" -- bash -c "..."

Closes #1851
Sweep the same positional-arg fix from 04-landlock-readonly.sh to
test-sandbox-rebuild.sh (lines 99, 104, 147). OpenShell 0.0.26 requires
--name instead of a positional sandbox name for `openshell sandbox exec`.

Co-authored-by: Aaron Erickson <aerickson@nvidia.com>
@ericksoa
ericksoa force-pushed the fix/landlock-exec-name-flag branch from 58a236b to 74f2585 Compare April 16, 2026 20:53
@ericksoa
ericksoa merged commit 56ae053 into NVIDIA:main Apr 16, 2026
11 checks passed
@wscurran wscurran added area: ci CI workflows, checks, release automation, or GitHub Actions area: e2e End-to-end tests, nightly failures, or validation infrastructure bug-fix PR fixes a bug or regression chore Build, CI, dependency, or tooling maintenance and removed CI/CD chore Build, CI, dependency, or tooling maintenance labels Jun 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci CI workflows, checks, release automation, or GitHub Actions area: e2e End-to-end tests, nightly failures, or validation infrastructure bug-fix PR fixes a bug or regression

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nightly E2E failed — 2026-04-14

4 participants