Skip to content

Design: node-side volume stack - #470

Merged
noctarius merged 4 commits into
mainfrom
design/volume-stack
Sep 4, 2026
Merged

noctarius merged 4 commits into
mainfrom
design/volume-stack

Conversation

@noctarius

Copy link
Copy Markdown
Collaborator

Adds a design document and its test plan for the node-side volume stack, plus
nothing else: no code changes, no CRD changes, no generated artifacts.

What the design covers

A persistent volume on a node is a stack of objects, each built on the one below
it. Today NodeStageVolume performs the connect, the format, and the mount
directly, which means every new node-side object becomes a conditional in every
data-path RPC. Client-side dedup and compression (#402) added an LVM physical
volume and a VDO logical volume to that chain, and pNFS adds two more shapes over
the same namespace.

The design replaces the conditionals with one interface. A layer is a reversible
transform with four verbs, Observe, Ensure, Release, and Destroy, and a
runner walks an ordered plan of them. The separation that matters most is
Release against Destroy: NodeUnstageVolume fires on an ordinary pod restart,
so a teardown path that removes durable objects there removes data on a pod
restart. That is the defect #402 fixed, and the contract is shaped so it cannot
be written again.

Phased so each phase ships alone:

Phase Scope Behavior change
1 The contract, the runner, the stack record, fabric and filesystem None. RWO parity with today
2 lvmPV and lvmVolume, #402's mechanics moved behind the contract None. VDO parity with #402
3 Healer and Grower, so heal, restage, and expand walk the stack Heal and expand become correct for every layer
4 Node requirements derived from the plan, controller side Topology gating stops being hand-written per feature

Review focus

The two sections that carry the risk, called out as such in §15:

  • §4.2 State — a misclassification formats a volume that had data.
  • §7.3 the unwind rule — a failed bring-up releases and never destroys.

Also worth a close read:

  • §6 and §6.1 — the stack record: host-local, written before the first side
    effect, atomic, and holding parameters rather than device paths. §6.1 specifies
    the on-disk JSON.
  • §17 Open Questions — six, of which Q2 (whether the LVM layers share one
    lock key) is an empirical question waiting on a load test rather than a
    decision.

Known gaps

  • Nine links resolve only on pnfs-design. The document references
    design-pnfs-rwx.md and design-pnfs-striped.md, which are not on this
    branch. They resolve once that branch lands.
  • One open question in §6. The record is keyed
    stacks/<volume-handle>.json, one file per volume per host. That holds only if
    a node has at most one role per volume. If an MDS host can also consume the
    volume through the client stack, the path needs the role in it.

Verification

house-style/scripts/quality-gate.sh passes all eight gates on both files.

Copilot AI lite review requested due to automatic review settings August 26, 2026 09:12

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a design document and companion test plan describing a layered, plan-driven “node-side volume stack” abstraction for the CSI node service (to replace per-RPC conditionals for VDO, pNFS shapes, raw block vs filesystem, etc.). This is documentation-only and does not change code, CRDs, or generated artifacts.

Changes:

  • Introduces the design doc defining the Layer contract (Observe/Ensure/Release/Destroy), State model, stack record format, runner semantics, and phased rollout plan.
  • Adds a detailed test plan with scenario matrix, axis coverage, and manual test concepts for concurrency/failure-injection gaps.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
operator/docs/designs/design-node-volume-stack.md New design doc specifying the layered stack model, runner semantics, host-local stack record, and phased delivery plan.
operator/docs/tests/test-plan-node-volume-stack.md New test plan enumerating unit/integration/e2e/manual scenarios and coverage axes for validating the design once implemented.
Suppressed comments (10)

operator/docs/tests/test-plan-node-volume-stack.md:32

  • In the Axes Selected table, the Failure domains row points to the Phase 4 block in §5, but the Phase 4 section is actually §4 ("Unit Tests — Phase 4 (Planned)").
| D. Failure domains          | No            | The node service performs no placement. Node eligibility is design §10 and is covered by the Phase 4 block in §5                                                                                                                                     |

operator/docs/tests/test-plan-node-volume-stack.md:173

  • This heading mixes the "design §n" convention with a bare "§9", which reads like a reference to this test plan (per the legend at the top). Make both references explicitly point at the design doc.
### Optional Interface Dispatch (design §4.4, §9)

operator/docs/tests/test-plan-node-volume-stack.md:401

  • The Unit scenario count in the coverage summary does not match the scenario IDs listed above (there is no U-59, so U-01…U-85 is 84 scenarios total). This makes the summary inconsistent with the matrix.
| Unit           | 85        | 3       | U-01 … U-70, U-73 … U-76, U-78 … U-85 |

operator/docs/tests/test-plan-node-volume-stack.md:428

  • The gap range "U-48 … U-59" doesn't match the matrix above (the stack record block ends at U-58, and there is no U-59).
| U-48 … U-59       | The stack record                                      | Phase 1                                                                                                                                         |

operator/docs/tests/test-plan-node-volume-stack.md:389

  • This reference uses "§Axes Selected", but the document defines "§n" for numeric section references only. Using a non-numeric "§" reference is inconsistent and may confuse readers.
| D. Failure domains      | —                                                                                                                     | —                                   | Excluded: the node service performs no placement (§Axes Selected) |

operator/docs/tests/test-plan-node-volume-stack.md:437

  • This gap row references "§Axes Selected", but the document defines "§n" as numeric section references. Refer to the section name without a § to keep the legend consistent.
| —                 | Failure domains and placement topology                | The node service performs no placement (§Axes Selected)                                                                                         |

operator/docs/designs/design-node-volume-stack.md:137

  • This link points to design-pnfs-striped.md, but that file is not present in this branch, so the link is currently broken. Consider referencing the filename without making it a hyperlink until the pNFS design docs land.
- **Striped pNFS** ([`design-pnfs-striped.md`](design-pnfs-striped.md) §2.1, §2.2)

operator/docs/tests/test-plan-node-volume-stack.md:141

  • This heading lists one design section reference as "design §5.3" but the second as a bare "§5.4", which can be read as a reference to this test plan. Repeat the "design" prefix for clarity (consistent with the legend at the top).
### LVM Naming and Primitives (design §5.3, §5.4)

operator/docs/tests/test-plan-node-volume-stack.md:197

  • This heading mixes a "design §n" reference with a bare "§7", which is ambiguous given the legend that "§n" refers to this plan. Make both references explicitly point to the design doc.
### Crash and Resume (design §6, §7)

operator/docs/tests/test-plan-node-volume-stack.md:253

  • This heading uses "design §3" and then a bare "§5", which is ambiguous per the legend that "§n" refers to this plan. Make the second reference explicitly "design §5".
### Stacked Plans (design §3, §5)

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread operator/docs/tests/test-plan-node-volume-stack.md Outdated
Comment thread operator/docs/designs/design-node-volume-stack.md Outdated
@noctarius noctarius self-assigned this Aug 26, 2026
@noctarius noctarius added this to the 26.4 milestone Aug 26, 2026
@noctarius
noctarius force-pushed the design/volume-stack branch from e8ea5c2 to 57740e9 Compare August 28, 2026 13:52
noctarius and others added 4 commits August 28, 2026 16:25
The §13 row read "unparseable", which is the British pattern of keeping the
silent e before -able. American English drops it, which is what the house style
wordlist already encodes for every comparable pair: useable to usable, moveable
to movable, sizeable to sizable. "parse" is not a soft-c or soft-g stem, so it
takes the same path.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
…plementation

The document under this filename described a generalized, pluggable
node-side-stack framework that was never built. Replace it with a design
doc and test plan describing the actual mechanism PR #402 implements:
StoragePool-level VDO compression/deduplication built on atlas-lib/lvm
and atlas-lib/lvm/vdo (PR #457), node capability gating, and CSI wiring.
Rename both files from design-node-volume-stack.md / test-plan-node-volume-stack.md
to design-client-side-vdo-compression.md / test-plan-client-side-vdo-compression.md
to match.
@noctarius
noctarius merged commit 329e34f into main Sep 4, 2026
15 checks passed
@noctarius
noctarius deleted the design/volume-stack branch September 4, 2026 20:01
noctarius added a commit that referenced this pull request Sep 4, 2026
These two documents reached no branch that survived. The branch that carried them
was rewritten, and the final commit on the rewritten copy retitled them as
design-client-side-vdo-compression.md and test-plan-client-side-vdo-compression.md.
That narrower pair covers the shipped VDO feature, and it is what reached main
in #470.

They are separate documents rather than earlier drafts of the VDO pair. The stack
design specifies the layer contract, the layer catalog, the stack record, and the
bring-up and bring-down verbs every node-side layer shares, and two files under
.claude/skills still cite its section 14 as an observability reference. Restoring
them adds both files back and leaves the VDO documents in place.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants