Update e2e backup tests for backup-rework CLI changes - #1255
Open
RaunakJalan wants to merge 209 commits into
Open
Update e2e backup tests for backup-rework CLI changes#1255RaunakJalan wants to merge 209 commits into
RaunakJalan wants to merge 209 commits into
Conversation
RaunakJalan
commented
Aug 19, 2026
RaunakJalan
commented
Collaborator
- backup import: positional arg → --from-file flag (3 locations)
- backup restore: remove --cluster-id flag (no longer exists)
Lvol migration test fixes
… clone's parent The all-zeros DR fail-over, root-caused at the RPC level this time. Fail-over (replicate_lvol_on_target_cluster) left do_replicate=True and the pending FN_SNAPSHOT_REPLICATION tasks queued on the source. The "dead" source cluster auto-recovers within minutes, the tasks then complete, and each completion runs _prune_internal_snapshots for the source volume. Retention keeps only the newest replicated internal snapshot and deletes the TARGET copies of older ones — including the snapshot the fail-over volume was just cloned from. That delete reaches SPDK as bdev_lvol_delete(sync=False) (validated in the spdk_proxy logs: prune line to delete RPC in ~26ms, same thread), and sync=False frees the blocks immediately, so every DB-level guard downstream fires after the data is gone. Observed timeline (2026-08-11 lab): fail-over 21:22:24-37 with 4 replication tasks still `running`; prune + delete RPC 21:23:54; the controller's soft-delete guard fired the same second and the monitor guard at 21:24:09 — both too late. The volume reads zeros from ~90s after a successful fail-over: no filesystem, md5 mismatch, while every status field says online. Case-1-style migration cutover is unaffected because it retires the source volume through the normal path instead of leaving its replication running. Two-part fix, both ahead of the RPC: * replicate_lvol_on_target_cluster now calls replication_stop() before recording the relationship: do_replicate=False and the pending replication tasks cancelled. A failed-over volume no longer lives on the source, so there is nothing left to replicate; any later source delta is by definition past the RPO the fail-over accepted. * _prune_internal_snapshots skips (and logs) a target snapshot that a live volume is cloned from, keeping the paired source copy too — so no other caller can ever issue the fatal delete. An in_deletion clone deliberately does not pin the snapshot, so retention cannot deadlock behind a dying clone. The earlier hardening commits (fail-over point selection, clone-under-lock, monitor live-clone guard) remain valid but each acted after the sync=False delete had already freed the blocks; this addresses the mechanism itself. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Follow-up to 7250fa0, hardening the dominant failure path of the 2026-08-11 soak: lvol_monitor's repair called add_ns for an already-bound namespace, SPDK rejected it with -32602 "Invalid parameters" (nsid taken), and add_lvol_thread returned on the error — before the listener loop — so the volume lost a PATH rather than just failing a redundant add, and the repair re-failed identically on every monitor cycle (20 hits across the two recovered nodes). The duplicate case is already recognised upstream since 7250fa0 (the add_ns idempotency probe matches by UUID), so reaching the error branch now means a real failure or a probe miss. Either way what matters for the client is whether the namespace is on the subsystem NOW: re-read it (bounded poll) and continue to listener setup when present; give up only when the namespace is genuinely absent. The empty-subsystem guard below stays fail-closed. Soak: SPDK verification is now a heal GATE rather than a fixed-window check. Redundant-path re-add after an outage runs on the health-check / reconcile cadence and legitimately takes minutes (measured 169s for full hublvol path convergence after a 30s all-node single-NIC outage; run 2 aborted spuriously because its 75s window undershot exactly that). The gate blocks the next iteration until every path, policy and listener has converged — a new outage on top of still-degraded redundancy would test an unplanned scenario — logs the healing time per phase as a measurement, and fails only on --path-heal-timeout (default 900s). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Root cause of the all-zeros DR fail-over, fourth and final layer. The fork addresses replicated cluster writes as (redirect_map_id << 48) | offset — the top 16 bits of the LBA carry the RECEIVING volume's map id so the target distrib routes the data into its map, and the end-of-transfer signal is only recognised as a control message when tagged (lib/lvol/lvol.c, R26.3 == fork-main). redirect_map_id comes from bdev_lvol_transfer's lvol_id parameter. snapshot_replication started every transfer WITHOUT lvol_id, defaulting to 0: writes went untagged, the target blob allocated clusters (used_size looked plausible) but no readable data ever landed in the map, and the completion signal was written as plain data at LBA 0. Every snapshot replicated this way is empty; a clone of it — the DR fail-over volume, or even a plain `snapshot clone` (verified live) — reads zeros with no filesystem while all metadata (chain, base_snapshot, allocated clusters) looks correct. The migration runner has always passed lvol_id=tgt_map_id to the same RPC, which is why planned cutover (case 1) worked while fail-over (case 2) failed 4/4. Fetch the receiving volume's map_id from the target node at transfer start and pass it as lvol_id; if it cannot be read, suspend and retry the task instead of launching a transfer that cannot land. The transfer→add_clone→convert wiring itself was verified correct — it just faithfully froze an empty volume. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Add kubectl install to Dockerfile with multi-arch support * Fix kubectl not found by installing to /usr/local/bin * replace kubectl binary with kubernetes Python client in collect_logs.py and add missing task-runner services * fix collect_logs: use since_seconds instead of since_time and replace kubectl with kubernetes Python client
…ap_id" This reverts commit e5b1cf6.
…ap id Replication transfers were aimed at the receiving volume's own namespace with no lvol_id. Per the fork's transfer contract (confirmed by the data-plane design): bulk transfers must go over a HUBLVOL, and the receiving volume's map id must ride in each write's LBA (top 16 bits, lvol_map.lvol[offset >> 48]) — the demux only exists on a hublvol namespace. The migration runner has always done hub+map_id and works; replication did neither half. The previous attempt (e5b1cf6, reverted) added the map id but kept the volume-namespace gateway: tagged writes at an endpoint with no demux, which failed transfers outright. Attach the target node's transfer hublvol on the source (reusing ensure_hub_attached, shared+persistent across cycles like the migration hub), fetch the receiving volume's map_id from the target, and start the transfer hub+map_id. Suspend-and-retry the task if either is unavailable. The per-volume controller detaches at finish go away — there is no per-volume controller any more. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…sters Reverts the leader-leg removal from ae46797 (its part 1, the inline non-leader sync legs, stays). That commit's premise — "the leader's async delete already removed the blob and unregistered the bdev" — is false: - bdev_lvol_delete sync=False ends at bs_delete_blob_finish_async -> blob_clear_clusters_async: data clusters cleared, in-memory clone-list entries stripped, blob metadata and bdev registration left in place. The delete-status "done" (deletion_status=2) means the unmap finished, nothing more. - The leader's sync=True delete (_vbdev_lvol_destroy is_sync=true) is the only operation that unregisters the bdev and deletes the blob md. SPDK admits it exactly once the async pass reports done. Evidence: upgrade run 20260812 (test_major_upgrade-20260812-170049). All 24 lvols of the teardown wave were "deleted" (records removed, subsystems gone, non-leader registrations gone) while EVERY leader kept every blob and bdev — end-of-run dumps show all 4 LVSes with their complete object sets. The follow-up snapshot delete failed EBUSY -16 ("Cannot remove snapshot because it is open", blobstore.c:11451) because the snapshot's two children were still alive on the leader (snapshot open_ref=3), and the soft-delete gate could not protect it: the children's DB records were already gone, so the snapshot looked clone-free to the control plane. Why the 20260807 evidence misled: - The 4361 "Clone entry not found" errors on the leader are BENIGN: blob_get_snapshot_and_clone_entries only logs when the async pass has already removed the in-memory entry; the sync delete proceeds and does the real work. Noise, not harm. - The "0 leftovers without the leader leg" check sampled the 162 create-rollback objects, which do not represent the regular delete path (failed creates often never registered a leader bdev, and the end dump postdates lvstore teardown). Changes: - lvol_monitor.process_lvol_delete_finish: leader sync delete restored (under the leader's lvstore lock), comment corrected. - snapshot_monitor.process_snap_delete_finish: leader sync delete restored (with special_delete passthrough), comment corrected. - snapshot_controller._rollback_snapshot_bdev: leader sync delete added after the bounded completion poll (this path NEVER had one — same leak for rolled-back snapshot bdevs), invariant docstring corrected. - lvol_controller._delete_lvol_from_all_nodes: comment corrected (the inline non-leader legs and fail-closed poll are unchanged). The "Clone entry not found" storm returns with the leader leg; it is understood and harmless. Proper long-term fix is SPDK-side: either the async delete carries through to metadata removal (then the leader leg and the noise both go away) or the post-async sync walk skips entries the async pass already stripped. Unit tier: test_async_delete_poll passes (9/9); ruff and mypy clean. pytest-style tests need Linux (SIGALRM conftest) and run in CI. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… snapshot
On the target, every replicated snapshot is chained onto its predecessor
(add_clone at receive time): the newest snapshot reads THROUGH the older ones.
Retention ("keep only the last replicated internal snapshot") deleted those
predecessors outright, destroying the shared clusters the survivor depends on.
Observed on the 2026-08-13 lab with transfers finally landing real data
(hub+map_id): a clone of the newest target snapshot had a valid XFS superblock
(its own delta) but an empty tree — everything underneath was gone.
Before pruning a target snapshot, decouple every child SPDK reports
(bdev_lvol_decouple_parent copies the parent's allocated clusters into the
child) on the primary and its online secondary. If any decouple fails, keep
the snapshot pair and retry next pass rather than deleting a parent something
still reads through. New rpc_client wrapper for bdev_lvol_decouple_parent
(fork RPC, {"name": <child>}).
Note tests/unit/test_rollback_sync_delete_all_peers.py fails on current main
independently of this change (5f4315e restored the leader's sync delete;
the test still asserts the pre-restore behaviour).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#1245) * fix(alerting): use ratio query for cluster capacity alerts to preserve cluster label in fired alerts * fix(test): update rollback sync delete assertions for leader node --------- Co-authored-by: hamdykhader <hamdy.khader@gmail.com>
The target_node_id was used authoritatively, not as a hint. It silently overrode the intended cluster. Because the cluster-id is defined by the pool, the cluster-id flag is dropped completely.
The introduced API fixes a number of issues with the API v1 metrics, so
it's not compatible with it:
- Dropped size_util / size_prov_util: v1's were lossy (int(used/total*100)),
not wrong. Adequate for a threshold alert, coarse for graphs. Dividing the
byte gauges is exact.
- Status as {status="degraded"} label instead of numeric code. The label
just doesn't hardcode magic numbers and survives map changes.
- health_check absent instead of NaN: Same information, no float sentinel
for consumers to special-case.
- Dropped date, record_duration, record_start_time, record_end_time:
Metadata, the record_* trio was never populated by any collector.
- simplyblock_ namespace: Avoids collision with the node and foundationdb
jobs and allows coexistence with v1 metrics.
- Added pool_name to volume series: Preserves the human-readable $pool
dropdown that v1 got for free from its (otherwise buggy) label.
…a target snapshot" This reverts commit acc8e0c.
…the hub session
Two receive-side steps the migration runner has always performed and
replication never did, both mandatory per the fork's transfer contract:
* bdev_lvol_set_migration_flag on the receiving lvol BEFORE the transfer.
The flag (a) stamps the receiving blob's writes special_io=1, which the raid
layer encodes into the LBA and the distrib stack uses for receive-mode
placement (blobstore.c bs_batch_open_s/special_io), and (b) arms the hub
write handler's detection of the end-of-transfer signal — hublvol_write only
routes a 1-page write at page 0 into process_migration_write_request ("add
this lvol as clone ... mark migrate process completed") when migration_flag
is set (vbdev_lvol.c:1414). Without the flag the payload lands as ordinary
client IO and the completion signal is written ONTO LBA 0 as data: the
receive never finalises and every clone of the converted snapshot reads
zeros — reproduced with a 3-step repro (replicate one snapshot, clone the
target copy, read) with no failure injection at all. add_clone/convert clear
the flag, closing the lifecycle.
* Detach the transfer hub on the source when the cycle finishes (success and
abort paths). The connect -> transfer -> convert -> disconnect cycle is part
of the contract; the next cycle re-attaches via ensure_hub_attached.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…sync c04cfcb adapted this test to the restored leader sync leg (5f4315e) with a bare call_count == 2, which would also pass if the leader received two async deletes and never its sync leg — the exact leak of upgrade run 20260812. Assert the full call list instead: phase-1 async first, then sync=True. Also correct the module docstring, which still stated the falsified "never on the leader" protocol. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ive path The flag drives the distrib-level special_io machinery of INTRA-cluster migration (copy-on-write context within one cluster's maps); it does not apply to a cross-cluster receive, where the source cluster's map/COW context does not exist on the target. It was added in 7ea16df by copying the intra-cluster migration recipe; reverting that half. The hub connect->transfer->disconnect session lifecycle from the same commit stays. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
replication_commit took one fire-and-forget internal snapshot, selected a cutover base that was 1-2 replication intervals old, and enqueued the final task — which froze immediately, WITHOUT waiting for anything to replicate. The writes between the last replicated snapshot and the pre-commit snapshot were covered neither by the final step's delta (top blob only = writes after the pre-commit snapshot) nor by anything on the target: every cutover silently lost up to ~2 intervals of data. Invisible to the harness because fio's sequential sweep rewrites its whole working set and verify_backlog only checks recent writes. The cutover now shrinks the delta iteratively before freezing: snapshot #1 (at commit) -> wait until replicated AND converted on the target -> IMMEDIATELY snapshot #2 (delta = just the wait window) -> wait again -> IMMEDIATELY build the target clone on that last replicated snapshot and run the freeze + ANA flip. replication_commit is now thin (validate, shrink snapshot #1, enqueue); the final-task runner owns the shrink state machine (bounded by REPL_CUTOVER_SHRINK_TIMEOUT_SEC, waiting does not burn task retries) and the clone/map-id/replication-record preparation, so the base is always the freshly replicated shrink snapshot. Old-style tasks with tgt_* params still run unchanged (skip shrink + prepare). Harness: case 1 now writes a baseline fio never touches and md5-verifies it through the cutover volume after a remount — the ONLY assertion that exercises the replicated snapshot history (fio's own data always arrives via the final step); target paths are connected inside the cutover wait loop as soon as the runner creates the target volume, keeping multipath ahead of the ANA flip. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The cutover enabled the target paths and only then set the source paths inaccessible. Two windows lose writes: (a) after the final delta is taken and the freeze lifts, the source is still ANA-optimized until the flip reaches it — a client write landing there is newer than the delta of record and silently lost; (b) during the flip itself both source and target are optimized — dual-writable. The intra-cluster migration runner already fences source replicas pre-freeze for exactly this reason; the cross-cluster cutover did not. New order: ALL source paths -> inaccessible (peers first, primary last), THEN freeze + final delta (nothing can land on the source by any means; the delta is definitively final), THEN target primary -> optimized, peers -> non_optimized. Client IO queues during the all-dark window (NVMe multipath semantics), bounded by freeze + residual delta — seconds, thanks to the delta-shrink rounds. If the freeze FAILS after the source was fenced, the source paths are restored (primary optimized, peers non_optimized): nothing moved, the source is still the authoritative copy, and a failed cutover attempt must not leave the volume dark. flip_ana_failover is split into fence_source_paths / enable_target_paths / restore_source_paths; ordering asserted in tests. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…eive and convert
Two non-leader hazards in the fork's data plane:
* the transfer hub REJECTS receive IO on a non-leader ("receive io for hublvol
in nonleader mode") — a transfer started against a non-leader target fails
outright;
* bdev_lvol_convert on a non-leader DEGRADES SILENTLY: the non-leader branch
marks the blob CLEAN and replies success without persisting anything, so the
"snapshot" looks converted while its metadata never reached the journal —
return-value checks cannot catch it.
Gate both: replication verifies the receiving node holds LVS leadership before
starting a transfer and again before add_clone/convert (suspend-and-retry
otherwise); the lvol-migration and batch-migration runners verify leadership
before their converts (fail-and-retry). Shared probe: lvol_controller.
is_node_leader.
Also: setup_repl_test_2clusters ssh_exec gets the reconnect-retry the test
harness already had — a 10054 reset killed a second deployment at the finish
line.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ive user
The hub is ONE session per target node. The per-cycle detach added with the
connect->transfer->disconnect contract ripped the shared qpair out from under
the other volumes' in-flight transfers: mass hub IO failures, LVS leadership
churn on the target ("receive io for hublvol in nonleader mode" storms,
observed live 2026-08-13), transfers landing nothing, converts silently
no-oping on flapped leadership. Detach now happens only when no other RUNNING
snapshot-replication task is transferring into the same target node — the
refcount discipline the migration runner's hub_manager embodies.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…n the remote cluster Replicated snapshots on the target were standalone blobs (clone=false, base=null in SPDK): the finish path only chained when snap_ref_id was set, but internal replication snapshots never populate it, and even then the lookup matched the remote node against the SOURCE snapshot's instances, which are source-cluster nodes. bdev_lvol_add_clone was never attempted (chain_attempts=0 across entire runs). A fail-over clone therefore read only the last delta and zeros elsewhere, and retention's delete could not swap-merge segments into a successor — the all-zeros DR fail-over. Resolve the predecessor by lvol + age (newest older snapshot with a completed remote copy, snap_ref_id still wins when set), chain to the remote copy's bdev, and fail-and-retry instead of silently finalizing an unchained snapshot when a predecessor exists but cannot be resolved. Harness: mark read-only DB/status pollers replayable so a mid-exec socket reset (WinError 10054) replays the query instead of aborting the case. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…etry backoff
The monitor ran one serial pass per cycle: take internal snapshots, then
process EVERY in_deletion record with a full DB fetch and RPCs. Two
consequences observed on the 2026-08-14 lab (1298 records in in_deletion):
- a delete that cannot complete is retried every cycle forever, so the
in_deletion set only grows and each cycle costs more;
- internal-snapshot creation shares that pass, so it never came around
again — five replicated volumes went an hour with zero snapshots, which
looks like 'replication stopped' but is starvation.
Creation now runs for all clusters before any delete work. Deletes are
processed by a bounded pool keyed on the owning volume, so a chain
(clone -> snapshot -> parent) still advances in order on one worker while
different volumes proceed concurrently. Failing deletes get exponential
backoff (5s..5min) instead of a slot every cycle.
Concurrency contract: per-object create+register stays serialized by
object_mutation_lock; every synchronous single-node RPC is mutually
exclusive per node via lvstore_op_lock, which the phase-2 sync deletes now
take individually. That is the creators' key space ('<lvs>@<node8>') — a
whole-lvstore key would be a different key and exclude nothing. DB
finalize stays outside the lock.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
_ensure_pool_and_sc() and the DHCHAP host-registration step called add_storage_pool()/add_host_to_pool() on the low-level K8sUtils returned by _ensure_k8s_utils(), but those methods only exist on the outer K8sSbcliUtils (self.sbcli_utils). Same class of bug as the security test fix, found by grepping the pattern across both files.
…nnect The K8s operator enforces DHCHAP purely from StoragePool.spec.dhchap + allowedNodes: it derives each allowed node's NQN itself, labels those nodes, and restricts scheduling via the generated StorageClass/PV nodeAffinity. No host NQN is ever supplied by a client. The previous K8s branches of TestLvolCryptoWithDhchap/TestLvolDhchapBidirectional ported the docker-mode manual `nvme connect --host-nqn` flow as-is, which doesn't reflect how a real workload (or the operator) actually enforces the restriction, and never exercised the negative case since _setup_pool_and_host allowed ALL workers rather than a subset. - create_utility_pod()/create_fio_job() gain a node_name param to hard-pin a pod via spec.nodeName, bypassing the scheduler so a pod can be deliberately forced onto a disallowed node. - New get_pod_events() reads FailedMount events (a Pod event, not a container waiting-state reason get_pod_status_detail could see). - New _k8s_setup_dhchap_pool_subset()/_k8s_verify_pod_scheduling() helpers: pool's allowedNodes is a strict subset of workers, and a pod pinned to an allowed node must mount; pinned to the disallowed one must fail with FailedMount. - TestLvolCryptoWithDhchap, TestLvolDhchapBidirectional, and TestDhchapPodScheduling now use this for their K8s branch (docker branches unchanged — manual connect-string is the correct native approach there). TestDhchapPodScheduling also gains the missing negative case (pod on a disallowed node).
… limit
Every lvol creation in RandomMultiClientMultiFailoverAllNodesTest was
failing with HTTP 400 ("exceeds the hard limit of 50 namespaces per
subsystem"), including the retry, for all 40 lvols, from the first
attempt onward. The test ran its full multi-hour outage cycle against
zero actual volumes. Lowered to 30, comfortably under the limit.
add_storage_pool() blindly reused ANY existing sbcli-visible pool regardless of the caller's dhchap/allowed_nodes request. On a shared test cluster with leftover pools from unrelated tests (e.g. "encryption-pool"), a DHCHAP test asking for allowedNodes-restricted access got handed back a pool with no such restriction at all — so a pod pinned to a deliberately-disallowed node mounted it just fine, because the enforcement was never actually configured on the pool being used. Now: a dhchap/allowed_nodes request only reuses an existing pool if its StoragePool CRD already has that exact dhchap+allowedNodes config; otherwise it creates a dedicated, uniquely-named pool. Also scoped the CRD-existence and Terminating-CRD checks to that dedicated pool's own resource name (previously "any StoragePool CRD exists in the namespace" would skip creating ours and hand back whichever unrelated pool the operator listed first). Callers that don't pass dhchap/allowed_nodes keep the original, proven blind-reuse behavior unchanged.
DHCHAP enforcement reaches the volume only when the StorageClass carries dhchap_node_label — the CSI driver then writes a matching nodeAffinity onto the PV and a non-allowed node fails to mount. Without it the pool reports DHCHAP enabled and any node mounts the volume, which is why the disallowed-node assertion kept failing. Verified by hand on Talos and RHCOS: allowed node mounts, non-allowed node gets "MountVolume.NodeAffinity check failed". - create_storage_class() takes dhchap_node_label; security tests derive simplyblock.io/pool.<ns>.<cluster CR>.<pool> and pass it on both the plain and crypto classes. Paired with Immediate binding and no allowedTopologies, which needs no CSI driver restart. - Skip instead of fail when the host kernel ignores the DHCHAP connect options (no CONFIG_NVME_AUTH — Talos 6.18.24 does not have it, RHCOS 9.6 does), matching the RDMA test's skip pattern. - _k8s_verify_pod_scheduling cleans its pod in a finally and tracks it for teardown; teardown now deletes pods BEFORE PVCs and removes test-created StorageClasses. A leaked pod held pvc-protection on its claim and left three PVCs stuck Terminating for 3+ hours. Pipeline cleanup (cleanup_upgrade_test.sh) missed three things that let stale state survive into later runs: - CR_TYPES had pool./simplyblockpool. but not storagepools., stale since the Pool->StoragePool rename, so pools were never deleted and the next run reused one. - StorageClasses were filtered by name containing "simplyblock", which matched 1 of 53 leaked classes; now selected by provisioner. - Only two node labels were stripped; simplyblock.io/storage-node-uuid.* and simplyblock.io/pool.* embed an id in the key so each deploy adds a new one. Left behind, the CSI driver advertises topology for clusters that no longer exist. Now stripped by prefix.
The operator derives simplyblock.io/pool.<ns>.<cluster>.<pool> from the StoragePool CRD's metadata.name, while self.pool_name holds the backend pool name. Those have matched in every run observed so far, but nothing guarantees it — and a wrong key means the StorageClass silently carries no enforcement, which is the exact failure mode this change set exists to fix. Look the label up on an allowed node (value == "allowed", key suffix == pool name) and only fall back to the computed string, warning loudly when that happens.
Four deviations from the documented K8s DHCHAP flow now carry explicit HACK comments saying what they work around and when to delete them: 1. We bypass the operator-generated StorageClass and hand-roll one with only dhchap_node_label, no allowedTopologies, Immediate binding — because the operator's class provisions nothing until the CSI driver re-registers. 2. spec.nodeName pinning is only sound because of (1); against the documented WaitForFirstConsumer class it is a false negative. 3. DHCHAP pools get a timestamp-suffixed name so a shared leftover pool cannot shadow them, since blind pool reuse is load-bearing for every non-DHCHAP caller. 4. The older security tests still register a host NQN by hand in K8s mode and pass every worker as allowed, contradicting the doc and making a disallowed-node rejection untestable there.
TestLvolCryptoWithDhchap failed with the PVC never binding. The CSI provisioner was rejecting the PV outright: PersistentVolume is invalid: spec.nodeAffinity...matchExpressions[0] .key: Invalid value: "simplyblock.io/pool.simplyblock.simplyblock- cluster.simplyblock-sec-test-pool-950958": name part must be no more than 63 characters The operator derives that label from the StoragePool CRD name, the CSI driver writes it into every PV's nodeAffinity, and a label key's name part is capped at 63. With ns=simplyblock and CR=simplyblock-cluster the fixed prefix costs 37, leaving 26 for the CRD name — "simplyblock-sec- test-pool" already used 25 of those, so the dedicated-pool timestamp suffix pushed it to 69. That is also why only the crypto test failed: the other two happened to take the unsuffixed path at 62/63. - Shortened the security tests' pool to "secpool" (label 56, ~7 chars of headroom) instead of sitting one char under the limit. - Shortened the dedicated suffix from 6 digits to 4. - add_storage_pool now budgets the CRD name against the real namespace and StorageCluster CR name and truncates, warning instead of emitting a name whose label cannot be enforced.
The security suite exists to prove NVMe-oF host authorization actually restricts access. In K8s mode it proved almost nothing while reporting PASSED, which is why several consecutive CI runs looked plausible before the real defects surfaced. Root causes of the vacuous passes, all now closed: - `_get_connect_str_dual` hardcoded its error slot to "", so every `assert not err` and every `rejected = bool(err) or not connect_ls` in the file collapsed to `not connect_ls`. It now routes through `exec_sbcli` for a real (stdout, stderr) and folds an sbcli `Error:` on stdout into the error channel. - `_setup_pool_and_host` passed EVERY worker as `spec.allowedNodes` and hand-registered an NQN (which contradicts the operator model). With no node outside the pool, not one of the ~14 classes built on it could exercise a rejection. It now returns (pool_id, allowed, denied) with a strict allowedNodes subset in K8s and registers nothing. - `_assert_cli_error` passed whenever stdout was empty; combined with the forced `err = ""` it could not fail. Replaced by `_assert_cli_rejected`, which corroborates the textual signal with a state check on the pool's allowed hosts and has no escape hatch. - `_k8s_pool_node_label` warned and fell back to a computed key. A wrong key means the StorageClass carries no enforcement at all, so it now raises rather than letting the suite run vacuously. - `_disconnect_and_unmount_dual` was a K8s no-op, so a pod still holding the RWO claim made the next mount fail with Multi-Attach -- which is indistinguishable from a DHCHAP denial. It now releases attachments, and `_assert_host_denied` rejects a Multi-Attach event outright. New vocabulary in SecurityTestBase replaces ~30 connect-string assertions: `_assert_host_authorized` / `_assert_host_denied` (docker asserts on the connect string; K8s pins a pod and asserts the mount outcome plus a DHCHAP-specific event reason), `_grant_host_dual` / `_revoke_host_dual` (K8s patches spec.allowedNodes and waits for observable convergence instead of sleeping), and `_k8s_assert_dhchap_wiring` which proves four enforcement links up front so a break is attributable to a stage. A denial assertion refuses to run without a positive control on the same volume -- a volume that cannot mount anywhere looks exactly like a denial. A setup-time canary runs once per process and fails loudly if enforcement is not wired, converting the silent failure mode into a loud one. Also fixed, found while tracing: - Every class opened with `self.fio_node[0]`, but cluster_test_base sets `fio_node = []` when there is no CLIENT_IP and no mgmt node -- the normal K8s-native shape. That was an IndexError on line 1 of run(), 19 times. Replaced by `_normalize_fio_node()`. - TC-SEC-103 created a non-DHCHAP pool but then created the lvol in self.pool_name / the DHCHAP StorageClass, so it asserted about the wrong pool -- broken in docker too. - TC-SEC-113 called delete_lvol(lvol_name=...) then asserted get_lvol_id(lvol_name) was falsy; in K8s the backend lvol is named after the PV, so that lookup returned None either way and the case ALWAYS passed. Now routed through the dual helpers. - TC-SEC-141 resized via sbcli_utils.resize_lvol, bypassing the PVC in K8s. Now uses _resize_lvol_dual and asserts the capacity actually grew before claiming the config survived the resize. - ext4/xfs coverage did not exist in K8s at all (`_pick_fs_type` was dead code there). The choice is now threaded into the StorageClass and verified from /proc/mounts inside the positive-control pod. - The three outage classes started a raw threading.Thread on ssh_obj.run_fio_test, which cannot work in K8s. Added _start_bg_fio_dual / _assert_bg_fio_alive_dual / _finish_bg_fio_dual, with md5 verify and a rescheduled-pod check. Explicit skips, each logging a grep-able token and naming the coverage lost rather than fake-passing: Bidirectional (spec.dhchap is one boolean, so direction is not obtainable in K8s -- the old branch was a duplicate green tick), RDMAv2, WithBackup (owned by the backup suite), MgmtNodeReboot's literal reboot (substituted by an operator restart that targets the same regression class), and NegativeConnect's tampered-secret case. IMPORTANT limitation, stated in the code and in the suite's own log output: K8s mode verifies that the pool's allowedNodes restriction is enforced at mount via the PV's nodeAffinity -- NOT that DH-HMAC-CHAP is negotiated in-band. A pool with `dhchap: false` carrying the same node label would satisfy every K8s assertion here. In-band negotiation is proven only by TestLvolSecurityNegativeConnect's tampered-secret case, which is docker-only. Docker behaviour is unchanged except where an assertion was provably wrong (TC-SEC-103, TC-SEC-113, TC-SEC-141) and where the removed escape hatch in _assert_cli_error now requires a real signal.
Both driven by live verification against the OpenShift cluster (6 workers, RHCOS 5.14 which does support in-band NVMe auth). `sbctl pool get <uuid> --json` turns out to expose two things the tests were inferring or missing: - `cr_name` names the StoragePool CRD the pool was reconciled from. `_k8s_resolve_pool_crd` now reads it directly instead of matching a CRD by its spec, falling back to the spec match only if the pool cannot be read. The CRD name is what the operator derives the node label key from, and it diverges from the backend pool name whenever the name picked up a timestamp suffix or was truncated to fit the 63-char label budget -- so guessing it was the weakest link in the chain. - `dhchap`, `dhchap_key` and `dhchap_ctrlr_key`. `_k8s_assert_dhchap_wiring` now asserts, as L0, that the pool reports dhchap=true and carries both a host and a controller key beginning with the DHHC- prefix. L0 matters out of proportion to its size: it is the only assertion on the K8s path that is about DH-HMAC-CHAP rather than about node placement. L1-L4 and the pod-placement matrix would all hold equally for a pool with dhchap:false that happened to carry the same node label, because nodeAffinity is a scheduling/mount gate and not authentication. Verified live, and recorded here because the dynamic-host tests depend on it and it was previously unproven: - Creating a StoragePool with a strict allowedNodes subset labels exactly those nodes and mirrors them into status.allowedNodes within ~10s. - Patching spec.allowedNodes to ADD a node converges in ~5s. - Patching to REMOVE a node DOES clear that node's label, in ~5s. This was the open question behind `_revoke_host_dual(hard=True)`; revocation is therefore observable, and the hard assertion in TestLvolDynamicHostManagement is safe. - Deleting the pool clears every label it created. - `allowed_hosts` is a flat list of NQN strings (not dicts) and matches the derived `nqn.2014-08.io.simplyblock:uuid:<node UID>` of status.allowedNodes exactly, so the L3 set-equality check holds.
Two defects from the first CI run of TestDhchapPodScheduling. Everything up to the crash worked: the canary proved L0-L4, the pool and wiring assertions passed, the PVC bound with the correct nodeAffinity, and Pod #1 reached Running on an allowed node. 1. AttributeError: 'tuple' object has no attribute 'splitlines'. `K8sUtils.exec_in_pod` returns (stdout, stderr), while its siblings `get_pod_logs` and `get_pod_events` on the same class return a bare string. Three new call sites treated it as a string: `_k8s_assert_fs_type` (the crash), and the marker write/read in TestDhchapPodScheduling -- where the read would not have crashed but would have failed the assertion, since `marker in (out, err)` is a tuple containment test. All three now unpack, and both check stderr rather than discarding it. Swept the file with an AST check for any other tuple-returning k8s_utils helper assigned to a single name: none left. The marker write also passed `sh -c '...'` to a helper that already wraps the command in `sh -c`; it now passes the bare shell line. 2. The canary leaked its StoragePool into the whole run. `_assert_dhchap_enforceable` deleted its PVC and StorageClass but not its pool. Because it asks for the same dhchap + allowedNodes that `_k8s_setup_dhchap_pool_subset` does, `add_storage_pool` matched it and handed it back to the first real test -- so the run's working pool was named `simplyblock-canaryq6xnc8` and the StorageClass pointed at the canary's node label. Functionally equivalent, but it leaks a pool per run and makes every downstream log line misleading. The canary now deletes its pool in the same `finally` that cleans the rest. Systemic cause, not fixed here: `K8sUtils` returns two different shapes for "read something out of a pod" -- `exec_in_pod` gives (stdout, stderr) while `get_pod_logs`/`get_pod_events` give a string. Nothing in the names signals which, so the mistake is invited at every call site and only shows up at runtime. A durable fix is to make the read helpers agree (either all return a small result object carrying both streams, or the tuple-returning ones get an `_exec`-style prefix that marks them). That is a change across ~40 call sites in k8s_utils and its consumers, so it belongs in its own commit rather than bundled here.
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| try: | ||
| if r_mnt: | ||
| self._unmount_and_disconnect(self.fio_node, r_mnt, r_id) | ||
| except Exception: |
…a transient impostor Fixes both causes of the TestDhchapPodScheduling failure in CI run 093822. The product behaved correctly there: the volume WAS refused on the non-allowed node with "MountVolume.NodeAffinity check failed : no matching NodeSelectorTerms". Two defects in the test tooling turned that into a failure. 1. We were not cleaning up behind ourselves. `delete_pod(wait=True)` waits for the Pod object only. The VolumeAttachment survives until kubelet finishes unmounting and the CSI controller completes ControllerUnpublishVolume, so a new pod created against the same ReadWriteOnce claim in that window fails with "FailedAttachVolume: Multi-Attach error". Measured in the run: 31 seconds after the pod was gone, the volume was still attached to the previous node. Adds the primitive that was missing: `get_volume_attachments`, `wait_volume_detached` and `delete_pod_and_wait_detached` on K8sUtils. `wait_volume_detached` deliberately does NOT force-delete a VolumeAttachment — removing one that is genuinely in use can strand the volume — it reports and lets the caller decide. `_k8s_release_pod` now takes `pvc_name` and waits for the detach, and all its call sites pass it. `_disconnect_and_unmount_dual` waits for the claim's volume after releasing the pods. The denial path's own cleanup waits too: the CI events show "SuccessfulAttachVolume" immediately before the NodeAffinity refusal, so even a *denied* pod leaves an attachment behind, which would block the next pod in classes that continue past a denial. 2. The impostor check ignored event ordering. `get_pod_events` sorts by .lastTimestamp, so the event list is a timeline, not a set of competing verdicts. The run's timeline was: 1. FailedAttachVolume: Multi-Attach error (transient) 2. SuccessfulAttachVolume: Attach succeeded (resolved) 3. FailedMount: NodeAffinity check failed (decisive) Testing for a disqualifying reason first rejected a conclusive denial. A positive DHCHAP reason now wins: if one is present the assertion passes, logging the transient impostor as a signal that some earlier release did not wait for detach. Only when no denial wording is present does an impostor disqualify the observation, which is the case the check was written for. Verified by replaying the exact CI event blob plus six other timelines against the decision logic: the real failure now passes, a multi-attach- only or image-pull-only timeline still fails, and a bare FailedMount is still accepted as weak evidence. Systemic cause: there was no shared "release this volume and wait until it is really detached" primitive, so every caller had to re-derive it. `e2e_tests/upgrade_tests/k8s_major_upgrade.py:700-729` already deletes stale VolumeAttachments for exactly this reason, but inline, so nothing else could inherit it. The new K8sUtils helpers are where that belongs; the upgrade test could be moved onto them in a follow-up.
…erator* The K8s substitute for TC-SEC-081 selected deployments to restart with `"operator" in name.lower()`. On the OpenShift cluster that matches two: `simplyblock-operator` and `mongodb-kubernetes-operator`. Restarting a third-party operator mid-suite is collateral damage unrelated to DHCHAP, and MongoDB backs the control plane. Now matches `simplyblock-operator` exactly, falling back to deployments carrying both "simplyblock" and "operator" so a renamed chart still resolves. Verified against the live deployment list from the cluster: the old filter picked 2, the new one picks 1.
…host In K8s-native the storage nodes ARE the worker nodes (10.0.0.10-15 == worker-0..5 on the OpenShift bed). The FIO pod is pinned to _dhchap_allowed_nodes[0] (worker-0) because DHCHAP requires an allowed node, while the outage targets primary_nodes[0] — very likely the same worker. Blacking out that host severs the FIO pod's own connectivity, so the test would be measuring its fixture rather than the product. Docker is unaffected and keeps running the class: there FIO runs on a separate client machine and the outage hits a storage node, so the two never collide. The skip is K8s-only. The blast radius also argues for skipping it inside a full-suite run. A full `iptables -A INPUT/OUTPUT -j DROP` is much harsher than the `sn shutdown` the other outage classes use: shutdown stops the SPDK service and leaves the worker and its pods alive with HA covering the volume, whereas the blackout also severs kubelet from the API server (the node goes NotReady in ~40s), which makes the liveness check read a stale "Running" and leaves the run one failed `iptables -F` away from a stranded worker. Logs SKIPPED-K8S plus DHCHAP-COVERAGE-LOST naming exactly what is lost: fabric-loss survival and DHCHAP re-authentication on reconnect (TC-SEC-092/093/094). The fix is small — pick an outage target whose K8s node differs from the FIO pod's node — but the class has never executed, so it should be re-enabled and validated on its own rather than inside a 2-hour suite run. K8s coverage is now 15 classes run / 4 skipped.
RaunakJalan
force-pushed
the
fix/backup-rework-e2e
branch
from
September 4, 2026 11:49
b36cf63 to
d30815f
Compare
An upgraded cluster's existing distribs stay on v1 write protection --
only freshly created clusters start on v2. `sbctl cluster
switch-write-protection` sends the runtime RPC to every online node and
records v2 only once they all accept it, so it has to run after the
upgrade and after every node is back online. A second round of restarts
then verifies the v2 generation was persisted and the nodes come back
cleanly under it.
The docker upgrade already did this (TEMP Step 11b/11c in
e2e_tests/upgrade_tests/major_upgrade.py). This adds the K8s equivalent
as a shared `_switch_write_protection_and_restart` helper on
K8sNativeMajorUpgrade, wired into both upgrade paths:
- rolling (R26+) -> Step 7b, after the per-node CRD restarts
- maintenance (R25→R26) -> Step 10b, after all nodes are back online
and before the maintenance window closes
K8sNativeMajorUpgradeDualNode inherits it.
The post-switch restart passes `--force`, matching docker: the nodes are
already online and healthy at that point, so a plain restart is refused
as unnecessary.
UPGRADE.md gains Step 10.2 documenting the same two-part sequence for
operators, placed after the Step 10.1 health checks (the switch requires
every node online) and before Step 11's workload restart, with the
`--force` requirement called out.
Note the helper uses `datetime.now().timestamp()` rather than
`time.time()`: this module imports `time` only locally inside two
functions, so a module-level `time.time()` would NameError at runtime.
The suite created its own StorageClass for DHCHAP pools. That tested a
configuration no customer ships: the documented flow is to consume the
class the operator generates, so a regression in what the operator emits
could never have been caught here.
Probing the cluster corrected two things I had wrong:
- The operator's generated class ALREADY sets `dhchap_node_label`. It
was only "an undocumented requirement" because we hand-rolled and had
to discover the parameter ourselves. Correcting the earlier RCA.
- `StoragePool.spec.storageClassParameters` controls `encryption`,
`filesystem`, `fabric`, `maxNamespacePerSubsys` and qos. Verified on
OpenShift: a pool with `{encryption: true, filesystem: ext4}` yields a
class with `encryption=True` and `csi.storage.k8s.io/fstype=ext4`. So
encrypted and ext4/xfs coverage survive the switch -- my earlier claim
that they could not was simply wrong, I had not checked the CRD.
Changes:
k8s_utils
- add_storage_pool accepts storage_class_parameters, emits them into the
StoragePool CRD, and compares them when deciding whether an existing
pool can be reused (a pool with different parameters must not be
handed back).
- operator_storage_class_name / wait_storage_class_exists resolve and
await the generated class, named
simplyblock-{namespace}-{clusterName}-{poolCRDname}.
- get_csi_topology_keys / restart_csi_node_driver re-register the CSI
node plugin and verify the pool label became a topology key.
- create_utility_pod and create_fio_job accept node_selector.
security suite
- _k8s_setup_storage_class now ADOPTS the operator's class; nothing in
the suite creates a StorageClass any more, canary and TC-SEC-103
included.
- Encrypted volumes get their own DHCHAP pool via
_k8s_encrypted_storage_class, because storageClassParameters is
immutable once the class exists (the CRD says to create a new
StoragePool to change it).
- All pinning moves from spec.nodeName to spec.nodeSelector. The
operator's class binds WaitForFirstConsumer and only the scheduler
triggers that, so a nodeName-pinned pod would leave the claim at
"waiting for first consumer" and never run -- making every negative
assertion pass vacuously.
- _k8s_bind_pvc schedules a short-lived binder pod so a WFFC claim binds
before callers that need the volumeHandle and the per-PV nodeAffinity
assertion.
- _DENIAL_REASONS gains the provisioning-time rejections
("is not in requisite", "volume node affinity conflict"), since with
allowedTopologies a denied node can now be refused before any mount.
"failedscheduling" is removed from the impostor denylist for the same
reason: it is now a denied node's correct symptom, not a false one.
- TC-SEC-103 additionally asserts the operator gives a non-DHCHAP pool a
class with NO dhchap_node_label.
Known product gap, worked around and worth raising: the operator's class
sets allowedTopologies keyed on the pool label, but the CSI node plugin
snapshots node labels as topology keys only at registration, so a pool
created afterwards fails to provision with "is not in requisite". We
restart the CSI node daemonset once per pool. Customers following the
documented flow hit exactly this. The cleaner product fix is to drop
allowedTopologies entirely -- dhchap_node_label alone already enforces at
mount via the PV's nodeAffinity, which this suite has proven green.
Docker mode is untouched.
The operator-StorageClass conversion worked: CSI re-register, SC
adoption, WaitForFirstConsumer binding, per-PV nodeAffinity, xfs
verification and the allowed-node re-attach all passed. Only the final
denial assertion failed, and the event it rejected actually proves the
denial:
FailedScheduling: 0/9 nodes are available:
1 node(s) didn't match PersistentVolume's node affinity,
3 node(s) had untolerated taint {node-role.kubernetes.io/master: },
5 node(s) didn't match Pod's node affinity/selector.
3 masters + 5 non-targeted workers + 1 = 9, so the "1 node" is
unambiguously worker-5 -- rejected for the PV's nodeAffinity, exactly the
DHCHAP denial we were asserting.
Two defects, both from one wrong assumption.
1. FailedScheduling is an AGGREGATE over every node in the cluster, not a
statement about this pod's target. The impostor denylist assumed each
reason described what blocked our pod -- true for FailedMount, which is
pod-scoped, but false for FailedScheduling. "untolerated taint" there
describes the masters and "insufficient <resource>" can describe any
node, so both fire on essentially every scheduler-side denial. Removed
from the denylist, with the reasoning recorded next to it. Multi-Attach,
image-pull and the rest stay: those are genuinely pod-scoped.
2. The scheduler words a nodeAffinity rejection differently from kubelet.
_DENIAL_REASONS carried the mount-time "NodeAffinity check failed" but
not the scheduling-time "didn't match PersistentVolume's node affinity",
which is what nodeSelector pinning now produces. Added.
This surfaced only after the nodeName -> nodeSelector switch put the
scheduler in the loop, which the operator's WaitForFirstConsumer class
requires.
Replayed 8 event shapes through the corrected logic, including both real
CI failures: the two genuine denials now pass, and multi-attach-only,
image-pull-only and taint-only-without-a-denial all still fail.
The Talos R25 upgrade run put simplyblock-storage-node-ds and
simplyblock-csi-node on the control-plane node. Checking the last
known-good run (33070003566) shows why: it passed
worker_nodes = worker-1,worker-2,worker-3,worker-4
and control-plane appears in that entire log exactly once, in a
`kubectl get nodes` listing. The recent run supplied control-plane as
well.
Nothing validated it. The workflow labels precisely what it is given:
for NODE in "${NODES[@]}"; do
kubectl label node "$NODE" io.simplyblock.node-type=simplyblock-storage-plane
and the storage-node DaemonSet selects purely on that label (verified on
the cluster: nodeSelector={io.simplyblock.node-type:
simplyblock-storage-plane}, no tolerations, no affinity), so a
control-plane name in the input silently joins the storage plane. On the
affected cluster all five nodes carried the label while only three
snode-spdk pods existed.
Two changes:
1. The labelling step now refuses any node carrying
node-role.kubernetes.io/control-plane, naming the offenders and
dumping `kubectl get nodes -L node-role.kubernetes.io/control-plane`
before exiting. Failing at the input beats discovering it later as an
unexplained extra storage node.
2. The WORKER_NODES fallback in k8s_major_upgrade.py was wrong in both
directions on Talos, at all five call sites:
kubectl get nodes -l node-role.kubernetes.io/worker ... || kubectl get nodes
Talos puts no role label on workers at all (confirmed: worker-1..4
have no node-role.kubernetes.io/* label, only control-plane does), so
the first command matches nothing, exits 0, and the `||` never fires
-- yielding an EMPTY worker list. And when the fallback does fire it
returns every node, control-plane included: the same defect by another
route.
Replaced with a documented _NON_CP_SELECTOR
("node-role.kubernetes.io/control-plane!="), the same selector the
security suite already uses. Against the real Talos node set the old
selector yields [] and the new one yields worker-1..4.
…odes
Correcting my earlier diagnosis. I assumed the Talos run had passed
control-plane in worker_nodes. The logs disprove that: the failing runs
(33874456135, 33878496856) passed exactly the same input as the
known-good run (33070003566) --
worker_nodes = worker-1,worker-2,worker-3,worker-4
and in every one of them the labelling step reports only
Labeled worker-1 .. Labeled worker-4
Control-plane was never labelled by any of these runs. It was carrying
io.simplyblock.node-type=simplyblock-storage-plane as residue from
earlier, and nothing has ever removed it. Since
simplyblock-storage-node-ds selects on that label alone (verified:
nodeSelector={io.simplyblock.node-type: simplyblock-storage-plane}, no
tolerations, no affinity), the node silently rejoined the storage plane
on every subsequent deploy.
Two cleanup gaps let it persist:
1. The workflow step "Remove stale storagenodeset labels from all worker
nodes" iterated only github.event.inputs.worker_nodes, so a node
outside that list was never touched -- and it stripped only
io.simplyblock.storagenodeset, never io.simplyblock.node-type. The
workflow had zero occurrences of `io.simplyblock.node-type-`.
It now walks every node from `kubectl get nodes` and strips
storagenodeset, node-type and simplyblock.io/role, plus the
prefix-keyed simplyblock.io/storage-node-uuid.* and
simplyblock.io/pool.* labels that accumulate one set per redeploy.
It prints the resulting labels so a future run shows its own state.
2. cleanup_upgrade_test.sh Phase 10 had the same defect: it looped
"${NODES[@]}", derived from WORKER_NODES, so a control-plane node was
never cleaned. It now derives the list from `kubectl get nodes` and
falls back to NODES only if that returns nothing.
The control-plane guard added in 4bc42d8 stays: it is still worth
refusing a control-plane name in worker_nodes, but it was defence in
depth, not the cause of this failure.
Stale label cleared on the affected Talos cluster.
_needs_db_migration returned True for ANY base != target, so a 26.2.8-PRE -> R26.3 upgrade ran the R25->R26 migration script unnecessarily (observed in run 33733403479). The script exists to backfill fields R26 introduced -- lvstore_ports, lvstore_stack_secondary, lvol_poller_mask, pollers_mask -- onto storage-node objects written by R25, and to rewrite lvol/snapshot objects in the new shape. On a cluster already running R26 those fields are present and correct, so running it there is at best pointless and at worst overwrites live values with recomputed ones. The old docstring also claimed the check skipped a "same base prefix" hotfix, but the code compared full equality, so even R25.10-Hotfix -> R25.10-Hotfix2 would have run it. Now gated on the base version starting with 25 (after stripping a leading "R"), and it logs the decision with both versions so the choice is visible in the run log. An unknown base_version no longer defaults to running the migration -- it skips and warns, since running it against R26 data is the harmful direction. Verified across six version pairs, including the pair from the affected run.
Both rolling upgrade sequences now live at the top of the test that
performs them, so the operator steps can be read (or handed to a
customer) without re-deriving them from the code and a CI log each time.
major_upgrade.py -- Docker/VM rolling R26.x -> R26.y, verified against run
33733403479 (26.2.8-PRE -> R26.3): pip install on every node, env_var
image pinning on the mgmt nodes, `cluster update --cp-only`, then the
per-node suspend / shutdown / deploy / restart loop, then
switch-write-protection followed by a second `--force` restart pass.
k8s_major_upgrade.py -- K8s rolling R26.x -> R26.y: helm upgrade of the
control plane, then per node a StorageNodeSet patch carrying the new
images followed by a StorageNodeOps action=restart, then the same
switch-write-protection and `--force` restart pass.
The R25 -> R26 maintenance-window path is deliberately not documented
here; it is a different flow and was not requested.
Three things the docstrings call out because they have each cost a run:
* Two images, not one. spdkImage and spdkProxyImage (--spdk-image and
--spdk-proxy-image) are different repositories with different tag
shapes ("ultra:R26.3-latest" vs "simplyblock:R26.3"), so neither can
be inferred from the other.
* The post-switch restart needs --force and NO image flags: the nodes
are already online, healthy and on the target images, so a plain
restart is refused as unnecessary.
* On K8s, image.csi.repository and image.csi.tag are separate Helm
settings. Passing a bare tag as the repository produces
"release-26.3.0:v26.2.6", which fails with "pull access denied"
because Docker resolves it as docker.io/library/release-26.3.0.
The K8s docstring also records that io.simplyblock.node-type is sticky
and must be cleaned from every node, not just worker_nodes.
In run k8s_native_failover_ha-20260904-151143 the suite failed with a
single error naming one job. The logs show 22 of ~28 FIO pods actually
had I/O errors -- 689 write, 2 read -- all inside one outage window. A
cluster-wide data-path failure was reported as one unlucky volume.
Two causes:
* validate_fio_jobs looped over pvc_details then clone_details calling
validate_fio_job directly, so the first RuntimeError aborted the loop
and every remaining volume went unchecked. It now validates all of
them, collects the failures, and raises once listing each affected
volume with its job name and error.
* validate_fio_job reported only the first `err=` it matched. It now
reports every distinct error code plus the io_u line count split by
read vs write, with a short sample. The read/write split is the first
thing needed to tell a failover-path problem from a data-placement
one, and "689 write, 2 read" is a very different signal from "err=5".
Also noted in the code: this check is reached even when the K8s Job
status is "succeeded" -- FIO can exit 0 for the Job while having reported
I/O errors internally, which is how the original failure surfaced.
…switch All eight are fallout from moving the suite onto the operator's StorageClass; none are DHCHAP product defects. Every enforcement assertion that passed before still passes. 1. Encrypted volumes were checked against the wrong pool label (TestLvolCryptoWithDhchap, TestLvolSecurityCombinations, TestLvolSecurityResize, TestLvolSecurityHAFailover) storageClassParameters is immutable per pool, so encrypted volumes get their own DHCHAP pool -- and therefore their own node label. The per-PV L4 assertion compared every volume against the MAIN pool's label, so all four encrypted cases failed with "nodeAffinity does not reference ...secpool". Volumes now record their own pool label at creation (_pvc_pool_label) and L4 asserts against that. 2. A non-DHCHAP pool request came back as the DHCHAP pool (TestLvolSecurityNegativeCreation) add_storage_pool's blind-reuse path returned an arbitrary existing pool whenever dhchap and allowed_nodes were both unset -- it ignored storage_class_parameters entirely. TC-SEC-103 therefore got the DHCHAP pool's StorageClass and correctly complained that it carried dhchap_node_label. Reuse now also requires no storage_class_parameters, since those select a StorageClass shape that cannot be changed later. 3. Clone PVCs never bound (TestLvolSecuritySnapshotClone) _create_clone_dual waits for Bound, but the operator StorageClass binds WaitForFirstConsumer, so a clone with no consumer never binds and the wait timed out after 300s. The K8s path now creates the clone claim and binds it with the same binder pod used for ordinary PVCs. 4. TC-SEC-112 was too strict (TestLvolSecurityNegativeConnect) It required a connect string when no --host-nqn is supplied. A DHCHAP pool refusing one entirely is stricter than returning a keyless string and still satisfies "no keys without host-nqn", so that outcome is now accepted and logged rather than failing. 5. Node selection relied on a field that is always zero (TestLvolSecurityStorageNodeOutage) The outage classes picked a target with `n["lvols"] > 0`. In K8s the REST storage-node payload reports lvols=0 for every node -- all 1498 occurrences in the run log -- while `sbctl sn list` shows the real counts (worker-2 and worker-4 each had 1). The filter matched nothing and the class died with "No primary storage nodes with lvols found". It now prefers a node reporting lvols and falls back to any online primary, warning when it has to fall back. The lvols discrepancy is worth raising with dev separately: the CLI and the REST API disagree about the same field at the same moment.
… test interface_full_network_interrupt was commented out of both outage_types lists and the multipath NIC block was short-circuited with a hard-coded use_multipath_outage = False, for a deliberate no-network-outage run. Restore both. The dispatch, HOST_LEVEL_OUTAGES / _HOST_UNREACHABLE_OUTAGES classification and the parent's npcs > 1 handling were all left intact while it was disabled, so nothing else needed changing.
sn dump-lvstore walks the whole lvstore on the SPDK app thread. Run against all 8 nodes at once during an outage recovery it held the thread for 1.1-1.5s at a stretch. Queued alceml IOs then sat undequeued for 4590ms, past the 4000ms _check_stuck_ios watchdog, which unregistered the bdev. The control plane read that unregister as a surprise hot-remove and retired a healthy device to the terminal "removed" state, leaving the cluster at 7 of 8 devices for 90 minutes and wedging every device_migration subtask. Gate it behind COLLECT_DOCKER_DUMP_LVSTORE so it is one flag to restore. fetch_distrib_logs still runs, so placement maps and stack dumps are unaffected, as is the whole k8s path.
Generalises COLLECT_DOCKER_DUMP_LVSTORE to COLLECT_DUMP_LVSTORE and gates the k8s branch of _collect_single_node_dump on it as well, so one flag covers both modes. On docker the dump was the trigger: it held the SPDK app thread long enough for the 4000ms _check_stuck_ios watchdog to unregister a healthy device. On k8s it was not the trigger, but it is a reliable casualty and it removes a node from service for minutes while it hangs. In k8s_native_resilient_failover-20260905-174555 the RPC to worker-2 never returned at all while the other five nodes finished in 18-24s, and the wrapper only gave up after 150s. Either way it is heavyweight enough to distort the runs it exists to diagnose. fetch_distrib_logs / fetch_distrib_logs_k8s still run on both paths, so placement maps and stack dumps are unaffected.
n_plus_k_failover_multi_client_ha_all_nodes-20260905-232656 failed with "NVMe reconnect did not converge within fault tolerance: clone_B289OXX3BQG5C2C: 0/3 succeeded" against a volume that was healthy the whole time. All three connects had returned "already connected", the device was present 4s later (/dev/nvme3n1, xfs_repair, mounted, FIO ran for the next 50 minutes), and the retry loop then spent 10 minutes getting the same reply before raising. "already connected" is the kernel declining a DUPLICATE controller for the same (hostnqn, subnqn, traddr, trsvcid): the path is already up. Two things made this systematic rather than a rare race: - connects use --ctrl-loss-tmo=-1, so the kernel never stops reconnecting and has almost always restored the path before retry_failed_nvme_connects runs. "already connected" is therefore the EXPECTED reply on retry, and a loop that only accepts empty stderr can never converge. - ns_id == 1 was read as "clone got a new subsystem, must connect". It does not mean that. The clone reported ns_id 1 with a subsystem NQN naming a different lvol (own uuid 12381b49, nqn ...:lvol:4d0c3986), i.e. namespace 1 of a subsystem the host already held a controller for. With max_namespace_per_subsys=30 that is normal placement. That run logged 63 "already connected" against 4 genuine "could not add new controller: connection refused". Changes: - TestClusterBase.nvme_connect_ok() classifies connect stderr, and _nqn_from_connect_cmds() extracts the NQN from a connect command. Both live on the shared base so every failover test inherits them. - All 7 record_failed_nvme_connect sites now gate on nvme_connect_ok instead of bare "if error". Two k8s sites already open-coded the check; they now use the shared helper. Audited: no bare "if error" deferral sites remain. - Clone connect decides connect-vs-rescan by comparing the reported subsystem NQN against the clone's own id, falling back to ns_id only when the NQN is unavailable. - Where a path was already up no NEW device appears in the before/after diff, so the lvol and clone paths in multi_client now resolve the device by NQN rather than raising LvolNotConnectException. multi_outage already had that fallback at Step 2. - record_failed_nvme_connect logs the actual stderr. The old message asserted "expected during outage" with no evidence, which is what pointed a real investigation at the product. Not changed: mke2fs version banners, xfs_repair progress and tmux "duplicate session" also land on stderr and get logged at ERROR, but exec_command only returns (out, err) without raising and no caller branches on them, so they are log noise rather than a functional bug.
…peline
Two changes, both aimed at stopping the harness from destabilising the cluster
it is measuring.
1. COLLECT_DISTRIB_PLACEMENT_DUMPS=False, gating fetch_distrib_logs on both the
docker and k8s paths, alongside the lvstore dump already disabled. These pull
a placement map plus a stack dump per distrib per node, so on an 8-node
cluster each collection round is dozens of RPCs plus docker/kubectl exec and
file copies. collect_outage_diagnostics now skips the whole parallel node-dump
fan-out when both collectors are off, rather than spawning threads and
creating empty node_dumps dirs.
2. Swap is now disabled on mgmt, storage and client nodes on every docker stress
run, in both stress-run-bootstrap.yml and stress-run-only.yml. The step runs
after the storage-node reboot and after the sbcli clone (it pipes
e2e/scripts/disable_swap.sh over ssh), so it is not undone by the reboot and
is in place before the stress run starts.
Why: the hosts were already out of memory headroom before the workload started.
First memory sample of run
n_plus_k_failover_multi_client_ha_all_nodes-20260905-232656, on 192.168.10.201:
Mem: 31Gi total, 30Gi used, 374Mi free, 339Mi available
Swap: 3.0Gi total, 171Mi used
No kernel OOM kill fired on any of the four hosts, which is why this never
surfaced as an obvious failure. But SPDK pins hugepages and is latency
critical: once the box swaps, poller threads stall and qpairs go delayed, and
in the logs that is indistinguishable from a storage fault. Keeping swap off
makes memory exhaustion fail loudly instead of quietly degrading into
something that reads as a product bug.
disable_swap.sh is idempotent, safe on a host with no swap, backs /etc/fstab up
once to /etc/fstab.bak-stress, comments swap entries while preserving the
original line so it can be restored, masks systemd .swap units (zram/zswap
setups do not use fstab), sets vm.swappiness=0, and prints the resulting swap
state. Host reboots between runs were already handled by the existing
"Reboot storage nodes" step in the bootstrap workflow.
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
6 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.