Commit 4c6a7bd
committed
fix(k8s): delete auto-created PVCs on sandbox deletion when opted in
Follow-up to #661, addressing the post-merge review by @yinsyim
(#661 (comment)).
The original PR assumed StorageClass.reclaimPolicy would clean up
auto-created PVCs, but reclaimPolicy only governs what happens to the PV
*after* the PVC is deleted — it does not delete the PVC when the
sandbox/workload terminates. Auto-created PVCs were leaking on every
sandbox deletion.
This PR fixes that by:
- Gating cleanup on the existing deleteOnSandboxTermination PVC field
(default false, opt-in), uniform with Docker semantics. Opt-in PVCs are
labeled with `opensandbox.io/volume-managed-by=server` and
`opensandbox.io/id=<sandbox_id>`. Pre-existing PVCs and opt-out PVCs are
never deleted by the server.
- Attaching ownerReferences from each opt-in PVC to the just-created
workload CR after create_workload succeeds. Kubernetes garbage
collection then deletes the PVC whenever the CR goes away — by the
delete_sandbox API, by TTL expiry handled in the controller, or by any
cascade. The label-based sweep in delete_sandbox remains as a fallback.
- Sweeping auto-created PVCs when sandbox creation fails before
returning. Previously create_workload / _wait_for_sandbox_ready
failures left labeled PVCs orphaned because the caller has no sandbox
id to call delete_sandbox with.
- Rejecting 400 when the same claim_name appears in multiple Volume
entries with inconsistent createIfNotExists / deleteOnSandboxTermination
flags (first-wins previously caused either silent leaks or silent
deletions).
Server changes:
- services/k8s/client.py: add list_pvcs, delete_pvc, patch_pvc.
- services/k8s/kubernetes_service.py:
- _ensure_pvc_volumes: label only on opt-in; reject conflicting flags;
return the list of newly auto-created managed PVCs.
- new _attach_pvc_owner_references: best-effort patch after
create_workload, gated on workload_info exposing apiVersion/kind/uid.
- new _cleanup_managed_pvcs: list-by-selector + delete; best-effort.
- delete_sandbox: call cleanup after workload deletion succeeds or 404s,
never on transient errors.
- create_sandbox: finally-sweep on any failure before return.
- services/k8s/batchsandbox_provider.py and agent_sandbox_provider.py:
return apiVersion and kind alongside name and uid so the
ownerReference can be constructed.
Spec, docs, SDKs, RBAC:
- specs/sandbox-lifecycle.yml + server/api/schema.py: replace the stale
"Docker only / no effect on K8s / StorageClass reclaim policy handles
it" description with correct cross-backend semantics. The reclaim-policy
reference remains but is reframed (the PVC delete triggers it).
- docs/public/api/spec-inline.js: regenerated via `pnpm docs:spec` so
the published API docs reflect the new description.
- SDKs: regenerated Python lifecycle and JS lifecycle from the spec;
hand-edited Python sandbox model, C# model, and Kotlin model
docstrings.
- kubernetes/charts/opensandbox-server: grant `delete` and `list` on
persistentvolumeclaims so the cleanup path runs under the stock chart.
Tests:
- Label stamping for opt-in and opt-out paths.
- Cleanup on success, on 404 sweep, no-cleanup on transient error,
best-effort tolerance of list_pvcs failure, no-touch for pre-existing
PVCs.
- Conflict validation rejects mismatched flags and allows matching ones.
- create_sandbox finally-sweep covers create_workload and
wait_for_ready failures.
- _attach_pvc_owner_references patches each managed PVC with the right
body, is best-effort on failure, and skips when owner info is
incomplete.
End-to-end verified on a real cluster:
- opt-in PVC carries labels and ownerReferences; delete_sandbox API
cleans it via the label sweep; direct CR delete (simulating TTL) is
cleaned via K8s GC.
- opt-out PVC has no labels or ownerReferences and survives any CR
delete.
- Pre-existing PVCs are untouched.1 parent 7e57438 commit 4c6a7bd
16 files changed
Lines changed: 693 additions & 40 deletions
File tree
- docs/public/api
- kubernetes/charts/opensandbox-server/templates
- sdks/sandbox
- csharp/src/OpenSandbox/Models
- javascript/src/api
- kotlin/sandbox/src/main/kotlin/com/alibaba/opensandbox/sandbox/domain/models/sandboxes
- python/src/opensandbox
- api/lifecycle/models
- models
- server
- opensandbox_server
- api
- services/k8s
- tests/k8s
- specs
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
28 | 28 | | |
29 | 29 | | |
30 | 30 | | |
31 | | - | |
| 31 | + | |
32 | 32 | | |
33 | 33 | | |
34 | 34 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
163 | 163 | | |
164 | 164 | | |
165 | 165 | | |
166 | | - | |
| 166 | + | |
| 167 | + | |
167 | 168 | | |
168 | 169 | | |
169 | 170 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1302 | 1302 | | |
1303 | 1303 | | |
1304 | 1304 | | |
1305 | | - | |
1306 | | - | |
1307 | | - | |
| 1305 | + | |
| 1306 | + | |
| 1307 | + | |
1308 | 1308 | | |
1309 | 1309 | | |
1310 | 1310 | | |
| |||
Lines changed: 1 addition & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
406 | 406 | | |
407 | 407 | | |
408 | 408 | | |
409 | | - | |
| 409 | + | |
410 | 410 | | |
411 | 411 | | |
412 | 412 | | |
| |||
Lines changed: 3 additions & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
46 | 46 | | |
47 | 47 | | |
48 | 48 | | |
49 | | - | |
50 | | - | |
51 | | - | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
52 | 52 | | |
53 | 53 | | |
54 | 54 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
200 | 200 | | |
201 | 201 | | |
202 | 202 | | |
203 | | - | |
204 | | - | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
205 | 206 | | |
206 | 207 | | |
207 | 208 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
180 | 180 | | |
181 | 181 | | |
182 | 182 | | |
183 | | - | |
184 | | - | |
185 | | - | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
186 | 186 | | |
187 | 187 | | |
188 | 188 | | |
| |||
Lines changed: 2 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
210 | 210 | | |
211 | 211 | | |
212 | 212 | | |
| 213 | + | |
| 214 | + | |
213 | 215 | | |
214 | 216 | | |
215 | 217 | | |
| |||
Lines changed: 4 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
297 | 297 | | |
298 | 298 | | |
299 | 299 | | |
| 300 | + | |
| 301 | + | |
300 | 302 | | |
301 | 303 | | |
302 | 304 | | |
| |||
364 | 366 | | |
365 | 367 | | |
366 | 368 | | |
| 369 | + | |
| 370 | + | |
367 | 371 | | |
368 | 372 | | |
369 | 373 | | |
| |||
0 commit comments