feat: implement OSSFS storage backend in sandbox lifecycle#340
feat: implement OSSFS storage backend in sandbox lifecycle#340hittyt wants to merge 10 commits intoalibaba:mainfrom
Conversation
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a72893a549
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
server/src/services/docker.py
Outdated
| current = self._ossfs_mount_ref_counts.get(mount_key, 0) | ||
| if current <= 1: | ||
| self._ossfs_mount_ref_counts.pop(mount_key, None) | ||
| should_unmount = True |
There was a problem hiding this comment.
Unmount only when OSSFS mount has tracked references
The release logic treats current == 0 the same as current == 1 and proceeds to unmount. After process restart, _ossfs_mount_ref_counts is empty (it is initialized in-memory and not rebuilt from existing containers), so deleting one sandbox can unmount a shared OSSFS path still used by other running sandboxes. This can break active workloads by tearing down their underlying bind source unexpectedly.
Useful? React with 👍 / 👎.
a72893a to
ce216ac
Compare
|
Please rebase main to resolve pyproject.toml format error |
- Implement OSSFS volume support in Docker runtime with mount reference counting. - Add comprehensive validation and unit tests for the new storage backend.
ce216ac to
c12516d
Compare
aaf4e73 to
25eb4ed
Compare
sdks/sandbox/python/src/opensandbox/api/lifecycle/models/credential_ref.py
Outdated
Show resolved
Hide resolved
server/src/services/docker.py
Outdated
| return pvc_inspect_cache | ||
|
|
||
| @staticmethod | ||
| def _derive_oss_region(endpoint: str) -> Optional[str]: |
There was a problem hiding this comment.
What is the purpose of resolving the region? It looks like the region is only required for ossfs 1.0 with sigv4 enabled, and it is not the region of the OSS bucket, but rather the region of client.
region=cn-hangzhou:OSS Bucket请求地域标识符,挂载时添加-oregion=<region_id>,默认为空。在使用V4签名时必须添加该选项来作为发起请求地域的标识符。
There was a problem hiding this comment.
But region is the region of client,not region of oss bucket
There was a problem hiding this comment.
Removed region logic
…vior Move Docker OSSFS-specific logic into a dedicated module to reduce branching noise in the core provisioning flow, and align OSSFS request validation with schema-driven constraints and platform support boundaries. This also fixes server pyproject TOML parsing so validation/test tooling can run reliably. Made-with: Cursor
Make OSSFS mount preparation transactional by rolling back already prepared mount keys when a later mount in the same batch fails. Add a regression test to prevent ref-count leaks in multi-volume partial-failure scenarios. Made-with: Cursor
a332819 to
288e383
Compare
Remove endpoint-derived region/sigv4 auto-injection for OSSFS 1.0 so region semantics stay explicitly controlled by caller options. Add assertions to keep v1 command generation free of implicit region flags. Made-with: Cursor
Install and expose both JDK 8 and JDK 17 in Java E2E so Gradle toolchain resolution does not depend on foojay auto-download in CI. Configure Gradle installation paths explicitly to avoid intermittent Java 8 provisioning failures. Made-with: Cursor
11e4b44 to
8338c4d
Compare

This PR implements the OSSFS storage backend for the sandbox lifecycle, as detailed in OSEP-0003.
Key features:
ossfsmounting.