inline-checksum: optional per-cluster CRC validation (rebased onto main, TD.100226.1) by boddumanohar · Pull Request #1250 · simplyblock/sbcli

boddumanohar · 2026-08-17T11:46:40Z

Summary

Rebases the checksum-validation feature from #1016 (inline-checksum-validation, diverged from main 873 commits ago, CONFLICTING/unreviewed) onto current main, and closes the one gap that branch never addressed: the REST API never accepted inline_checksum/atomic_4k.

Cluster-level, optional inline CRC checksum validation for silent-data-error protection (design ref TD.100226.1).
Frozen at cluster-create time — no update/toggle path, matching how the backend applies it (baked into each device's bdev_alceml_create call once, at creation).
Per-device mode (cv_md_method vs cv_fallback_method) is auto-detected from NVMe metadata support, not user-configured.
New in this PR (not in inline-checksum: optional per-cluster CRC validation (TD.100226.1) #1016): POST /api/v2/clusters/ now accepts inline_checksum/atomic_4k, and GET responses include them via ClusterDTO. This is what unblocks non-CLI clients (the Kubernetes operator, UI) from ever using this feature — previously only reachable by running sbcli directly on the management node.

Relationship to #1016

This supersedes #1016 rather than amending it: that branch is 873 commits stale, in CONFLICTING merge state, has an unaddressed CodeQL failure, and an incomplete manual test plan. Rather than force-pushing over shared history, this PR carries the 7 commits from that branch not already present on main (6 of its 13 commits — unrelated soak-test/perf/RAID fixes — turned out to already be on main under different hashes; git cherry confirms this), hand-resolves the conflicts against main's current state (notably cluster_ops.add_cluster's ClusterCreateLock wrapper, added after this branch forked), plus:

a small fixup commit for test-infra drift (the tests/ tree now enforces a unit/integration tier split; RPCClient's password param became SecretStr-typed)
the REST API wiring described above

Test plan

All 27 original tests/unit/test_inline_checksum.py tests pass (relocated to the unit tier)
2 new tests for the REST API wiring (test_passes_inline_checksum_and_atomic_4k, test_inline_checksum_and_atomic_4k_default_false)
Full tests/unit/ suite passes except one pre-existing, unrelated failure (test_retry_ceiling, confirmed to also fail on plain origin/main)
Lab dual-fault test on an FT=2 cluster mixing md-capable and fallback-mode drives (carried over from inline-checksum: optional per-cluster CRC validation (TD.100226.1) #1016, still not run)
Negative test: simulated corruption under an inline-checksum cluster returns an IO error rather than silent corruption (carried over from inline-checksum: optional per-cluster CRC validation (TD.100226.1) #1016, still not run)

🤖 Generated with Claude Code


-def format_nvme_device(nvme_device: str, lbaf_id: int):
-    if is_namespace_4k_from_nvme_list(nvme_device):
+def format_nvme_device(nvme_device: str, lbaf_id: int, force_reformat: bool = False):


+        for host in self.node_hosts.values():
+            try:
+                host.close()
+            except Exception:


+        if cached is not None:
+            try:
+                cached.close()
+            except Exception:


…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.

Add --verify=md5 --do_verify=1 --verify_fatal=1 to the fio job so data corruption is caught and fatal. Soften the "unrelated pair" category from a hard TestRunError to a warning plus fallback to primary_tertiary then primary_secondary, since a dense FT=2 ring with N <= 4 has no unrelated pair and should still run the scenario. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Pin BRANCH to inline-checksum-validation. Make ssh_exec accept a timeout (default 600s) and use 1800s for the parallel docker pull so large image pulls don't time out. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Adds the sbcli control-plane support for the inline silent-data-error protection feature whose data-plane work landed on ultra branch checksum-validation. Frozen at cluster-create time; no upgrade path for existing clusters. Per-device alceml mode (md-on-device vs fallback) is auto-detected from the bound SPDK bdev's md_size at add-node and on restart, so a cluster can have a heterogeneous mix of drives. Cluster + per-device flags - Cluster.inline_checksum (bool, default False): persisted only via create_cluster / add_cluster; no mutator. - NVMeDevice.md_size (int) and NVMeDevice.md_supported (bool): set in addNvmeDevices from bdev_get_bdevs' top-level md_size field (spdk_bdev_get_md_size; emitted by lib/bdev/bdev_rpc.c). Refreshed on every restart-device path so a between-restart `nvme format` is reflected. alceml RPC plumbing (matches checksum-validation branch) - bdev_alceml_create gains optional checksum_method (1=md-on-device, 2=fallback, default 0=off), cache_size, cache_eviction_threshold; zero-defaults are not sent so the data plane keeps its built-ins (cv_default_cache_size=2000, threshold 90%). - utils.alceml_checksum_params(cluster, dev) picks 0/1/2 from the cluster flag plus md_supported. Data plane refuses method=1 on md_size==0, so the per-device decision is mandatory. - Both alceml call sites (storage_node_ops._create_storage_device_stack and device_controller restart-device) thread the params through and warn when a device falls back. CLI - cluster create / cluster add / sn configure each gain --enable-inline-checksum. - sn configure runs `find_md_lbaf_id` against `nvme id-ns` JSON to pick the smallest qualifying LBAF (ds=12, ms>=8) and forces a reformat past the existing 4K-already-formatted early-out (SectorSize stays 4096 across an md/no-md transition). Capacity accounting - alceml_fallback_overhead_bytes(cluster, size): 6 lost data blocks per 2 MiB extent (510 -> 504, ~1.171875%) when the device runs in cv_fallback_method. - lvol_controller charges that as initial provisioned utilization rather than reducing reported raw cluster_size_total, so the overhead surfaces through the existing prov_cap_warn / prov_cap_crit thresholds and md-on-device drives contribute zero. Tests - 27 new unit tests in tests/test_inline_checksum.py: model defaults, find_md_lbaf_id corner cases, alceml_checksum_params combos, fallback-overhead math (including 6/512 ratio sanity), RPC param wire-up for each method, and addNvmeDevices md detection from a mocked bdev_get_bdevs payload. Behaviour notes - Default-off cluster flag means no behaviour change for existing clusters; full unit-test suite (870 passing) stayed green during development on a separate worktree. - Data plane reads md_size itself via spdk_bdev_get_md_size, so the control plane never has to pass it. Cv_md_method does not auto fall back: control plane must pick correctly per-device, hence the add-node-time detection plumbing. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

- cli-reference.yaml: add --enable-inline-checksum under storage-node configure, cluster create and cluster deploy. The flag was hand-edited into cli.py but missing from the YAML source-of-truth, so the generator kept dropping it. - storage_node_ops.py: annotate attached as set[str] for mypy. - test_subsystem_add_ns_idempotent.py: drop unused MagicMock import. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

…gure The lab perf bring-up now opts the cluster into inline CRC checksum validation for silent-data-error protection. Two flags, paired: * `cluster create --enable-inline-checksum`: sets the cluster-level flag at create time (frozen — no mutator, no upgrade path for existing clusters). * `sn configure --enable-inline-checksum`: picks an LBAF with ds=12, ms>=8 (8B+ NVMe metadata per 4K block) and force-reformats through the existing 4K-already-set early-out, so alceml can run in md-on-device mode (checksum_method=1). Devices with no md-capable LBAF fall through to plain 4K + cv_fallback (checksum_method=2, ~1.17% capacity overhead) — auto-detected per-device at sn add-node and refreshed on every restart-device path. Also preserves the existing branch pin (BRANCH=inline-checksum- validation) and the corresponding pip install URL so the lab pulls the matching control-plane and data-plane builds. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Restore the checksum-validation docker/ultra images that a prior rebase onto origin/main clobbered back to the :main defaults. A local merge=keepdev driver now keeps this branch's env_var across future rebases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Adds a frozen-at-create cluster flag for devices with a <4K logical block size that still guarantee 4K write atomicity (e.g. AWS NVMe, 512B logical but 4K-atomic). When set, bdev_alceml_create sends cv_ignore_block_size=True so the data plane skips its >=4K block-size gate, allowing fallback-mode inline checksum on such devices. - cluster.atomic_4k model field, persisted to FDB, set in create/add - rpc_client.bdev_alceml_create sends cv_ignore_block_size (True/False) within the checksum-enabled branch - storage node device stack passes cluster.atomic_4k at add-node time - CLI --4k_atomic on cluster create/add (yaml + regenerated cli.py) - setup_perf_test1.py enables --4k_atomic for the AWS perf cluster Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…t SecretStr drift Not part of the original PR — main drift since the branch forked: - tests/ now enforces a unit/integration tier split; moved test_inline_checksum.py into tests/unit/ accordingly. - RPCClient's password param became SecretStr-typed on main; updated the test helper to match (was passing a plain str).

Closes the gap the original PR left open: cluster_ops.add_cluster/create_cluster already accept inline_checksum/atomic_4k, but the REST layer never read them from the request body. Adds both fields to ClusterParams (default False, matching the CLI default) and to ClusterDTO for read-back, so non-CLI clients (the K8s operator, UI, etc.) can set this at cluster-create time too. No update path added intentionally -- the setting is frozen at create time on the backend (baked into bdev_alceml_create per-device), matching the CLI's own behavior.

ruff's EXE001 flags a shebang-bearing file that lacks the executable bit.

… corruption e2e test - sn device-testing-mode: add corrupt_data_on_write choice (cli-reference.yaml + regenerated cli.py), matching the new PASS-TM testmode added on the ultra side (simplyblock/ultra#checksum-validation-rebased-onto-main). - e2e/e2e_tests/test_checksum_corruption_detection.py: proves inline checksum validation actually detects corruption, not just that normal I/O still works with it enabled. Arms corrupt_data_on_write on a test-device, writes known data (persisted corrupted while alceml's checksum reflects the original correct buffer), disarms, and asserts a fresh read fails instead of silently returning corrupted bytes. Requires a cluster created with --enable-inline-checksum and a node added with --enable-test-device, so registered but excluded from the default bulk suite (matches the UNCERTAIN/ special-precondition convention already used for TestSharedPlacement etc).

…checksum corruption e2e test" This reverts commit 0a03e5b.

…pu_count to cluster-create test payloads

github-code-quality Bot found potential problems Aug 17, 2026

View reviewed changes

This was referenced Aug 17, 2026

feat(storagecluster): add spec.checkSumValidation for inline CRC checksum validation simplyblock/simplyblock-operator#434

Draft

Support enabling/disabling inline checksum validation (CRC) on StorageCluster simplyblock/simplyblock-operator#433

Open

boddumanohar force-pushed the inline-checksum-rebased-onto-main branch from a31fd75 to 3357ab2 Compare August 17, 2026 18:33

github-code-quality Bot found potential problems Aug 17, 2026

View reviewed changes

Comment thread e2e/e2e_tests/test_checksum_corruption_detection.py Fixed

Comment thread e2e/e2e_tests/test_checksum_corruption_detection.py Fixed

Comment thread e2e/e2e_tests/test_checksum_corruption_detection.py Fixed

schmidt-scaled and others added 13 commits August 21, 2026 14:00

fix(ci): mark lab_dual_node_outage_soak_mixed_churn.py executable

d4c7bea

ruff's EXE001 flags a shebang-bearing file that lacks the executable bit.

Revert "feat(cli+e2e): wire corrupt_data_on_write passtest mode, add …

2ce2876

…checksum corruption e2e test" This reverts commit 0a03e5b.

test(rebase-fixup): add now-required max_subsys/hugepages_mem/spdk_vc…

f4e9911

…pu_count to cluster-create test payloads

boddumanohar force-pushed the inline-checksum-rebased-onto-main branch 2 times, most recently from 50c24d0 to f4e9911 Compare August 21, 2026 12:07

Navigation Menu

Uh oh!

inline-checksum: optional per-cluster CRC validation (rebased onto main, TD.100226.1) - #1250

inline-checksum: optional per-cluster CRC validation (rebased onto main, TD.100226.1)#1250
boddumanohar wants to merge 13 commits into
mainfrom
inline-checksum-rebased-onto-main

boddumanohar commented Aug 17, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

boddumanohar commented Aug 17, 2026

Summary

Relationship to #1016

Test plan

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants