storage: bounded PVC autoscaling, because the fill-up alert could never fire - #1105
storage: bounded PVC autoscaling, because the fill-up alert could never fire#1105mdheller wants to merge 3 commits into
Conversation
…er fire workspace-minio-pvc hit 100% and took out every push to registry.socioprophet.ai. PR #1091 fixes why it filled (zot kept every tag forever). This fixes why nobody knew. The second half is not "no alert was configured". kube-prometheus-stack ships KubePersistentVolumeFillingUp and it is loaded and healthy in the observability Prometheus right now — state=inactive, health=ok, lastError=none. It queries kubelet_volume_stats_available_bytes{job="kubelet"}. That Prometheus has no kubelet scrape job at all; its job labels are apiserver, kube-state-metrics, node-exporter, otel-collector and the stack's own components. The metric has zero series, so the rule evaluates an empty set forever and sits at inactive no matter what the disks do. On a dashboard that is indistinguishable from "volumes fine". The cause is structural, not a misconfiguration: GKE Autopilot's Warden denies nodes/proxy cluster-wide, so nothing in-cluster can scrape kubelet here. That also rules out every off-the-shelf volume-autoscaler controller — they all read exactly that metric, so one would deploy cleanly, report healthy, and never act, reproducing the failure it was installed to prevent. GKE's managed collection does scrape kubelet and publishes kubelet_volume_stats_* into Google Managed Prometheus over a PromQL endpoint. Verified: 9 PVC-keyed series matching df inside the pods. A CronJob reads that and patches PVCs — no CRDs, no operator, stdlib only. Deliberately the same metric names, so if in-cluster scraping is ever fixed this repoints via PROM_BASE alone. Bounds, because a GCE PD can never be shrunk and every expansion is permanent spend: fire at 75%; step +50% floored at 5Gi and capped at 50Gi per cycle, so a runaway push loop provisions at most 200Gi/hour; a mandatory per-PVC maxGi with no default and no wildcard, so an unconsidered volume is never grown; and a 700Gi estate budget against 402Gi provisioned today as the brake on correlated growth. Every outcome alerts — growing a disk is a symptom, not a success. An empty PromQL result raises rather than reporting all-clear, and an enrolled PVC with no data alerts instead of being skipped: three estate volumes currently publish no stats (mesh-xl-cache is bound but mounted by nothing, arcticdb-gateway-data's pod is in CreateContainerConfigError, workspace-mail-backup has been Pending 12 days), and silently skipping them is the same defect class. postgres-data-pvc is enrolled but can never be grown — static PV, no StorageClass. It is listed so it raises PvcGuardCannotExpand rather than sitting unwatched; the real fix is a migration to dynamic provisioning. Proven by watching it fire, not by reading it. Against a real 1Gi probe volume filled to 82%: grew to 2Gi with FileSystemResizeSuccessful online, then at the 2Gi ceiling and 76% full refused to grow and raised PvcGuardAtMaximum, with both alerts confirmed active in Alertmanager. That run also caught a real bug — patching size and annotations as two requests raced the CSI external-resizer (VolumeResizeFailed: the object has been modified) and left the volume needing a pod restart to finish the filesystem resize. Now a single atomic patch. Alertmanager still has no receivers, so these alerts page nobody yet. That is the next link in the chain and is not addressed here.
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds a Kubernetes CronJob-based “PVC capacity guard” that queries Google Managed Prometheus for kubelet volume usage, then (optionally) expands explicitly-enrolled PVCs within strict per-PVC and estate-wide bounds while emitting Alertmanager alerts for every outcome.
Changes:
- Introduces a
pvc-capacity-guardCronJob (Python stdlib) plus RBAC to list/get/patch PVCs and read StorageClasses. - Adds a generated ConfigMap carrying the guard script and an explicit enrollment/bounds
policy.json. - Wires deployment via an Argo CD
Applicationand documents design, operation, and prerequisites.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| infra/k8s/pvc-capacity-guard/base/rbac.yaml | ServiceAccount + ClusterRole/Binding for reading PVCs/StorageClasses and patching PVCs |
| infra/k8s/pvc-capacity-guard/base/policy.json | Enrollment list + per-PVC ceilings and estate-wide bounds/defaults |
| infra/k8s/pvc-capacity-guard/base/kustomization.yaml | Kustomize base with ConfigMap generator for guard code + policy |
| infra/k8s/pvc-capacity-guard/base/guard.py | Core implementation: GMP PromQL query, decision logic, PVC patching, Alertmanager posting |
| infra/k8s/pvc-capacity-guard/base/cronjob.yaml | Scheduled execution environment, security context, and ConfigMap mounts |
| infra/k8s/pvc-capacity-guard/README.md | Rationale, design constraints, runbook, and prerequisites |
| infra/argocd/pvc-capacity-guard.yaml | Argo CD Application to deploy/sync the guard |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Estate budget is computed over the volumes this guard is allowed to grow. | ||
| provisioned = 0.0 | ||
| for ent in pol["pvcs"]: | ||
| try: | ||
| pvc = get_pvc(base, hdrs, ctx, ent["namespace"], ent["name"]) | ||
| provisioned += parse_gi(pvc["spec"]["resources"]["requests"]["storage"]) | ||
| except Exception: | ||
| pass | ||
| log(f"estate provisioned across guarded PVCs: {provisioned:.0f}Gi / budget {budget_gi}Gi") |
| def alert(name: str, severity: str, summary: str, description: str, extra: dict) -> None: | ||
| payload = [{ | ||
| "labels": dict({"alertname": name, "severity": severity, | ||
| "component": "pvc-capacity-guard"}, **extra), | ||
| "annotations": {"summary": summary, "description": description}, | ||
| "startsAt": datetime.now(timezone.utc).isoformat(), | ||
| }] |
| def sc_expandable(base, hdrs, ctx, sc_name: str | None) -> bool: | ||
| """A PVC with no StorageClass is bound to a static PV and can never be expanded.""" | ||
| if not sc_name: | ||
| return False | ||
| try: | ||
| sc = _http("GET", f"{base}/apis/storage.k8s.io/v1/storageclasses/{sc_name}", | ||
| hdrs, None, ctx) | ||
| return bool(sc.get("allowVolumeExpansion")) | ||
| except urllib.error.HTTPError: | ||
| return False |
| | Alert | Severity | Meaning | | ||
| |---|---|---| | ||
| | `PvcGuardExpanded` | warning | A volume was grown. Permanent spend; find out why it is growing. | | ||
| | `PvcGuardAtMaximum` | critical | Over threshold and at its ceiling. **The guard has stopped.** Raise `maxGi` deliberately or reclaim space. | | ||
| | `PvcGuardCannotExpand` | critical | Over threshold on a volume that can never be expanded (static PV / non-expanding StorageClass). Needs manual migration. | | ||
| | `PvcGuardNoData` | warning | Enrolled but reporting nothing — its consumer pod is not Running, so the kubelet publishes no stats. **The volume is unmonitored.** | | ||
| | `PvcGuardTargetMissing` | warning | Policy and cluster disagree about a PVC's existence. | | ||
| | `PvcGuardBroken` | critical | The run itself failed; no volume was evaluated this cycle. | |
The guard was authored in infra/argocd/. The root app-of-apps
(infra/tofu/environments/gcp-gke/argocd.tf, gitops_path=deploy/argocd,
directory.recurse) has never watched that tree — as the header of
deploy/argocd/observability-services.yaml already records, "nothing under
infra/argocd/ ever reached the cluster."
So the PR as authored deploys nothing: the CronJob that stops PVCs filling
would itself have been a control that exists, reports healthy, and cannot
fire. Same defect class as the KubePersistentVolumeFillingUp alert the PR was
written to replace, one layer up.
Only the Application manifest moves. The kustomize base stays where it was
authored at infra/k8s/pvc-capacity-guard/base — the root app only needs the
Application itself inside the watched tree.
Also brought to the conventions of the live tree:
- socioprophet.io/tier: reference. MANDATORY — tools/preflight_deploy_contract.py
check 5 walks deploy/argocd and fails any Application without it. Verified
both ways: with the annotation the gate exits 0, without it exits 1 naming
this file. reference rather than foundation because the guard is a
replaceable implementation running a digest-pinned stock python image, not
part of the interoperability spine (check 6 would require a foundation
image to be built by images.yml).
- repoURL without the .git suffix, which is the form used throughout
deploy/argocd; the suffixed form survives only in the two dead trees.
- app.kubernetes.io/part-of: platform-ops, matching the label the
kustomization already stamps on the workload.
Relocated into the tree Argo actually watches — pushed to this branch (66cf1dc)Coordination: pushed directly to The findingThe Application landed in
So the PR as authored deployed nothing. A guard against volumes silently filling would itself have been a control that exists, reports healthy, and cannot fire — the same defect class as Proof it is now in the live treeA/B against the same tree, one commit apart, by rendering
Then deployed for real from the branch — not just "the YAML moved": All probe objects deleted afterwards; the cluster is back to matching Also brought to live-tree conventions
Unchanged and still yoursThe Workload Identity GSA + Not verified: CI is at the Actions spend cap (hundreds queued, zero running), so Also worth noting for whoever picks up the alert path: |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (6)
infra/k8s/pvc-capacity-guard/base/guard.py:198
- Any
HTTPError(including transient 5xx or auth issues) is treated as ‘cannot expand’, which can generate a misleadingPvcGuardCannotExpandalert and skip remediation when the real issue is ‘failed to read StorageClass’. Consider only returningFalseon 404 (missing SC) and re-raising (or separately alerting) for other HTTP statuses so failures are surfaced as guard-breakage rather than non-expandability.
def sc_expandable(base, hdrs, ctx, sc_name: str | None) -> bool:
"""A PVC with no StorageClass is bound to a static PV and can never be expanded."""
if not sc_name:
return False
try:
sc = _http("GET", f"{base}/apis/storage.k8s.io/v1/storageclasses/{sc_name}",
hdrs, None, ctx)
return bool(sc.get("allowVolumeExpansion"))
except urllib.error.HTTPError:
return False
infra/k8s/pvc-capacity-guard/base/rbac.yaml:32
- RBAC is broader than the current implementation needs:
guard.pyonly performs per-object GETs (no list calls) and patches specific PVCs, and it only GETs StorageClasses by name. To reduce blast radius, remove thelistverbs. Additionally (optional but stronger), consider replacing the cluster-wide patch permission with per-namespace Roles/RoleBindings (andresourceNamesrestrictions where practical) for the explicitly enrolled PVCs, so the ServiceAccount can’t patch arbitrary PVCs elsewhere in the cluster.
rules:
- apiGroups: [""]
resources: ["persistentvolumeclaims"]
verbs: ["get", "list", "patch"]
- apiGroups: ["storage.k8s.io"]
resources: ["storageclasses"]
verbs: ["get", "list"]
infra/k8s/pvc-capacity-guard/README.md:65
- The Markdown table uses
|| ...row prefixes, which won’t render as a GitHub table. Use a standard pipe table format (| ... | ... |) so the options/verdicts render correctly.
| Option | Verdict |
|---|---|
| A volume-autoscaler controller (e.g. DevOps-Nirvana/kubernetes-volume-autoscaler) | **Rejected.** Every one of them reads `kubelet_volume_stats_*` from an in-cluster Prometheus. On Autopilot that metric does not exist (above), so the controller would deploy cleanly, report healthy, and never act — reproducing the exact failure mode we are fixing. Making it work would mean first getting kubelet scraping past Warden, which is not in our gift. |
| A CronJob reading usage and patching the PVC | **Chosen.** ~250 lines of stdlib Python, no CRDs, no operator lifecycle, no new failure domain. The reconcile loop for "resize a disk occasionally" does not need to be level-triggered — disks fill over hours, not milliseconds. |
infra/k8s/pvc-capacity-guard/README.md:118
- The README’s Python snippet is syntactically invalid as shown (a quoted string is split across lines). Update the snippet to match the real implementation (e.g., adjacent-string concatenation, triple-quoted string, or explicit newline escapes) so readers can copy/paste it without errors.
infra/k8s/pvc-capacity-guard/README.md:112 - The implementation emits an additional alert (
PvcGuardBudgetExhausted) when an expansion would exceedestateMaxTotalGi, but it isn’t documented in the README’s alert table (and the table formatting itself is currently broken as noted above). AddPvcGuardBudgetExhaustedto the documented alert list so operators know what it means and how to respond.
| Alert | Severity | Meaning |
|---|---|---|
| `PvcGuardExpanded` | warning | A volume was grown. Permanent spend; find out why it is growing. |
| `PvcGuardAtMaximum` | critical | Over threshold and at its ceiling. **The guard has stopped.** Raise `maxGi` deliberately or reclaim space. |
| `PvcGuardCannotExpand` | critical | Over threshold on a volume that can never be expanded (static PV / non-expanding StorageClass). Needs manual migration. |
| `PvcGuardNoData` | warning | Enrolled but reporting nothing — its consumer pod is not Running, so the kubelet publishes no stats. **The volume is unmonitored.** |
| `PvcGuardTargetMissing` | warning | Policy and cluster disagree about a PVC's existence. |
| `PvcGuardBroken` | critical | The run itself failed; no volume was evaluated this cycle. |
infra/k8s/pvc-capacity-guard/base/guard.py:139
- The PromQL query is unscoped, but the GMP endpoint is project-wide (
.../projects/<project>/.../prometheus). If multiple clusters feed metrics into the same project, this query can return series from other clusters and thenamespace/pvckey can collide/overwrite, leading to incorrect decisions in this cluster. Prefer adding label matchers to scope to the current cluster/location (passed via env vars) and include those labels in the key used for mapping.
rows = promql(
"kubelet_volume_stats_used_bytes / kubelet_volume_stats_capacity_bytes"
)
| # Estate budget is computed over the volumes this guard is allowed to grow. | ||
| provisioned = 0.0 | ||
| for ent in pol["pvcs"]: | ||
| try: | ||
| pvc = get_pvc(base, hdrs, ctx, ent["namespace"], ent["name"]) | ||
| provisioned += parse_gi(pvc["spec"]["resources"]["requests"]["storage"]) | ||
| except Exception: | ||
| pass | ||
| log(f"estate provisioned across guarded PVCs: {provisioned:.0f}Gi / budget {budget_gi}Gi") |
|
| if age < cooldown_s: | ||
| log(f" cooldown: grew {age / 60:.0f}m ago (<{cooldown_s / 60:.0f}m), skipping") | ||
| continue | ||
| except ValueError: |
| alert("PvcGuardBroken", "critical", "pvc-capacity-guard run failed", | ||
| f"{type(exc).__name__}: {exc}. No volume was evaluated this cycle — " | ||
| "the estate is running without capacity protection.", {}) | ||
| except Exception: |
| def main() -> int: | ||
| pol = load_policy() | ||
| d = pol.get("defaults", {}) | ||
| thresh = d.get("thresholdPct", 75) / 100.0 |
…o this cluster Two fixes to a control that could report headroom it cannot see: 1. The estate-budget accumulator did `except Exception: pass`, so an unreadable enrolled PVC was silently counted as 0Gi. That undercounts provisioned, and the aggregate brake (provisioned - cur + new > budget) would then permit a grow that actually breaches estateMaxTotalGi. Now a failed read makes the estate total UNKNOWN, raises PvcGuardBudgetUnknown, and refuses every grow this cycle — the same fail-closed posture utilisation() already takes on an empty result. 2. The utilisation query hit Google Managed Prometheus (project-wide) unscoped, so a second cluster's kubelet_volume_stats_* collide on namespace/pvc and this guard — which PATCHES disks — could resize a volume off another cluster's fill level. The query is now pinned to cluster (+location), read from env or the GKE metadata server; an undeterminable scope fails closed rather than sweeping the project. Adds a stdlib pytest proving a simulated read error near budget now refuses (was: grew) and that the query carries the cluster matchers.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (5)
infra/k8s/pvc-capacity-guard/base/guard.py:141
- This allows running with only
cluster=...whenlocationcan’t be determined. Because GKE cluster identity is(project, location, name), omittinglocationreintroduces the cross-cluster collision hazard the PR is trying to eliminate (a same-named cluster in another location could match and drive resizes off foreign data). Consider failing closed unless both cluster name and location are known, and update/add a test case for the 'cluster known but location missing' branch.
cluster = CLUSTER_NAME or _metadata_attr("cluster-name")
if not cluster:
return ""
location = CLUSTER_LOCATION or _metadata_attr("cluster-location")
matchers = ['cluster="%s"' % cluster]
if location:
matchers.append('location="%s"' % location)
return "{%s}" % ", ".join(matchers)
infra/k8s/pvc-capacity-guard/base/guard.py:310
- Treating all exceptions as ‘budget unknown’ means a single stale policy entry (e.g. a 404 for a deleted/renamed PVC) will block all growth for the entire estate, even though the budget could still be computed from the remaining readable PVCs. Consider handling 404 (and possibly other non-transient ‘does not exist’ cases) separately: alert
PvcGuardTargetMissing, but don’t forcebudget_known=Falsefor that case.
for ent in pol["pvcs"]:
try:
pvc = get_pvc(base, hdrs, ctx, ent["namespace"], ent["name"])
provisioned += parse_gi(pvc["spec"]["resources"]["requests"]["storage"])
except Exception as exc: # noqa: BLE001 - any read failure blinds the total
budget_known = False
budget_blind.append(f"{ent['namespace']}/{ent['name']} ({type(exc).__name__})")
infra/k8s/pvc-capacity-guard/README.md:118
- This code sample is not valid Python: the string literal is broken across lines without proper continuation, which makes the example misleading for readers. Keep the message on one line, or show a valid multi-line string form (e.g. implicit concatenation or triple-quoted string) so the snippet is syntactically correct.
infra/k8s/pvc-capacity-guard/README.md:65 - The table markup has an extra leading
|on each row (|| ...), which renders as an unintended empty first column in GitHub-flavored Markdown. Use a single leading pipe per row (| Option | Verdict |) to render a standard 2-column table.
| Option | Verdict |
|---|---|
| A volume-autoscaler controller (e.g. DevOps-Nirvana/kubernetes-volume-autoscaler) | **Rejected.** Every one of them reads `kubelet_volume_stats_*` from an in-cluster Prometheus. On Autopilot that metric does not exist (above), so the controller would deploy cleanly, report healthy, and never act — reproducing the exact failure mode we are fixing. Making it work would mean first getting kubelet scraping past Warden, which is not in our gift. |
| A CronJob reading usage and patching the PVC | **Chosen.** ~250 lines of stdlib Python, no CRDs, no operator lifecycle, no new failure domain. The reconcile loop for "resize a disk occasionally" does not need to be level-triggered — disks fill over hours, not milliseconds. |
infra/k8s/pvc-capacity-guard/base/guard.py:379
- The cooldown behavior (including the invalid/parse-failure annotation path) is newly introduced/critical to preventing double-growth, but it isn’t exercised by the current tests. Add unit tests that cover: (1) a recent
last-grownskips growth, (2) an oldlast-grownallows growth, and (3) a malformed timestamp doesn’t crash/doesn’t incorrectly skip.
# -- cooldown: PD resize + filesystem resize is not instantaneous ---------------
last = annos.get("capacity-guard.socioprophet.ai/last-grown")
if last:
try:
age = time.time() - datetime.fromisoformat(last).timestamp()
if age < cooldown_s:
log(f" cooldown: grew {age / 60:.0f}m ago (<{cooldown_s / 60:.0f}m), skipping")
continue
except ValueError:
pass
| volumeMounts: | ||
| - name: guard | ||
| mountPath: /opt/guard/guard.py | ||
| subPath: guard.py | ||
| readOnly: true | ||
| - name: guard | ||
| mountPath: /etc/guard/policy.json | ||
| subPath: policy.json | ||
| readOnly: true |
|
Merge-gate disposition (Copilot 3-channel + adversarial pass) FIXED in
Acknowledged, non-blocking: Verdict: MERGE-READY after the fixes above. |
What
A CronJob that grows estate PVCs before they fill, with hard bounds and an alert on every outcome.
Companion to #1091. That PR fixes why
workspace-minio-pvcfilled (zot kept every tag forever). This fixes why nobody knew — and stops it recurring on the other eleven volumes.The finding that shaped the design
The estate was not missing a disk-fullness alert. It had one, and it was incapable of firing.
KubePersistentVolumeFillingUpis loaded and healthy in theobservabilityPrometheus right now:Its query is
kubelet_volume_stats_available_bytes{job="kubelet",...} / kubelet_volume_stats_capacity_bytes{...} < 0.03.That Prometheus has no
kubeletscrape job. Label values forjobareapiserver,kube-state-metrics,node-exporter,otel-collector, and the stack's own components. Both metrics return 0 series. The rule evaluates an empty set forever and reportsinactiveregardless of what the disks are doing — indistinguishable, on every dashboard, from "the volumes are fine".It is structural, not a misconfiguration. GKE Autopilot's Warden denies
nodes/proxycluster-wide:Why a CronJob and not a controller
kubelet_volume_stats_*from an in-cluster Prometheus. On Autopilot that metric does not exist, so the controller would deploy cleanly, report healthy, and never act — reproducing the exact failure mode being fixed.Signal comes from GKE's managed collection, which does scrape kubelet and publishes
kubelet_volume_stats_*into Google Managed Prometheus over a PromQL endpoint. Verified: 9 PVC-keyed series matchingdfinside the pods. Deliberately the same metric names upstream uses, so if in-cluster scraping is ever fixed this repoints viaPROM_BASEalone.Bounds
A GCE PD can never be shrunk, so every expansion is permanent spend.
thresholdPctmaxGiestateMaxTotalGi1.75x ($70/mo pd-balanced).cooldownMinutesIt alerts rather than silently absorbing
PvcGuardExpanded·PvcGuardAtMaximum·PvcGuardCannotExpand·PvcGuardNoData·PvcGuardTargetMissing·PvcGuardBrokenAn empty PromQL result raises instead of reporting all-clear, and an enrolled PVC with no data alerts instead of being skipped — silently skipping the unobservable is the same defect class as the alert above.
Proof it fires
Against a real 1Gi probe volume filled to 82%, ceiling set to 2Gi:
Refilled to 76% of the new 2Gi, now at its ceiling:
Both confirmed live in Alertmanager, not just logged:
Probe PVC and pod deleted afterwards.
That run caught a real bug. Patching size then annotations as two requests races the CSI external-resizer:
The disk still grew, but the interrupted handshake left the volume needing a pod restart to finish the filesystem resize. Now a single atomic patch — and the re-run shows
FileSystemResizeSuccessfulwith no restart.The shipped policy also dry-run against the live estate: 402Gi/700Gi, 12 PVCs evaluated, 3 blind spots correctly alerted.
PvcGuardCannotExpandexercised viaFORCE_UTIL.At-risk volumes this surfaces
postgres-data-pvc— static PV, no StorageClass, cannot be expanded by any mechanism. Enrolled so it alerts rather than sitting unwatched; needs manual migration.workspace-mail-backup— Pending 12 days, and the highest latent risk in the estate:mail-backupwrites a new timestamped full tarball every run and deletes nothing. Fix retention before fixing scheduling.mesh-xl-cache— 120Gi, Bound,Used By: <none>. Provisioned and paid for, mounted by nothing.arcticdb-gateway-data— pod inCreateContainerConfigError, so no stats published.clickhouse-data— DDL partitions by month across 7 tables with noTTLanywhere.Not addressed here
Alertmanager has no receivers configured, so these alerts land and page nobody. That is the next link and deliberately out of scope.
Workload Identity GSA +
roles/monitoring.vieweris a project-level IAM change, documented in the README, not created by this PR. Until it exists the CronJob fails loudly withPvcGuardBroken.Coordination
Touches only new paths (
infra/k8s/pvc-capacity-guard/**,infra/argocd/pvc-capacity-guard.yaml). No overlap with #1091,images.yml,infra/k8s/zot/**,gitops-promote.yml, orvalidate-target-diagnostics.yml.🤖 Generated with Claude Code