Skip to content

VSC-only snapshot model in snapshot-controller and csi-snapshotter#27

Open
duckhawk wants to merge 1 commit into
mainfrom
feat/vsc-only-snapshot-model
Open

VSC-only snapshot model in snapshot-controller and csi-snapshotter#27
duckhawk wants to merge 1 commit into
mainfrom
feat/vsc-only-snapshot-model

Conversation

@duckhawk
Copy link
Copy Markdown
Member

Description

This PR adds support for a VSC-only snapshot model in the snapshot-controller and the
csi-snapshotter external-snapshotter sidecar shipped by storage-foundation.

With this change, VolumeSnapshotContent objects can be created and processed without
a corresponding VolumeSnapshot. This enables internal controllers (e.g. storage-foundation
VCR/VRR) to perform CSI snapshot and restore operations using service-level APIs without
exposing VolumeSnapshot resources to user namespaces.

The implementation is backward-compatible and preserves existing behavior for legacy
VolumeSnapshot-based workflows.

This change does not trigger restarts of control-plane components, ingress controllers,
or other critical cluster services.

This PR mirrors deckhouse/snapshot-controller#34 in the storage-foundation module.

Why do we need it, and what problem does it solve?

Some internal Deckhouse components (virtualization, managed services, backup infrastructure)
require the ability to:

  • create CSI snapshots of PVs,
  • detach PVs from PVCs,
  • restore PVCs from snapshots or detached PVs,

without creating VolumeSnapshot or temporary PVC objects in user namespaces.

The standard CSI snapshot workflow enforces a strict VolumeSnapshot ↔ VolumeSnapshotContent
binding, which makes it unsuitable for service-level, short-lived operations where:

  • user-visible resources are undesirable,
  • additional RBAC exposure is unacceptable,
  • namespaces may be deleted shortly after the operation.

This PR removes the hard requirement for VolumeSnapshotRef in VolumeSnapshotContent
and allows the controller and sidecar to work directly with VSC as the source of truth,
enabling a clean, secure, and minimal service workflow.

What is the expected result?

After applying these changes:

  • VolumeSnapshotContent can be created without `spec.volumeSnapshotRef`.
  • The CRD (`crds/snapshot.storage.k8s.io_volumesnapshotcontents.yaml`) no longer lists
    `volumeSnapshotRef` in `required`, and its `x-kubernetes-validations` rule allows
    the field to be entirely empty (VSC-only) while keeping the legacy name+namespace
    constraint when it is set.
  • snapshot-controller (`images/snapshot-controller`):
    • accepts and reconciles VSC-only objects,
    • adds finalizers and handles deletion correctly,
    • preserves legacy behavior when `VolumeSnapshotRef` is present.
  • csi-snapshotter sidecar (`images/csi-external-snapshotter`):
    • performs `CreateSnapshot` / `DeleteSnapshot` for VSC-only objects,
    • derives snapshot identity from `VolumeSnapshotContent.UID` when no
      `VolumeSnapshot` exists.
  • No `VolumeSnapshot` objects are created implicitly.
  • Existing `VolumeSnapshot`-based workflows continue to work unchanged.

Correctness can be verified by:

  • creating a `VolumeSnapshotContent` without `volumeSnapshotRef`,
  • observing successful CSI `CreateSnapshot` and `ReadyToUse=true`,
  • deleting the VSC and observing CSI `DeleteSnapshot` execution.

Implementation notes

The Go-side changes are delivered as a single patch applied to upstream
`kubernetes-csi/external-snapshotter` v8.5.0:

  • `images/snapshot-controller/patches/003-vsc-only-model.patch`
  • `images/csi-external-snapshotter/patches/v8.5.0/003-vsc-only-model.patch`

Both image patch directories ship the identical patch so that the
`snapshot-controller` and `csi-snapshotter` binaries are built from
the same source tree.

The patch touches:

  • `pkg/common-controller/snapshot_controller.go` — VSC-only branch in `syncContent`,
    finalizer handling, legacy fallback when `volumeSnapshotRef` is set.
  • `pkg/sidecar-controller/csi_handler.go` — fall back to `content.UID` when
    `volumeSnapshotRef.UID` is empty when generating the CSI snapshot name.
  • `pkg/sidecar-controller/snapshot_controller.go` — VSC-only handling in
    `syncContent` and `shouldDelete`, extra logging.
  • `pkg/sidecar-controller/framework_test.go` — minor test helper fix to ignore
    `DeletionTimestamp` time-precision in equality checks.
  • `pkg/sidecar-controller/vsc_only_test.go` — new unit tests covering VSC-only and
    legacy paths.

The patch was verified by:

  • `git apply` against a clean `v8.5.0` checkout (in addition to the existing
    001/002 CVE patches);
  • `go build ./...`;
  • `go test ./pkg/sidecar-controller/... ./pkg/common-controller/...`.

Checklist

  • The code is covered by unit tests (including VSC-only scenarios).
  • e2e tests passed.
  • Documentation updated according to the changes (to be done in storage-foundation ADRs).
  • Changes were tested in the Kubernetes cluster manually.

Add support for a VSC-only snapshot model in snapshot-controller and
the csi-snapshotter sidecar shipped by storage-foundation.

VolumeSnapshotContent objects can now be created and reconciled
without a corresponding VolumeSnapshot. This enables internal Deckhouse
controllers (e.g. storage-foundation VCR/VRR) to drive CSI snapshot
and restore operations via service-level APIs without exposing
VolumeSnapshot resources to user namespaces.

Changes:
- crds/snapshot.storage.k8s.io_volumesnapshotcontents.yaml:
  spec.volumeSnapshotRef is no longer in 'required' and its
  x-kubernetes-validations rule allows the field to be completely
  empty (VSC-only) while keeping the legacy name+namespace constraint
  when it is set.
- doc-ru CRD updated to match.
- images/snapshot-controller/patches/003-vsc-only-model.patch and
  images/csi-external-snapshotter/patches/v8.5.0/003-vsc-only-model.patch:
  patch external-snapshotter v8.5.0 so that common-controller and the
  csi-snapshotter sidecar:
  * reconcile VSCs with empty volumeSnapshotRef as VSC-only,
  * derive CSI snapshot identity from VolumeSnapshotContent.UID when
    volumeSnapshotRef.UID is empty,
  * perform CreateSnapshot / DeleteSnapshot for VSC-only objects,
  * keep the existing finalizer and deletion semantics,
  * preserve full backward compatibility for VSCs that still carry
    volumeSnapshotRef.
- patches/README.md files updated to describe the new patch.

The change does not trigger restarts of control-plane components,
ingress controllers, or other critical cluster services.

Mirrors deckhouse/snapshot-controller#34.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant