feat: expose thin-pool chunk_size, metadata size, and zeroing as StorageClass params

[abonillabeeche]

Summary

The driver previously ran lvcreate --thinpool with only -l90%FREE, letting LVM auto-select all pool creation parameters. LVM's generic chunk_size policy scales chunk_size with pool size to keep metadata bounded — so multi-TB pools land on 8-16 MiB chunks by design. On random 4K workloads this produces up to a 4096:1 allocation write-amp — severe enough that a field customer measured 326 random 4K IOPS on hardware capable of at least 10x that.

This PR exposes three new StorageClass parameters honored at first-time pool creation and ships sensible defaults through the chart.

Changes

Driver (pkg/lvm, cmd/provisioner):

• CreateLVS signature extended with chunkSize, poolMetadataSize, zeroBlocks string.
• New helper buildThinPoolCreateArgs assembles the lvcreate line and omits any flag whose value is the empty string (preserves backwards compat).
• controllerserver.go extracts the three new parameters from StorageClass.parameters.
• cmd/provisioner/createlv.go grows three matching CLI flags.
• cmd/provisioner/clonelv.go passes empty strings (thin pool always exists at clone time, so pool-creation flags are moot).

Chart (deploy/charts):

• New templates/storageclass.yaml renders the storageClasses: values block (previously declared but never rendered).
• Two SCs shipped by default:
  • harvester-lvm-thin — type: dm-thin, with chunkSize: 1M, poolMetadataSize: 16G, zeroBlocks: false
  • harvester-lvm-striped — type: striped, no thin-pool params
• Legacy linear/striped/mirror keys retained (disabled by default) for backwards compat.

Example (examples/storageclass-dm-thin.yaml):

• Documents the three new parameters inline with tuning guidance.

Tests (pkg/lvm/lvm_test.go, new):

• Table-driven tests for buildThinPoolCreateArgs covering empty (auto-select), full (chart defaults), case-insensitive zeroBlocks, and edge cases. 7 subtests, all pass.

Why 1M default

Harvester runs predominantly on hardware-RAID-backed volume groups, where the dominant factor is stripe alignment, not the general-purpose seed. A chunk equal to the RAID full-stripe width — (number of data disks) × (strip size) — makes every first-touch allocation a whole-stripe write. A sub-stripe chunk (for example 512K on a 1 MiB stripe) forces partial-stripe read-modify-write and, on parity RAID (5/6), widens the write-hole window, so an unclean shutdown without a protected controller cache (BBU/FBWC) can leave a stripe with inconsistent parity.

1M aligns with the most common power-of-two layouts (e.g. 4 data disks × 256 KiB strip = 1 MiB stripe), matches the fixed 1 MiB block used by mainstream hypervisor stacks, and stays under the 2 MiB CoW cap:

Source	Block / chunk
VMware VMFS-5/6	1 MiB (fixed)
Nutanix AHV	1 MiB vBlock (fixed)
LVM upstream performance policy	512 KiB seed (general-purpose, RAID-agnostic)
Linux kernel dm-thin admin guide	"512 KiB general; 64 KiB snapshot-heavy"
Red Hat Gluster admin guide	cap at 2 MiB "to reduce COW problems"

Operators whose RAID full stripe differs (e.g. a 3-data-disk RAID5 = 768 KiB, or a single-disk / non-RAID VG where alignment does not apply) should override chunkSize to match; snapshot-heavy tenants can drop to 128K.

Why 16G metadata default

Metadata bytes per chunk ≈ 64. So metadata_size ≈ 64 × (pool_size / chunk_size). At 1M chunks:

Pool size	Metadata needed	Fits in 16G?
64 TB	~4 GB	✅
128 TB	~8 GB	✅
256 TB	~16 GB	✅ (at limit)

A larger chunk needs less metadata to address the same capacity, so moving from 512K to 1M doubles the pool size that 16G covers — the entire practical Harvester deployment range with headroom.

Why zeroBlocks: false default

Adds --zero n at pool creation, disabling per-chunk zero-write on allocation. On single-tenant Harvester nodes this halves the write-amp on first-touch to unallocated regions. Multi-tenant clusters that care about inter-tenant data-leak isolation can opt in with zeroBlocks: true.

Backwards compatibility

• Empty parameter values preserve current LVM auto-select behavior (no behavior change for callers who don't opt in).
• Existing thin pools are unaffected — chunk_size cannot be changed after pool creation.
• The three new fields are additive on the CLI, in the volumeAction struct, and in the StorageClass params — no removal or rename.

Test plan

• Unit tests pass (go test ./pkg/lvm/... -run TestBuildThinPoolCreateArgs -v)
• go build ./... clean
• go vet ./pkg/lvm/... ./cmd/provisioner/... clean
• helm lint deploy/charts clean
• helm template deploy/charts renders both SCs correctly with parameters
• Existing bats integration suite (tests/bats/) — maintainers to verify on Linux CI (requires a real LVM device, cannot be run from macOS dev env)
• End-to-end on a Harvester cluster: create fresh VG → provision PVC → confirm dmsetup table <vg>-thinpool-tpool shows skip_block_zeroing and 1M chunk (2048 sectors)

Field evidence

Customer running everything from our runbook ended up with a 16 MiB chunk_size thin pool (LVM auto-select). Diskspd on comparable KubeVirt+VMDP+LVM hardware in our lab achieves 15,000+ QD1 4K IOPS with an explicit chunk size and zero=n; the customer's auto-selected pool measured 326 IOPS on the same workload class. The fix is setting an explicit, stripe-aligned chunk size at pool creation.

Related: harvester/harvester#11128 (LVM CSI cross-driver panic — separate bug, but same field investigation surfaced both).

---

🤖 Generated with Claude Code

[abonillabeeche]

@WebberHuang1118 is this PR covered in your #64 ? Let me know if I need to rebase.

[WebberHuang1118]

Thanks for checking. After further review, I don’t think the approach in this PR is suitable. The dm-thin-pool setting applies across all LVs that share the same thin pool, rather than to an individual LV. Additionally, the thin pool is immutable after creation. For both reasons, this setting should not be propagated through a StorageClass.

Therefore, #64 will not include thin-pool control. We’ll address this separately as a future enhancement in harvester/harvester#11335.

No need to rebase for now. Thanks!

[mergify]

This pull request is now in conflict. Could you fix it @abonillabeeche? 🙏

[abonillabeeche]

Re the conflict ping: this PR is parked behind harvester/harvester#11335, per @WebberHuang1118's review above — thin-pool geometry belongs to the pool, not to a StorageClass, and it is immutable after creation, so a per-SC parameter is the wrong surface.

Rather than let the reasoning get lost, I have carried the field data over to that enhancement (harvester/harvester#11335 (comment)): the 4096:1 allocation write-amp from auto-selected 8–16 MiB chunks on multi-TB pools, the customer's 326 random-4K IOPS, the case for a 1 MiB default, and the two API implications (spec only applies at first pool creation; observed chunk size needs to be reported in status, with a drift condition for pools that cannot be converted).

Not rebasing, per your "no need to rebase for now" above — this comment is just to preserve the data before the PR is closed out as superseded by #11335. harvester/docs#1083 documents the tuning for users in the meantime.