docs: design for SeiNode import existing storage#106
Merged
Conversation
Tracks #105. Direction-setting doc for adding first-class "import existing volume" support to SeiNode, rather than relying on the `ensure-data-pvc` `Create`-then-`AlreadyExists` code path (which is exactly the bug in #104). Decision: adopt Shape A (import PVC by name) + Shape D (import PV by name) together. Defer Shape E (VolumeSnapshot) to a later pass when snapshot-restore is specifically required. Reject Shape C (EBS snapshot ID with controller-side CreateVolume) to avoid making the controller cloud-provider-aware. The import-path design forces the validation logic missing from #104 — same refactor fixes both. Open questions for the LLD: deletion semantics for imported volumes, scope of validation (mutate vs. read-only-fail), and init-plan interaction with bootstrap-Job progression. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bdchatham
commented
Apr 20, 2026
Three changes from #106 review: 1. Scope down to Shape A only. Drop Shape D from the Decision; move to "Additional options considered" as evaluated-but-not-adopted. Orphan adoption is reachable via Shape A with one extra operator step (create a PVC bound to the orphan PV, then reference it). Less CRD surface, less task-path branching. 2. Flip deletion semantics to "always preserve imported PVCs." deleteNodeDataPVC finalizer is a no-op for imported volumes. Operators opted into import explicitly; controller should not touch storage it didn't create. Moved from open question to decided section. 3. Turn validation requirements from open question into a concrete table. Seven requirements the imported PVC must satisfy (exists, not terminating, Bound, RWO, capacity, PV integrity). Explicit list of what we do NOT check (fstype, content, labels, SC) with reasoning. Two open questions remain for the LLD (bootstrap-plan interaction, validation retry budget). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Per PR feedback: keep the design as idiomatic to the existing code path as possible. Import does not change the init plan; it only changes what ensure-data-pvc does internally. - When spec.dataVolume.import.pvcName is set, ensure-data-pvc verifies the named PVC rather than creating one. Every other task in the init plan runs unchanged. - No skipBootstrap flag, no branching in the planner, no diverged reconcile progression. - The operator is trusted to provide a PVC whose contents are compatible with the rest of the init progression. Failures surface through the normal plan-Failed channel. This removes open question #1 (bootstrap-plan interaction) entirely. Only one open question remains: validation retry budget semantics. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
Addressed your latest feedback in c86790b:
Only one open question remains for the LLD (retry budget semantics). |
Collaborator
Author
|
Pushed 98a1179 — retry semantics decided: retry indefinitely with exponential backoff, surface via Condition. All direction-level open questions are now resolved. LLD work is implementation specifics (backoff curve, Condition schema, tests). Ready for merge whenever you're happy with the direction. |
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
1 participant
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.
Tracks #105. Related: #104.
Summary
Direction-setting doc for adding first-class "import existing volume" support to
SeiNode, so users can stage a pre-populated PVC/PV and reference it from the SeiNode spec — rather than relying on the currentensure-data-pvcCreate()-then-AlreadyExistsaccident (which is #104).Decision in the doc: adopt Shape A (`import.pvcName`) + Shape D (`import.pvName`) together. Defer Shape E (
VolumeSnapshotCR) to a later pass once snapshot-restore is directly needed. Reject Shape C (EBS snapshot ID with controller-sideCreateVolume) to avoid making the controller cloud-provider-aware.Why this is just the design doc (no code)
This PR is deliberately narrow — prose + tables, no CRD changes, no task implementation. Goal is to get directional sign-off inline via comments, then a follow-up PR with the full LLD and implementation.
What's left open
Three open questions called out at the bottom of the doc — deletion semantics, validation scope (mutate vs. read-only-fail), init-plan interaction with the bootstrap Job. Each has my current lean, but all three are genuinely open for the LLD.
Review ask
Test plan
N/A — documentation only. If the direction lands, follow-up PR does the LLD + implementation with unit tests.
🤖 Generated with Claude Code