Skip to content

test(sandbox): add NFS kernel extraArgs integration test#168

Closed
devin-ai-integration[bot] wants to merge 3 commits into
mainfrom
cdrappier/devin/nfs-kernel-support
Closed

test(sandbox): add NFS kernel extraArgs integration test#168
devin-ai-integration[bot] wants to merge 3 commits into
mainfrom
cdrappier/devin/nfs-kernel-support

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Fixes ENG-2980

Summary

Adds an integration test for the new nfs kernel variant in extra_args. The test creates a sandbox with {"nfs": "enabled"} and verifies the value persists on retrieval.

Depends on controlplane PR https://github.com/blaxel-ai/controlplane/pull/4348 which adds "nfs" to the allowedExtraArgKeys validation.

Note: Generated client models (sandbox_runtime_extra_args.py, sandbox_runtime.py) will auto-update on next OpenAPI codegen cycle once the controlplane spec is released.

Link to Devin session: https://app.devin.ai/sessions/9fcb86f5a6fe43b5bd4e8aad77a95b1f
Requested by: @drappier-charles


Note

Adds an integration test for creating a sandbox with nfs=enabled extra args, with graceful skip when NFS support isn't deployed yet and proper resource cleanup.

Written by Mendral for commit 650d3e5.

Add test case for creating a sandbox with nfs=enabled extraArgs,
matching the new kernel variant added in controlplane PR #4348.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR that start with 'DevinAI' or '@devin'.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment, CI, and merge conflict monitoring

@mendral-app

mendral-app Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

🔄 Interaction Flow

This PR adds an integration test for the nfs kernel extra_args option. Here's the sequence of interactions exercised by the new test:

sequenceDiagram
    participant Test as Test Runner
    participant SI as SandboxInstance
    participant CP as Controlplane API

    Test->>SI: create({name, image, extra_args: {nfs: "enabled"}, labels})
    SI->>CP: POST /sandboxes (with nfs extra_arg)
    CP-->>SI: 201 Created
    SI-->>Test: sandbox created

    Test->>SI: get(name)
    SI->>CP: GET /sandboxes/{name}
    CP-->>SI: Sandbox spec (with extra_args)
    SI-->>Test: SandboxInstance

    Test->>Test: assert extra_args["nfs"] == "enabled"

    Test->>SI: delete(name)
    SI->>CP: DELETE /sandboxes/{name}
    CP-->>SI: 200 OK
Loading

Summary: The test validates the round-trip persistence of extra_args: {"nfs": "enabled"} through the SDK → Controlplane flow: create a sandbox with the NFS flag, retrieve it, assert the value persists, then clean up.

⚠️ Dependency note: This test requires controlplane PR #4348 to land first (adds "nfs" to allowedExtraArgKeys validation). Until then, the create call will likely be rejected by the API.

Note

Posted by PR Sequence Diagram · Tag @mendral-app with feedback.

@mendral-app

mendral-app Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

📋 Created Linear issue ENG-2980 — status: In Progress

  • Assignee: @drappier-charles (reviewer — bot PR)
  • Labels: SDK, Sandox
  • Estimate: S (19 additions, 1 file)
  • PR linked: ✅ Issue will auto-close when this PR merges

Auto-created because no Linear reference was found in the PR title, description, or branch name.

Note

Posted by Linear Issue Enforcer · Tag @mendral-app with feedback.

…t yet

The test gracefully skips if the API returns 'unsupported extraArgs key nfs',
and will automatically start passing once controlplane PR #4348 is deployed.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@mendral-app

mendral-app Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

🧪 Testing Guide

What this PR addresses

Adds an integration test for a new nfs kernel variant in sandbox extra_args. The test creates a sandbox with {"nfs": "enabled"} and verifies the value persists when the sandbox is retrieved. This depends on a controlplane change (controlplane#4348) that adds "nfs" to allowedExtraArgKeys.

Steps to reproduce / how to exercise

  1. Ensure the controlplane with NFS support (PR #4348) is deployed to the test environment.
  2. Run the new integration test:
    pytest tests/integration/core/sandbox/test_extra_args.py::TestExtraArgs::test_creates_sandbox_with_nfs_enabled -v
  3. If the controlplane hasn't deployed NFS support yet, the test should skip gracefully with a message rather than failing.

What to verify (expected behavior)

  • With NFS support deployed: The test passes — a sandbox is created with extra_args: {"nfs": "enabled"}, and retrieving it confirms the value persists.
  • Without NFS support deployed: The test is skipped via pytest.skip() with the message "controlplane hasn't deployed nfs support yet" (no hard failure).
  • Cleanup: The sandbox is deleted in the finally block only if creation succeeded (created flag guards against deleting a non-existent resource).
  • Existing tests unaffected: Run the full extra_args test suite (pytest tests/integration/core/sandbox/test_extra_args.py -v) and confirm no regressions in test_creates_sandbox_with_nvme_enabled or test_creates_sandbox_with_both_iptables_and_nvme.

Note

Posted by PR Testing Guide · Tag @mendral-app with feedback.

mendral-app[bot]

This comment was marked as outdated.

Wrap entire post-create flow in try/finally with a 'created' flag
so the sandbox is always cleaned up even if get/assertions fail.

Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>

@mendral-app mendral-app Bot 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.

LGTM

Previous resource leak concern is fully addressed — the created flag with a single try/except/finally ensures cleanup runs in all post-creation failure paths. Code is clean and follows the same patterns as the existing tests.

Tag @mendral-app with feedback or questions. View session

@drappier-charles drappier-charles marked this pull request as ready for review June 10, 2026 22:45
@drappier-charles drappier-charles deleted the cdrappier/devin/nfs-kernel-support branch June 10, 2026 22:46

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 2 additional findings.

Open in Devin Review

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant