Added Shared Volume Support via Volume Mount Options#20
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds sandbox-local shared tmpfs volumes that can be mounted by multiple containers within the same sandbox (while still disallowing sharing across sandboxes), spanning orchestrator types/validation, sbxlet runtime mounting/cleanup, and documentation.
Changes:
- Extend sandbox specs/APIs with
spec.volumesandcontainers[].volume_mounts(orch) /volumesandcontainers[].volumeMounts(sbxlet). - Validate and forward shared volume definitions/mounts from orchestrator to node agent, and mount tmpfs-backed shared volumes in the runtime.
- Update docs and add examples/tests covering shared volume forwarding and validation.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| sandboxd-orch/types/types.go | Adds Volumes to SandboxSpec and VolumeMounts to container spec types. |
| sandboxd-orch/service/sandbox_ops.go | Validates shared volumes/mounts and forwards them to sbxlet create requests. |
| sandboxd-orch/service/sandbox_ops_test.go | Adds tests for forwarding shared volumes and create-time validation failures. |
| sandboxd-let/sandbox/state.go | Persists incoming Volumes and container VolumeMounts into sandbox state. |
| sandboxd-let/sandbox/service.go | Ensures sandbox state copies include Volumes and VolumeMounts. |
| sandboxd-let/sandbox/runtime.go | Implements tmpfs shared volume mounts, container mount wiring, and cleanup on delete. |
| sandboxd-let/model/validate.go | Adds model-level validation for volume/mount basic constraints. |
| sandboxd-let/model/validate_test.go | Adds validation tests for shared volumes and mounts. |
| sandboxd-let/model/types.go | Introduces VolumeSpec/VolumeMount types and wires them into API models. |
| sandboxd-ctl/cmd/resource_cmds.go | Minor refactor using maps.Copy for log printing bookkeeping. |
| README.md | Documents volumes / volume_mounts and updates container field list. |
| examples/shared-volume.yaml | Provides a runnable manifest demonstrating shared volumes between containers. |
| docs/sandboxd.md | Updates sbxlet API docs with volumes and volumeMounts fields and constraints. |
| docs/orchestrator.md | Updates orchestrator API docs with spec.volumes and spec.containers[].volume_mounts. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Previously, container volumes within a sandbox were completely isolated and not shared between containers. While this aligned with the project's original philosophy, there are cases where sharing specific paths between containers is desirable.
To address this, the following functionality has been added in this PR (excerpt from the README).
volumes
A Sandbox may define sandbox-local shared ephemeral volumes through
spec.volumes.name: The volume name. It must be unique within the sandbox.ephemeral_storage: Required size limit for the shared tmpfs volume. Once this limit is exceeded, writes returnENOSPC.Volumes are local to a single sandbox. They are created when the sandbox starts and automatically removed when the sandbox is deleted. Sharing volumes across different sandboxes is not supported.
volume_mounts
Each container may mount one or more sandbox-local shared volumes through
volume_mounts.name: References one of the entries declared inspec.volumes.mount_path: Absolute path inside the container where the shared volume will be mounted.read_only: Optional flag that mounts the volume as read-only for that container.Note
/tmpremains reserved for the container's own ephemeral tmpfs budget and cannot be used as a shared volume mount path.An example manifest is shown below:
For more details, please refer to the updated source code and documentation.