Skip to content

feat: add lblk (non-NVMe block device) support for storage nodes - #437

Draft
boddumanohar wants to merge 15 commits into
mainfrom
feat/lblk-block-device-support
Draft

feat: add lblk (non-NVMe block device) support for storage nodes#437
boddumanohar wants to merge 15 commits into
mainfrom
feat/lblk-block-device-support

Conversation

@boddumanohar

@boddumanohar boddumanohar commented Aug 17, 2026

Copy link
Copy Markdown
Member

Summary

Adds operator support for sbcli's lblk device mode (simplyblock/sbcli#1224, not yet merged), which lets storage nodes run on generic Linux block devices (SPDK AIO bdevs) instead of requiring physical NVMe hardware — cloud block volumes, virtio-blk disks, non-NVMe local disks.

Closes/addresses #436.

  • StorageCluster.spec.deviceMode (nvme|lblk, default nvme, immutable): cluster-wide choice, threaded into the POST /api/v2/clusters/ create payload as device_mode.
  • StorageNodeSet.spec (+ per-node nodeConfigs override, + StorageNode.spec.overrides): enableLblk, blkNames, blkNamesExclude, blkSerials, lblkJournalPercent — mirrors the existing pcieAllowList/pcieDenyList/deviceNames selector pattern.
  • These per-node fields flow into the per-node ConfigMap (buildPerNodeEnvFile) as new env vars (LBLK, BLK_NAMES, BLK_NAMES_EXCLUDE, BLK_SERIALS, LBLK_JM_PERCENT), which the DaemonSet init container already sources and turns into node_configure.py flags — extended with --lblk/--blk-names/--blk-names-exclude/--blk-serials/--jm-percent, matching the existing PCI_ALLOWED/--pci-allowed wiring exactly.
  • CRDs/deepcopy/Helm chart CRDs/dist/install.yaml regenerated via make generate manifests helm-sync and make -C operator build-installer.

Why this shape

sbcli#1224 does not expose device selection through the "add storage node" REST API — only a single force_format field was added there. Device selection happens at the node-local sn configure --lblk/node_configure.py step (already has a k8s=True path), and the cluster-wide mode is set once at cluster create. That's exactly the mechanism this operator's DaemonSet init container already drives for NVMe PCIe selection (PCI_ALLOWED etc. → node_configure.py), so lblk selection is added as more of the same rather than a new integration point.

Known gaps (v1, flagged for follow-up)

  • No webhook validation yet for mutual exclusivity (lblk selectors vs. NVMe PCIe selectors) — an invalid combination is currently caught by node_configure.py's own argparse validation, surfacing as an init-container CrashLoopBackOff rather than a clean CR-admission rejection.
  • No cross-check that StorageNodeSet's lblk fields are only meaningful when the parent StorageCluster.spec.deviceMode == "lblk" — currently just documented in field comments.
  • Untested against sbcli#1224 end-to-end (that PR isn't merged); this PR is draft specifically because the backend dependency isn't landed yet.

Test plan

  • go build ./...
  • go vet ./..., golangci-lint run on touched packages — 0 issues
  • go test ./... — all green, including new unit tests:
    • TestBuildPerNodeEnvFile_LblkFleetDefaults / _LblkOverrideWinsOverFleet / updated _ContainsAllRequiredKeys
    • TestStorageClusterReconcileCreationSendsDeviceMode (sends lblk, omits by default)
  • End-to-end against a cluster running sbcli#1224 once that PR merges — blocked on the backend dependency, tracked as follow-up.

🤖 Generated with Claude Code

@boddumanohar
boddumanohar force-pushed the feat/lblk-block-device-support branch 2 times, most recently from 63b7682 to d2953d2 Compare August 19, 2026 13:08
boddumanohar added a commit that referenced this pull request Aug 19, 2026
@boddumanohar @claude
feat(storagenodeset): wire --force-format through for lblk partitione…
c54c8e5 
…d-disk reuse

blkForceFormat (StorageNodeSet fleet-wide + per-node override) threads
through two places lblk's --force-format actually matters:
- node_configure.py's --force-format flag (marks a partitioned disk
  eligible at configure time)
- the add-node API's force_format param (triggers the actual wipefs at
  add-node time — configure-time eligibility alone doesn't wipe anything)

Found while working through the lblk-e2e-testing.md test plan: PR #437
never wired this at all, so partitioned-disk reuse and the
disk+own-partition-conflict test were both unreachable through the CRD.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@noctarius noctarius added this to the 26.4 milestone Aug 20, 2026
boddumanohar and others added 14 commits August 21, 2026 13:42
@boddumanohar
feat: add lblk (non-NVMe block device) support for storage nodes
43c329c 
Wires the sbcli lblk device mode (simplyblock/sbcli#1224, not yet merged)
through the operator: a cluster-wide StorageCluster.spec.deviceMode
("nvme"|"lblk") passed to cluster-create, and per-node/fleet block-device
selectors (enableLblk, blkNames, blkNamesExclude, blkSerials,
lblkJournalPercent) on StorageNodeSet/StorageNode that flow into the
per-node ConfigMap and from there into the DaemonSet init container's
node_configure.py invocation (--lblk/--blk-names/--blk-names-exclude/
--blk-serials/--jm-percent), mirroring the existing PCIe selector wiring.

Lets storage nodes run on cloud block volumes, virtio-blk disks, or other
non-NVMe local disks instead of requiring physical NVMe hardware.
Addresses #436.

Depends on sbcli#1224 landing before device_mode=lblk/the lblk configure
flags are recognized by the backend; sending them against an unpatched
backend fails at cluster-create/node-configure time.
@boddumanohar @claude
fix(storagenodeset): mount /dev and /sys into the config-generator in…
518c7c1 
…it container

node_configure.py's lblk eligibility check (--lblk/--blk-names/--blk-serials)
inspects host block devices eagerly at config-generation time, unlike NVMe
PCIe discovery which defers to the main container's SPDK startup. Without
/dev and /sys mounted, every lblk device lookup failed with "not present"
regardless of what was actually attached to the host — found live while
testing device_mode=lblk against sbcli#1224 on a real cluster.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@boddumanohar
chore: regenerate CRDs/install manifest after rebase onto main
519cb61
@boddumanohar @claude
feat(storagenodeset): wire --force-format through for lblk partitione…
db9dc5c 
…d-disk reuse

blkForceFormat (StorageNodeSet fleet-wide + per-node override) threads
through two places lblk's --force-format actually matters:
- node_configure.py's --force-format flag (marks a partitioned disk
  eligible at configure time)
- the add-node API's force_format param (triggers the actual wipefs at
  add-node time — configure-time eligibility alone doesn't wipe anything)

Found while working through the lblk-e2e-testing.md test plan: PR #437
never wired this at all, so partitioned-disk reuse and the
disk+own-partition-conflict test were both unreachable through the CRD.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@boddumanohar @claude
fix(storagenode): finalize deletion even when parent StorageNodeSet i…
87f456d 
…s gone

Reconcile fetched the parent StorageNodeSet before checking DeletionTimestamp,
so a StorageNode whose parent had already been deleted (cascading delete
racing ahead of its children) could never reach handleDeletion and got stuck
requeuing "parent StorageNodeSet not found" forever. handleDeletion never
used the parent, so check deletion first.

Reproduced live during config-israel lblk cluster teardown: a leftover
StorageNode from an earlier test was permanently stuck this way.

Also add lblk e2e test plan and findings notes from testing on config-israel.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015pjXzhB8JQ6ySSm1vvwZq7
@boddumanohar @claude
fix(lblk): enforce blkNames/blkSerials immutability and lblk/PCIe mut…
f7e313e 
…ual exclusivity at admission time

Two real gaps found while working through the lblk e2e test checklist on the
GCP lblk cluster:

1. BlkNames/BlkNamesExclude/BlkSerials had no immutability guard at all, unlike
   EnableLblk. Adding a naive `self == oldSelf` rule (matching EnableLblk's
   existing pattern) doesn't catch the unset-to-set transition — Kubernetes
   skips CEL validation on optional fields when the old value was absent, by
   design. That's exactly the scenario that matters here: a StorageNodeSet
   created with no selector (auto-select) that later gets blkNames added via
   edit. Fixed using optionalOldSelf:true with an explicit
   oldSelf.hasValue()-based rule, on both StorageNodeSetSpec and
   StorageNodeOverrides.

2. enableLblk/blkNames/blkNamesExclude/blkSerials selectors and the NVMe PCIe
   selectors (pcieAllowList/pcieDenyList/pcieModel/driveSizeRange/deviceNames)
   were mutually exclusive only by backend convention — nothing at the CRD
   level enforced it, so setting both crashed the pod (CrashLoopBackOff)
   instead of failing at admission time. Fixed with an object-level CEL rule
   on both structs; a webhook wasn't needed since this is a same-object,
   multi-field check CEL can express directly, and it covers create+update
   for free.

Needed +kubebuilder:validation:MaxItems=64 on the per-node-override string
slices too, since the nested nodeConfigs map's unbounded CEL cost estimate
otherwise exceeds the API server's rule-cost budget.

Both fixes verified empirically end-to-end against a live GCP lblk cluster
(unset->set rejected, set->different rejected, set->same-value no-op allowed,
lblk+pcie combo rejected, lblk-only/pcie-only still succeed).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015pjXzhB8JQ6ySSm1vvwZq7
@boddumanohar @claude
fix(storagenodeset): shorten storage-node-sa token expiration to self…
8c9cf18 
…-heal after SA recreation

The storage-node-ds DaemonSet relied on Kubernetes' default auto-injected
ServiceAccount token, which defaults to a ~1 hour expiration (kubelet refreshes
at ~80% of that, so ~48 minutes). We hit this directly this session: the
simplyblock-storage-node-sa ServiceAccount got deleted and recreated (exact
trigger not conclusively root-caused — suspected informer-cache resync race
under rapid CRD schema updates), and every already-running pod's mounted token
stayed bound to the old, now-nonexistent ServiceAccount UID. The API server
correctly rejects those with 401 Unauthorized, and nothing self-heals until
kubelet's next refresh — in practice this read as a long, manual-intervention
outage (had to force-delete pods to get fresh tokens).

Mount an explicit short-lived (10 minute) projected ServiceAccountToken volume
instead of relying on the default auto-injected one, with
AutomountServiceAccountToken: false on the pod spec to avoid a duplicate mount
at the same default path. This doesn't prevent a ServiceAccount recreation
event, but bounds its blast radius to a few minutes of self-healing via
kubelet's normal refresh cycle instead of requiring manual pod deletion.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015pjXzhB8JQ6ySSm1vvwZq7
@boddumanohar @claude
docs(lblk-testing): document cluster-expansion gap and SA token fix
eb06f9c 
Real, significant finding from live GCP testing: adding exactly one new
node to an already-active cluster leaves it permanently stuck oscillating
between IN_ACTIVATION and SUSPENDED. sbcli's cluster_expand() requires a
minimum of 2 new nodes (they pair with each other as HA secondaries), and
separately the operator has no code path that calls cluster complete-expand
after add-node at all. Recovery required manually sn remove + sn delete the
new node, then cluster activate.

Also confirms the pre-existing lvol survived fully intact throughout, and
notes commit 83b2ece's SA-token-expiration fix.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015pjXzhB8JQ6ySSm1vvwZq7
@boddumanohar @claude
docs(lblk-testing): device/JM failure, ghost-task, and GPT/MBR findings
4dbdabf 
Documents five real gaps found while continuing the Error Cases and
Compatibility checklist against the GCP lblk cluster and config-israel:

- restart-device fails to recover an lblk device after the kernel
  renames its backing block device (stale bdev_aio survives teardown)
- JM (journal) device failures are not proactively detected the way
  storage device failures are, despite reaching the data path as real
  I/O errors
- leftover failed_device_migration tasks from an earlier node removal
  retry forever against devices confirmed deleted from the DB
- partition-table-type detection (lsblk PTTYPE) silently breaks without
  /run/udev mounted, which the real production DaemonSet doesn't mount
- once detection is fixed, the GPT journal-split path has a sector-size
  unit mismatch that corrupts partition boundaries on 4Kn disks

Also confirms the "device disappears" storage-device case degrades
gracefully, and MBR is correctly rejected once detection works.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015pjXzhB8JQ6ySSm1vvwZq7
@boddumanohar @claude
docs(lblk-testing): document migration-task-scheduler circular-wait d…
679a635 
…eadlock

Found while continuing Error Cases testing: device_migration and
new_device_migration task families can deadlock each other when both
target the same node+distrib (SUSPENDED new_device_migration blocks
device_migration; device_migration's mere existence blocks
new_device_migration). Reproduced twice, stalled the whole cluster in
ACTIVE - REBALANCING with all nodes unhealthy for 2+ hours. Also notes
a JM-device recovery gap missed during the earlier device-disappears
test (own oversight, fixed via restart-jm-device).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015pjXzhB8JQ6ySSm1vvwZq7
@boddumanohar @claude
docs(lblk-testing): complete remaining Error Cases — SPDK crash and m…
02a5368 
…esh-hole findings

Completes the last two untested Error Cases:

- SPDK process crash mid add-node: opposite of expected behavior. The
  bdts process's wrapper exits cleanly (code 0) instead of relaunching
  it, restartPolicy: Never means Kubernetes never restarts the
  container, and the operator polls forever with no remediation. The
  node gets permanently stuck in_creation with no CLI-level recovery
  path (remove/delete both refuse it) — required a raw DB write to
  escape. Hugepages are confirmed genuinely freed at the OS level once
  the pod is force-deleted, so it's a stale DB figure, not a literal
  leak.

- Add-node retry leaves JM-mesh holes: confirmed via incidental
  evidence from repeated add/remove/delete cycling — all 3 surviving
  nodes carry a permanent stale failed remote-device bdev reference to
  a node/device that no longer exists anywhere in the DB. Node removal
  cleans up the removed node's own stack but never the peers'
  references to it.

Cluster restored to ACTIVE/healthy afterward (cluster activate +
manual DB cleanup of the wedged test node). All originally-listed
Error Cases are now tested.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015pjXzhB8JQ6ySSm1vvwZq7
@boddumanohar @claude
docs(lblk-testing): note Hung IO test attempt, blocked by tooling con…
d8d584e 
…straints

Attempted the Hung IO on an AIO device case via a dm-linear passthrough
on a disposable test node, to later dmsetup suspend/resume for a true
indefinite-stall simulation. Blocked by two non-bug environment issues:
node_configure.py's auto-discovery correctly excludes an already-held
disk (so it never selected the wrapped device), and leaving the
wrapper in place seemed to slow down add-node's device rescan across
the whole host for unrelated disks. Neither isolated enough to call a
confirmed bug, so marked partial with a suggested approach (pre-wrap
the disk before first boot) for whoever picks this up next. Test node
fully cleaned up; the real 3-node cluster was untouched throughout.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015pjXzhB8JQ6ySSm1vvwZq7
@boddumanohar @claude
fix(lblk): correct inverted immutability CEL rule for blkNames/blkNam…
2bf26a2 
…esExclude/blkSerials

The rule was "oldSelf.hasValue() && self == oldSelf.value()", which requires
an old value to already exist for the object to be valid at all -- this
rejected setting the field on a brand-new object's creation, and also
rejected the very unset-to-set transition the optionalOldSelf pattern was
introduced to allow. Confirmed live on a fresh config-israel install:
kubectl apply of a new StorageNodeSet with blkNames set failed with "field
is immutable" even though the object didn't exist yet.

Corrected to "!oldSelf.hasValue() || self == oldSelf.value()": valid when
there was no old value yet (first time setting, including at creation), or
when the new value matches the unchanged old value. Same bug existed in the
per-node StorageNode override fields (storagenode_types.go). Regenerated
CRDs via make manifests + helm-charts sync-from-operator.sh.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015pjXzhB8JQ6ySSm1vvwZq7
@boddumanohar @claude
fix(operator): fix two lblk unit tests broken by main's cluster-scope…
6fb2971 
…d sizing rebase

buildPerNodeEnvFile gained a required *StorageCluster parameter on main
(cluster-scoped SPDK sizing), but these two lblk-specific unit tests still
called it with the old 2-arg signature -- caught as a compile failure
after rebasing feat/lblk-block-device-support onto origin/main.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015pjXzhB8JQ6ySSm1vvwZq7
@boddumanohar
boddumanohar force-pushed the feat/lblk-block-device-support branch from d551c39 to 6fb2971 Compare August 21, 2026 11:50
@boddumanohar @claude
fix(lblk): avoid quote characters in CEL rule broken by gofmt's doc-c…
e453622 
…omment reformatting

CI failed three ways on the same root cause: golangci-lint's gofmt
check, "Operator: Manifests" (dist/install.yaml staleness), and the
real e2e suite (kubectl apply rejecting the CRD outright: "Syntax
error: token recognition error at: '”'").

Go 1.19+ gofmt reformats doc comments as prose, which converts straight
quotes to typographic quotes -- fatal here since the doc comment is a
+kubebuilder:validation:XValidation:rule="..." marker whose content is
a literal CEL expression, not prose. `self.pcieModel != ''` and
`self.driveSizeRange != ''` became `self.pcieModel != "` (U+201D),
invalid CEL syntax that the API server correctly rejected at apply
time.

Fixed by eliminating the quote characters instead of relying on exact
byte preservation: replaced the empty-string comparisons with
size(...) > 0, consistent with the size() calls already used earlier
in the same rule for the list fields. Regenerated CRDs and
dist/install.yaml (stale since the last two commits changed the CEL
rule without a final rebuild).

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015pjXzhB8JQ6ySSm1vvwZq7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@boddumanohar boddumanohar

Labels

operator

Projects

None yet

Milestone

26.4

Development

Successfully merging this pull request may close these issues.

2 participants

@boddumanohar @noctarius