feat: auto-create PVC/Docker volumes on sandbox creation#661
feat: auto-create PVC/Docker volumes on sandbox creation#661xfgong wants to merge 3 commits intoalibaba:mainfrom
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f39558b56e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
f39558b to
deb2cf1
Compare
When a sandbox requests a PVC volume that doesn't exist yet, the server now auto-creates it instead of failing. This eliminates the need for callers to independently manage storage lifecycle outside OpenSandbox. Changes: - Extend PVC model with optional provisioning hints (storageClass, storage size, accessModes) for auto-creation - Add volume_auto_create config flag (default: true) and volume_default_size (default: "1Gi") to StorageConfig - Add get_pvc/create_pvc methods to K8sClient - Kubernetes: ensure PVCs exist before creating workload, with race-condition handling (409 Conflict) and graceful RBAC degradation - Docker: auto-create named volumes when not found (via docker API) - Update Helm chart RBAC to include PVC verbs - Update OpenAPI spec, configuration docs, and example configs - Update tests for new behavior
deb2cf1 to
52d1493
Compare
| return # Skip all remaining PVCs — same SA, same permissions | ||
| raise | ||
| if existing is not None: | ||
| logger.info("PVC '%s' already exists in namespace '%s'", claim_name, self.namespace) |
There was a problem hiding this comment.
INFO level maybe is not suitable, there exists hotpath for sandbox creation wiith existing PVC.
|
System automatically create PVC if absent will help user a lot. it seems we didn't handle deletion of PVC created by us, maybe it's a problem? |
This log fires on every sandbox creation with an existing PVC, which is the common hot path. DEBUG avoids noise in production.
Add opt-in volume_auto_delete config (default false, Docker only). Auto-created volumes are labeled with opensandbox.io/volume-managed-by and cleaned up on sandbox deletion. Kubernetes PVCs are unaffected as their lifecycle is managed by StorageClass reclaim policy.
Good catch. For Docker, we've added opt-in volume cleanup (volume_auto_delete = false by default). Auto-created volumes are labeled with For Kubernetes PVCs, they don't need server-side cleanup — their lifecycle is governed by the StorageClass reclaimPolicy (Retain/Delete), so Kubernetes handles it natively. |
Resolves #660
When a sandbox requests a PVC volume that doesn't exist yet, the server now auto-creates it instead of failing. This eliminates the need for callers to independently manage storage lifecycle outside OpenSandbox.
Changes:
Summary
Testing
Breaking Changes
Checklist