fix(crd): remove spec.storage.retainOnDelete rather than leave it inert
The field validated, shipped in the CRD, and reached no controller. The world PVC survives deletion unconditionally -- it is a StatefulSet VolumeClaimTemplate, StatefulSet deletion does not cascade to template PVCs, and no finalizer exists anywhere in the operator. So setting it true described what already happened, and setting it false did nothing at all. False is the worse half: it reads as a request to delete a world, and was silently ignored. This departs from spec v4.1 §5, which asks for "删除:finalizer 清 Service/STS/ConfigMap,PVC 按 retainOnDelete". Neither half was ever built. Restoring that line means adding a finalizer whose other listed duties -- Service, StatefulSet, ConfigMap -- ownerReference GC already performs, so the only work it would newly do is delete worlds, on a path that does not pass the reaper's verified-backup check. The reaper is the one thing in the system allowed to destroy a world and it earns that by proving a backup first. A second door without that check is not an improvement. The spec is a frozen versioned document, so it is left alone and the departure is recorded in troubleshooting.md §13, beside the behaviour it explains. §12 loses its inert row and its opening sentence, which existed to introduce this one field: every field in that table is now read by a controller. Deployed installs need nothing. A CR still carrying retainOnDelete keeps working, because a v1 CRD prunes unknown keys on the next write and the behaviour the field claimed to control was never conditional. go build, go vet and go test ./... pass on Linux with zero failures; the CRD still parses and storage keeps size and storageClassName.
This commit is contained in:
3 files changed
+19
-14
No files matched your search
@@ -269,10 +269,6 @@ spec:
|
|||||||
storage:
|
storage:
|
||||||
description: Storage configures the world PVC.
|
description: Storage configures the world PVC.
|
||||||
properties:
|
properties:
|
||||||
retainOnDelete:
|
|
||||||
description: RetainOnDelete keeps the PVC when the MinecraftServer
|
|
||||||
is deleted.
|
|
||||||
type: boolean
|
|
||||||
size:
|
size:
|
||||||
description: Size is the requested PVC capacity (e.g. "10Gi").
|
description: Size is the requested PVC capacity (e.g. "10Gi").
|
||||||
type: string
|
type: string
|
||||||
|
|||||||
+19
-8
@@ -510,9 +510,8 @@ reaper, but it does **not** auto-stop empty running servers.
|
|||||||
|
|
||||||
## 12. A configuration field seems to be ignored
|
## 12. A configuration field seems to be ignored
|
||||||
|
|
||||||
One CRD field exists and validates but is read by no controller; the rest of
|
Every field below is read by a controller. What varies is the condition that
|
||||||
this table records fields that *are* read, together with the condition that
|
decides whether setting it does anything.
|
||||||
decides whether setting them does anything.
|
|
||||||
|
|
||||||
| Field | What you might expect | Reality |
|
| Field | What you might expect | Reality |
|
||||||
|---|---|---|
|
|---|---|---|
|
||||||
@@ -520,7 +519,6 @@ decides whether setting them does anything.
|
|||||||
| `spec.startup.readinessTimeoutSeconds` | First-probe budget | Read by `readinessTimedOut` (`reconciler.go:490`), called at `:157`. `0` or unset falls back to **300s**, then `markFailed("ReadinessTimeout")`. Not to be confused with the prober's own 5s dial timeout (`prober.go:45`) |
|
| `spec.startup.readinessTimeoutSeconds` | First-probe budget | Read by `readinessTimedOut` (`reconciler.go:490`), called at `:157`. `0` or unset falls back to **300s**, then `markFailed("ReadinessTimeout")`. Not to be confused with the prober's own 5s dial timeout (`prober.go:45`) |
|
||||||
| `spec.idle.autoStopEnabled` | Auto-stop empty servers | Read at `reconciler.go:175` — but gated on `spec.rcon.enabled`, since the player tally comes from the RCON probe (§11) |
|
| `spec.idle.autoStopEnabled` | Auto-stop empty servers | Read at `reconciler.go:175` — but gated on `spec.rcon.enabled`, since the player tally comes from the RCON probe (§11) |
|
||||||
| `spec.idle.emptySecondsBeforeStop` | Empty grace period | Same branch. Must be `> 0`; the guard treats `0` as "off", not "stop immediately" |
|
| `spec.idle.emptySecondsBeforeStop` | Empty grace period | Same branch. Must be `> 0`; the guard treats `0` as "off", not "stop immediately" |
|
||||||
| `spec.storage.retainOnDelete` | Keep/drop PVC on delete | **[INERT]** — world PVCs **always** survive server deletion; only the reaper ever deletes a world PVC (§13) |
|
|
||||||
|
|
||||||
Both startup budgets are measured from the same `status.startRequestedAt`, so
|
Both startup budgets are measured from the same `status.startRequestedAt`, so
|
||||||
`readinessTimeoutSeconds` is not a budget *after* pod readiness — it is a
|
`readinessTimeoutSeconds` is not a budget *after* pod readiness — it is a
|
||||||
@@ -534,16 +532,29 @@ This is expected. The world PVC is a StatefulSet `VolumeClaimTemplate`. There is
|
|||||||
**no `persistentVolumeClaimRetentionPolicy` and no finalizer** anywhere in the
|
**no `persistentVolumeClaimRetentionPolicy` and no finalizer** anywhere in the
|
||||||
operator. Deleting the `MinecraftServer` garbage-collects the StatefulSet, but
|
operator. Deleting the `MinecraftServer` garbage-collects the StatefulSet, but
|
||||||
StatefulSet deletion does **not** cascade to its template PVCs, and nothing else
|
StatefulSet deletion does **not** cascade to its template PVCs, and nothing else
|
||||||
cleans them up. So the world PVC **always survives** server deletion, regardless
|
cleans them up. So the world PVC **always survives** server deletion. The
|
||||||
of `spec.storage.retainOnDelete` ([INERT], §12). The **only** code that deletes a
|
**only** code that deletes a world PVC is the reaper, and only after a verified
|
||||||
world PVC is the reaper, and only after a verified backup (§10). To reclaim a
|
backup (§10). To reclaim a world PVC manually:
|
||||||
world PVC manually:
|
|
||||||
|
|
||||||
```
|
```
|
||||||
kubectl get pvc -l app.kubernetes.io/name=<name>
|
kubectl get pvc -l app.kubernetes.io/name=<name>
|
||||||
kubectl delete pvc <pvc> # irreversible — the world is gone
|
kubectl delete pvc <pvc> # irreversible — the world is gone
|
||||||
```
|
```
|
||||||
|
|
||||||
|
`spec.storage.retainOnDelete` sat in the CRD and reached no controller. Spec
|
||||||
|
v4.1 §5 asks for it — 「删除:finalizer 清 Service/STS/ConfigMap,PVC 按
|
||||||
|
`retainOnDelete`」 — and neither half was ever built: there is no finalizer, and
|
||||||
|
nothing read the field. It was removed rather than implemented, which is a
|
||||||
|
deliberate departure from that line, recorded here because the spec is a frozen
|
||||||
|
document and still says otherwise.
|
||||||
|
|
||||||
|
The reasoning is that implementing it buys a second path that deletes a world —
|
||||||
|
one that skips the reaper's verified-backup check — in order to restore a
|
||||||
|
finalizer whose other listed duties (Service, StatefulSet, ConfigMap)
|
||||||
|
ownerReference GC already performs. A CR still carrying the field keeps working:
|
||||||
|
the API server prunes the unknown key on its next write, and nothing above
|
||||||
|
changes, because retention was never conditional in the first place.
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
## 14. Metrics for diagnosis (spec §23)
|
## 14. Metrics for diagnosis (spec §23)
|
||||||
|
|||||||
@@ -192,8 +192,6 @@ type StorageSpec struct {
|
|||||||
Size string `json:"size,omitempty"`
|
Size string `json:"size,omitempty"`
|
||||||
// StorageClassName selects the StorageClass; empty uses the default.
|
// StorageClassName selects the StorageClass; empty uses the default.
|
||||||
StorageClassName string `json:"storageClassName,omitempty"`
|
StorageClassName string `json:"storageClassName,omitempty"`
|
||||||
// RetainOnDelete keeps the PVC when the MinecraftServer is deleted.
|
|
||||||
RetainOnDelete bool `json:"retainOnDelete,omitempty"`
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// LifecycleSpec tunes graceful shutdown (spec §7). The operator injects a
|
// LifecycleSpec tunes graceful shutdown (spec §7). The operator injects a
|
||||||
|
|||||||
Reference in new issue
Block a user