Restore volumes via real CSI provisioning instead of hand-built PVs

[boddumanohar]

Situation

BackupRestoreReconciler bypasses normal CSI dynamic provisioning entirely. ensurePV() hand-constructs the restored PersistentVolume object directly (internal/controller/backuprestore_controller.go) instead of letting a real PVC/StorageClass trigger the CSI driver's CreateVolume. ensurePVC() then statically binds a PVC to it via spec.volumeName.

This is necessary because the CSI spec's CreateVolumeRequest.VolumeContentSource only has two cases — Snapshot and Volume (clone) — and both assume the source lives, right now, in the same cluster. Confirmed in spdk-csi's handleVolumeContentSource: it switches on exactly these two types and nothing else. There is no CSI primitive for "materialize a volume from an S3-archived, potentially cross-cluster-imported backup." Getting a third VolumeContentSource case into the actual upstream CSI spec is a cross-vendor standardization effort, not something we can do unilaterally.

The cost of the current workaround: every property that CSI dynamic provisioning would normally derive for you (from the driver's CreateVolume response and the StorageClass) has to be hand-copied into the manually-built PV, field by field, forever, or it's silently wrong.

• Already hit: the restored PV never set CSIPersistentVolumeSource.FSType, so kubelet always fell back to the CSI driver's ext4 default regardless of the source volume's actual filesystem. Restoring an XFS-formatted backup produced a PV that mount(8) rejected outright ("wrong fs type, bad superblock") — TestBackupFilesystemXFS. Patched by threading FSType through StorageBackup.Status → BackupRestore.Status → the PV (see the PR that closed this).
• Still open, found while fixing the above: the same hand-built PV also never sets NodeAffinity/AccessibleTopology (normally populated from the driver's CreateVolume response for topology-aware scheduling) or MountOptions (normally from the StorageClass). Nothing exercises these yet, but they're the exact same bug shape waiting to be found by the next test that does.

Patching these one property at a time is treating the symptom. The root cause is bypassing CreateVolume at all.

Solution

Don't extend the CSI spec (not ours to change) and don't require users to adopt Kubernetes's generic dataSourceRef/Volume Populator feature (bigger surface change, feature-gate dependency, and a UX change away from today's BackupRestore CRD). Instead, make BackupRestoreReconciler drive real CSI provisioning internally, in two phases, with no change to the CRD's external API:

1. Provision — create a disposable "shadow" PersistentVolumeClaim against a StorageClass parametrized for this backup's pool and filesystem type (StorageBackup.Status.FSType, already captured today). This is a completely ordinary PVC — no special annotations — so it goes through real dynamic provisioning: external-provisioner calls the CSI driver's actual CreateVolume, and FSType, MountOptions, and AccessibleTopology/NodeAffinity all land on the resulting PV correctly, for free, exactly the way they do for any other PVC in the cluster.
2. Populate — once the shadow PVC is Bound, extract the lvol ID from the bound PV's CSI volume handle, perform cross-cluster source-switch if needed (unchanged from today), then call the backend to recover the backup's S3 data chain into that already-existing lvol (see Status/Dependencies — this needs a small backend API change, since today's restore endpoint always creates a brand new lvol as part of triggering recovery). Poll to completion exactly as today.
3. Swap — patch the shadow PV's reclaim policy to Retain, retarget its claimRef to the real PVC named by spec.pvcTemplate, delete the now-unclaimed shadow PVC (safe: Retain means this never triggers DeleteVolume), and let the final PVC bind to that PV the normal way. From here, reconciliation continues exactly as it does today (PVCBinding → Done).

Nothing mounts the shadow PVC as a pod volume at any point, so kubelet's format-on-first-mount logic never fires against it — data population happens directly against the raw block device via the backend API, before any pod (including the eventual real one) ever calls NodeStageVolume. By the time the real pod mounts the final PVC, blkid finds the backup's actual filesystem already in place and mounts it as-is, matching today's "restore without reformat" behavior.

Behaviour after this change

• BackupRestore's spec/status API is unchanged. This is a purely internal reimplementation — existing manifests/automation keep working.
• Cross-cluster source-switch phases (SwitchingSource/SwitchingSourceLocal) are unchanged; they just happen between provision and populate instead of around a single restore call.
• FSType, MountOptions, and NodeAffinity/AccessibleTopology are now always correct by construction, closing the whole bug class instead of one field at a time.
• spdk-csi needs no changes — its CreateVolume already works generically for any pool via a StorageClass; it has no concept of "restore" at all in this design, which is the point.
• sbcli/core needs a new capability: recover backup data into an already-existing lvol, as a step distinct from "create lvol + recover" bundled together (today's restore_backup() in simplyblock_core/controllers/backup_controller.py always does both). This isn't a new capability at the SPDK layer — bdev_lvol_create and bdev_lvol_s3_recovery are already separate RPCs called separately (see simplyblock_core/services/tasks_runner_backup.py::_run_restore) — it's an API-surface change to make that split callable independently.

Status / Dependencies

• Depends on an sbcli core API change (new endpoint, or a target_lvol_id parameter on the existing restore endpoint) to decouple lvol creation from S3 data recovery.
• No spdk-csi changes required.
• Related: FSType propagation fix (merged/pending as its own PR) is the immediate motivating bug; this issue is the durable fix that makes fixes like that unnecessary going forward. The NodeAffinity/AccessibleTopology/MountOptions gap mentioned above is not separately tracked — it's closed as a side effect of this design, not worth fixing standalone first.

Open questions

• StorageClass proliferation: do we create one StorageClass per (pool, fsType) permanently and leave them, or garbage-collect ones no longer referenced by any in-flight restore?
• Crash/resume safety: the shadow PVC's name needs to be tracked in BackupRestore.Status so a reconciler restart mid-populate or mid-swap can resume idempotently instead of leaking a shadow PVC or double-provisioning.
• Observability: should provisioning/populating/swapping each get an explicit CRD phase (e.g. a new Provisioning / Populating phase between today's Pending and InProgress), or fold into the existing phases?
• Phase 2 (optional, out of scope for this issue): once the internal mechanism exists, should we also expose it via the standard Kubernetes dataSourceRef on a plain PVC (the "Volume Populator" pattern) for users who'd rather not go through the BackupRestore CRD at all? Not required to fix the bug class above.

🤖 Filed with Claude Code