feat(storagecluster): add spec.checkSumValidation for inline CRC checksum validation - #434
Draft
boddumanohar wants to merge 8 commits into
Draft
feat(storagecluster): add spec.checkSumValidation for inline CRC checksum validation#434boddumanohar wants to merge 8 commits into
boddumanohar wants to merge 8 commits into
Conversation
1 task
boddumanohar
force-pushed
the
feat/checksum-validation
branch
from
August 17, 2026 18:31
6d10050 to
5f86e19
Compare
…ksum validation
Adds StorageCluster.spec.checkSumValidation.{inlineChecksum,atomic4k}, mirroring
sbcli's cluster-level inline_checksum/atomic_4k fields (design ref TD.100226.1).
Both are frozen at cluster-create time on the backend, so the whole struct is
marked immutable (kubebuilder XValidation equality rule + k8s:immutable), same
pattern as enableFailureDomains.
Requires the corresponding sbcli backend support (simplyblock/sbcli#1250) on
POST /api/v2/clusters/ -- against an unpatched backend, setting these fields
is a silent no-op since the field is simply unread by the API.
…Cluster atomic4k only has meaning as an inline-checksum fallback-path escape hatch (tells the data plane to skip its own >=4K block-size gate for devices like AWS NVMe); with checksum validation off it configures nothing on the backend and just misleads readers of the CR. Both fields are already immutable once checkSumValidation is set (CEL rule on StorageClusterSpec), so the webhook only needs to catch the combination at create/first-set time.
…fix gocyclo - make helm-sync: propagate the new checkSumValidation CRD field and storagecluster validating webhook into the Helm chart. - make build-installer: regenerate dist/install.yaml with the same. - Split the two new checksum-validation subtests out of TestStorageClusterReconcileCreationPaths into their own test function -- the combined table pushed that function's cyclomatic complexity over golangci-lint's gocyclo limit (35 > 30).
Wires StorageNodeSet.spec.enableTestDevice through to the /storage-nodes add-node payload as enable_test_device, letting a node's devices get a passtest bdev inserted below alceml for fault injection (see sbcli sn device-testing-mode, e.g. corrupt_data_on_write -- used by TestChecksumCorruptionDetection to prove inline checksum validation actually detects corruption). No production safeguard yet: unlike enable_hang_device (never exposed via any operator CRD), this field is left in the schema as-is for now so k8s-native e2e can exercise it. Restricting it (e.g. an admission webhook allow-listing a test-runner ServiceAccount, mirroring StorageNodeValidator's spec.workerNode gate) is tracked as a followup before this should be considered safe for production clusters.
…ot enable_test_device The v2 storage-node REST API's StorageNodeParams field is `test_device` (simplyblock_web/api/v2/cluster/storage_node/__init__.py); `enable_test_device` is only the internal storage_node_ops.add_node() kwarg name it maps to server-side. Sending `enable_test_device` on the wire was silently dropped as an unrecognized field -- verified live: no passtest bdev was created despite spec.enableTestDevice: true.
…evice, not enable_test_device" This reverts commit e3c567e.
…e2e testing" This reverts commit 13a9191.
boddumanohar
force-pushed
the
feat/checksum-validation
branch
from
August 21, 2026 12:00
e06fbe4 to
d9dfde1
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes: #433
Summary
Adds
StorageCluster.spec.checkSumValidationto expose sbcli's inline CRC checksum validation feature (silent-data-error protection, design ref TD.100226.1) through the operator.inlineChecksum(*bool, defaultfalse) — enables inline CRC checksum validation on every IO.atomic4k(*bool, defaultfalse) — only meaningful wheninlineChecksumistrue; declares 4K write atomicity on devices with a <4K logical block size (e.g. AWS NVMe).checkSumValidationstruct is immutable once set (CELXValidationequality rule ++k8s:immutable), matching the backend: the checksum method is baked into each device'sbdev_alceml_createcall once, at cluster-create time, and never re-applied.ClusterAddParams→POST /api/v2/clusters/, following the same pattern asenableFailureDomains.StorageClusterValidator,/validate-storage-simplyblock-io-v1alpha1-storagecluster) rejectsatomic4k: truewithoutinlineChecksum: true— that combination configures nothing on the backend and would just mislead readers of the CR.Blocked on
inline_checksum/atomic_4kto thePOST /api/v2/clusters/REST layer (ClusterParams/ClusterDTO). Without it, this operator change is a silent no-op: the backend will simply not read the fields this PR sends. Marking this draft until #1250 merges.Test plan
make generate/make manifestsregenerated deepcopy, CRD manifests, and the newValidatingWebhookConfigurationentrycheckSumValidationfields make it into thePOST /api/v2/clusters/payload when set, and are omitted from the payload when unsetatomic4krequiresinlineChecksumrule (unset, both false, inlineChecksum alone, both true, atomic4k alone, atomic4k with nil inlineChecksum)🤖 Generated with Claude Code