revert(kubelet): restore image GC thresholds to 70/60 #501

Merged
k.renner merged 1 commits from revert-image-gc-thresholds into master 2026-08-18 19:45:01 +00:00
Member

Reverts the imageGCHighThresholdPercent / imageGCLowThresholdPercent half of #500. The toolbox and the alerts from that PR are untouched and working.

It was measured, and it did nothing

Applied to k8s-2 alone via talosctl patch machineconfig (no reboot required, node stayed Ready, 50 pods running, no image-pull fallout). GC fired exactly as designed:

before after
unique cached images ~89 61
containerd content store 9.93 GB 8.62 GB
/var free 31.49% 31.47%

28 images deleted, ~1.3 GB of blobs freed, and free space did not move.

Why the premise was wrong

/var/lib/containerd is:

io.containerd.snapshotter.v1.overlayfs   20.53 GB
io.containerd.content.v1.content          8.62 GB

#500 assumed the ~19 GB of snapshots would be freed alongside their images at ~2.9x the compressed size. They are not. Those snapshots are unpacked layers shared with running containers, and image GC cannot reclaim them — deleting an unused image frees only its unshared blobs. Reclaimable content is now ~0.7 GB, so further sweeps free nothing.

What this means for MON_DISK_LOW

Nothing — that was the point of the exercise and the answer is that these thresholds are not the lever. Confirmed with the toolbox #500 added:

$ ceph config get mon mon_data_avail_warn
30
$ talosctl -n k8s-2 usage /var/lib/rook
106 MB

A ~106 MB mon database is being judged against a 60 GB filesystem it merely shares with container storage. 70/60 is restored on its own merit as a bound on the image cache, and the comment no longer claims it does anything for Ceph.

The CephMonDiskLow runbook text carried the same wrong claim and now points at finding the actual consumer instead.

Live state

k8s-2's kubelet is already back to 70/60 via talosctl patch — node Ready, Ceph HEALTH_OK, mon quorum a,b,c intact, 3/3 OSDs up. k8s-1 and k8s-3 were never changed; staging on one node is what caught this.

On the mon-volume migration

I looked into mon.volumeClaimTemplate, which I had called the best fix. It is not viable here as things stand, for two independent reasons:

  1. Every StorageClass in this cluster is Ceph-backed (ceph-block, ceph-filesystem, ceph-bucket — all rook-ceph.*.csi.ceph.com). Putting mons on Ceph-backed PVCs is circular: the mons are the cluster's quorum authority, so on a cold start there would be no Ceph to serve the volume the mons need to boot. It would need a node-local provisioner first.
  2. Rook does not migrate existing mons. Upstream docs state volumeClaimTemplate "only applies to new monitors that are created when the requested number of monitors increases, or when a monitor fails and is recreated" — so even with a suitable class it means deliberately failing and recreating mons one at a time.

Remaining options, for a separate decision: grow /var; or set mon_data_avail_warn declaratively (the CephCluster CRD does support a cephConfig map, so this can be GitOps-managed rather than a live ceph config set) on the grounds that the 30% default assumes a dedicated mon filesystem. mon_data_avail_crit stays at 5%, and the node-level CephMonDiskLow alert covers the real risk.

Reverts the `imageGCHighThresholdPercent` / `imageGCLowThresholdPercent` half of #500. The toolbox and the alerts from that PR are untouched and working. ## It was measured, and it did nothing Applied to **k8s-2 alone** via `talosctl patch machineconfig` (no reboot required, node stayed Ready, 50 pods running, no image-pull fallout). GC fired exactly as designed: | | before | after | |---|---|---| | unique cached images | ~89 | **61** | | containerd content store | 9.93 GB | **8.62 GB** | | **/var free** | **31.49%** | **31.47%** | 28 images deleted, ~1.3 GB of blobs freed, and free space did not move. ## Why the premise was wrong `/var/lib/containerd` is: ``` io.containerd.snapshotter.v1.overlayfs 20.53 GB io.containerd.content.v1.content 8.62 GB ``` #500 assumed the ~19 GB of snapshots would be freed alongside their images at ~2.9x the compressed size. They are not. Those snapshots are unpacked layers **shared with running containers**, and image GC cannot reclaim them — deleting an unused image frees only its unshared blobs. Reclaimable content is now ~0.7 GB, so further sweeps free nothing. ## What this means for MON_DISK_LOW Nothing — that was the point of the exercise and the answer is that these thresholds are not the lever. Confirmed with the toolbox #500 added: ``` $ ceph config get mon mon_data_avail_warn 30 $ talosctl -n k8s-2 usage /var/lib/rook 106 MB ``` A ~106 MB mon database is being judged against a 60 GB filesystem it merely shares with container storage. 70/60 is restored on its own merit as a bound on the image cache, and the comment no longer claims it does anything for Ceph. The `CephMonDiskLow` runbook text carried the same wrong claim and now points at finding the actual consumer instead. ## Live state k8s-2's kubelet is **already back to 70/60** via `talosctl patch` — node Ready, Ceph `HEALTH_OK`, mon quorum `a,b,c` intact, 3/3 OSDs up. k8s-1 and k8s-3 were never changed; staging on one node is what caught this. ## On the mon-volume migration I looked into `mon.volumeClaimTemplate`, which I had called the best fix. **It is not viable here as things stand**, for two independent reasons: 1. **Every StorageClass in this cluster is Ceph-backed** (`ceph-block`, `ceph-filesystem`, `ceph-bucket` — all `rook-ceph.*.csi.ceph.com`). Putting mons on Ceph-backed PVCs is circular: the mons *are* the cluster's quorum authority, so on a cold start there would be no Ceph to serve the volume the mons need to boot. It would need a node-local provisioner first. 2. **Rook does not migrate existing mons.** Upstream docs state `volumeClaimTemplate` "only applies to new monitors that are created when the requested number of monitors increases, or when a monitor fails and is recreated" — so even with a suitable class it means deliberately failing and recreating mons one at a time. Remaining options, for a separate decision: grow `/var`; or set `mon_data_avail_warn` declaratively (the CephCluster CRD does support a `cephConfig` map, so this can be GitOps-managed rather than a live `ceph config set`) on the grounds that the 30% default assumes a dedicated mon filesystem. `mon_data_avail_crit` stays at 5%, and the node-level `CephMonDiskLow` alert covers the real risk.
claude-bot added 1 commit 2026-08-18 19:35:59 +00:00
revert(kubelet): restore image GC thresholds to 70/60
ci/woodpecker/pr/ci-scripts-test Pipeline was successful
ci/woodpecker/pr/ci-manifests-test Pipeline was successful
ci/woodpecker/pr/pr-review Pipeline was successful
b7f599b2c1
The 60/50 change in #500 was based on a wrong premise. Reverting the
values and correcting the reasoning recorded around them.

Applied to k8s-2 alone and measured. GC did fire: 28 images deleted,
content store 9.93 -> 8.62 GB. Free space moved 31.49% -> 31.47% — no
effect.

The premise was that ~19 GB of overlayfs snapshots would be reclaimed
along with their images, at roughly 2.9x the compressed size. That is
wrong. /var/lib/containerd is 20.5 GB of snapshots against 8.6 GB of
content, and those snapshots are unpacked layers shared with running
containers. Image GC cannot touch them, so deleting unused image
references frees only their unshared blobs. Remaining reclaimable
content is now ~0.7 GB; further sweeps free nothing.

70/60 stands on its own merits as a bound on the image cache — it is
simply not a lever on MON_DISK_LOW, and the comment no longer claims it
is. The mon's own data is ~106 MB, so a 30%-free threshold on a 60 GB
filesystem it merely shares says nothing about Ceph.

The CephMonDiskLow runbook text carried the same wrong claim and is
corrected to point at finding the real consumer instead.

k8s-2's live kubelet is already back to 70/60 via talosctl patch; node
Ready, Ceph HEALTH_OK with mon quorum a,b,c intact. k8s-1 and k8s-3 were
never changed.

Co-Authored-By: Claude Bot <claude-bot@renner.uno>
review-bot left a comment
Member

🤖 Automated review — model claude-haiku-4-5-20251001 (complexity: trivial), commit b7f599b2c1a331f1f15e1b8d2863eb74ff6d1ca8.

Advisory review by the review-bot pipeline. It never approves; a human merges. Push new commits to get a fresh review.


Summary

This PR reverts Kubelet image GC thresholds (imageGCHighThresholdPercent/imageGCLowThresholdPercent) from 60/50 back to 70/60, along with comprehensive comment updates explaining the reversion. The change is backed by measured evidence showing that the previous configuration did not achieve its intended goal of reducing /var utilization.

🟡 Suggestions

  1. kubernetes/talos/patches/cluster.yaml:39 — The comment explaining the revert is thorough, but it could be tightened slightly. The sentence "Dropping high to 60 deleted 28 images and moved free space by 0.02 percentage points" is precise and good. However, you might consider extracting the key measurement data to a brief inline comment or footnote for future operators, since the current explanation spans 14 lines and the specific numbers (28 images, 0.02%, 8.6 GB, 20 GB) are the evidence that justifies this choice. This makes it easier to reference without re-reading the full block.

  2. kubernetes/apps/monitoring-rules/prometheusrule-ceph.yaml:44-47 — The alert rule comment is clearer than before, but the phrase "Look at what actually grew before reaching for a threshold" might be slightly vague for operators in crisis mode. Consider adding a concrete example: "e.g., overlayfs snapshots, rook logs, other pod data" to guide troubleshooting faster.

🟢 Nits

  • The PR body is exceptionally well-researched and documented. The section headers ("It was measured, and it did nothing", "Why the premise was wrong", etc.) make it easy to follow the reasoning.

Tests

N/A — this is infrastructure configuration. The operator provides evidence of live testing on k8s-2 (node stayed Ready, no image-pull fallout, measured before/after state), which is the appropriate verification for this type of change.

Recommendation

Looks reasonable — no blocking issues; suggestions above are optional refinements to documentation clarity.

🤖 **Automated review** — model `claude-haiku-4-5-20251001` (complexity: trivial), commit `b7f599b2c1a331f1f15e1b8d2863eb74ff6d1ca8`. > Advisory review by the `review-bot` pipeline. It never approves; a human merges. Push new commits to get a fresh review. --- ## Summary This PR reverts Kubelet image GC thresholds (imageGCHighThresholdPercent/imageGCLowThresholdPercent) from 60/50 back to 70/60, along with comprehensive comment updates explaining the reversion. The change is backed by measured evidence showing that the previous configuration did not achieve its intended goal of reducing `/var` utilization. ## 🟡 Suggestions 1. **kubernetes/talos/patches/cluster.yaml:39** — The comment explaining the revert is thorough, but it could be tightened slightly. The sentence "Dropping high to 60 deleted 28 images and moved free space by 0.02 percentage points" is precise and good. However, you might consider extracting the key measurement data to a brief inline comment or footnote for future operators, since the current explanation spans 14 lines and the specific numbers (28 images, 0.02%, 8.6 GB, 20 GB) are the evidence that justifies this choice. This makes it easier to reference without re-reading the full block. 2. **kubernetes/apps/monitoring-rules/prometheusrule-ceph.yaml:44-47** — The alert rule comment is clearer than before, but the phrase "Look at what actually grew before reaching for a threshold" might be slightly vague for operators in crisis mode. Consider adding a concrete example: "e.g., overlayfs snapshots, rook logs, other pod data" to guide troubleshooting faster. ## 🟢 Nits - The PR body is exceptionally well-researched and documented. The section headers ("It was measured, and it did nothing", "Why the premise was wrong", etc.) make it easy to follow the reasoning. ## Tests N/A — this is infrastructure configuration. The operator provides evidence of live testing on k8s-2 (node stayed Ready, no image-pull fallout, measured before/after state), which is the appropriate verification for this type of change. ## Recommendation Looks reasonable — no blocking issues; suggestions above are optional refinements to documentation clarity. <!-- pr-review:sha=b7f599b2c1a331f1f15e1b8d2863eb74ff6d1ca8 -->
k.renner merged commit 96d60e3da7 into master 2026-08-18 19:45:00 +00:00
Sign in to join this conversation.