Skip to content

Prevent concurrent DeleteVolume/DeleteSnapshot during CreateVolume - #6322

Open
nixpanic wants to merge 2 commits into
ceph:develfrom
nixpanic:source-vol-locking
Open

Prevent concurrent DeleteVolume/DeleteSnapshot during CreateVolume #6322
nixpanic wants to merge 2 commits into
ceph:develfrom
nixpanic:source-vol-locking

Conversation

@nixpanic

@nixpanic nixpanic commented Jun 4, 2026

Copy link
Copy Markdown
Member

Describe what this PR does

There is a race condition possible when CreateVolume uses a source volume/snapshot and the source is deleted at the same time. The creation of the volume may fail in weird ways. By grabbing a lock for the source volume/snapshot, a concurrent DeleteVolume procedure has to wait until the CreateVolume procedure has finished.

Related issues

Fixes: #6321

Note: NFS does not use the source volume/snapshot in any way, it forwards the CreateVolume call on to the CephFS Controller. There is no need for the added locking in NFS. NVMe-oF only gets the Clone/Restore functionality through the work-in-progress #6277.


Show available bot commands

These commands are normally not required, but in case of issues, leave any of
the following bot commands in an otherwise empty comment in this PR:

  • /retest ci/centos/<job-name>: retest the <job-name> after unrelated
    failure (please report the failure too!)

@nixpanic
nixpanic requested a review from a team June 4, 2026 14:44
@nixpanic nixpanic added component/cephfs Issues related to CephFS component/rbd Issues related to RBD labels Jun 4, 2026
Comment thread internal/rbd/controllerserver.go
@nixpanic
nixpanic force-pushed the source-vol-locking branch from 231b980 to f63a16c Compare June 10, 2026 09:55
@nixpanic
nixpanic requested review from a team and iPraveenParihar June 10, 2026 09:55
Comment thread internal/cephfs/controllerserver.go Outdated
@nixpanic
nixpanic requested a review from black-dragon74 July 23, 2026 08:04
black-dragon74
black-dragon74 previously approved these changes Jul 23, 2026

@black-dragon74 black-dragon74 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

GH is still processing the updates

@nixpanic
nixpanic force-pushed the source-vol-locking branch from f63a16c to cfc44cd Compare July 23, 2026 08:46
@mergify
mergify Bot dismissed black-dragon74’s stale review July 23, 2026 08:48

Pull request has been modified.

@iPraveenParihar

Copy link
Copy Markdown
Contributor

@Mergifyio rebase

@mergify

mergify Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

rebase

🛑 The pull request rule doesn't match anymore

Details

This action has been cancelled.

@iPraveenParihar

Copy link
Copy Markdown
Contributor

@Mergifyio queue

@mergify

mergify Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Merge Queue Status

  • 🟠 Waiting for queue conditions
  • ⏳ Enter queue
  • ⏳ Run checks
  • ⏳ Merge
Waiting for
  • github-review-approved [🛡 GitHub branch protection]
  • #changes-requested-reviews-by=0
  • approved-reviews-by=@ceph/ceph-csi-maintainers
  • any of:
    • label=ci/skip/e2e
    • all of:
      • base~=^(release-.+)$
      • status-success=ci/centos/k8s-e2e-external-storage/1.33
      • status-success=ci/centos/k8s-e2e-external-storage/1.34
      • status-success=ci/centos/k8s-e2e-external-storage/1.35
      • status-success=ci/centos/mini-e2e-helm/k8s-1.33
      • status-success=ci/centos/mini-e2e-helm/k8s-1.34
      • status-success=ci/centos/mini-e2e-helm/k8s-1.35
      • status-success=ci/centos/mini-e2e/k8s-1.33
      • status-success=ci/centos/mini-e2e/k8s-1.34
      • status-success=ci/centos/mini-e2e/k8s-1.35
      • status-success=ci/centos/upgrade-tests-cephfs
      • status-success=ci/centos/upgrade-tests-rbd
    • all of:
      • base=release-v3.16
      • status-success=ci/centos/k8s-e2e-external-storage/1.32
      • status-success=ci/centos/k8s-e2e-external-storage/1.33
      • status-success=ci/centos/k8s-e2e-external-storage/1.34
      • status-success=ci/centos/mini-e2e-helm/k8s-1.32
      • status-success=ci/centos/mini-e2e-helm/k8s-1.33
      • status-success=ci/centos/mini-e2e-helm/k8s-1.34
      • status-success=ci/centos/mini-e2e/k8s-1.32
      • status-success=ci/centos/mini-e2e/k8s-1.33
      • status-success=ci/centos/mini-e2e/k8s-1.34
      • status-success=ci/centos/upgrade-tests-cephfs
      • status-success=ci/centos/upgrade-tests-rbd
    • all of:
      • status-success=ci/centos/k8s-e2e-external-storage/1.34
      • status-success=ci/centos/k8s-e2e-external-storage/1.35
      • status-success=ci/centos/k8s-e2e-external-storage/1.36
      • status-success=ci/centos/mini-e2e-helm/k8s-1.34
      • status-success=ci/centos/mini-e2e-helm/k8s-1.35
      • status-success=ci/centos/mini-e2e-helm/k8s-1.36
      • status-success=ci/centos/mini-e2e/k8s-1.34
      • status-success=ci/centos/mini-e2e/k8s-1.35
      • status-success=ci/centos/mini-e2e/k8s-1.36
      • status-success=ci/centos/upgrade-tests-cephfs
      • status-success=ci/centos/upgrade-tests-rbd
    • all of:
      • base=ci/centos
      • status-success=ci/centos/jjb-validate
      • status-success=ci/centos/job-validation
All conditions
  • any of [🔀 queue conditions]:
    • all of [📌 queue conditions of queue rule default]:
      • github-review-approved [🛡 GitHub branch protection]
      • all of:
        • #changes-requested-reviews-by=0
        • approved-reviews-by=@ceph/ceph-csi-maintainers
        • any of:
          • all of:
            • label=ci/skip/e2e
            • base!=ci/centos
          • all of:
            • base~=^(release-.+)$
            • status-success=ci/centos/k8s-e2e-external-storage/1.33
            • status-success=ci/centos/k8s-e2e-external-storage/1.34
            • status-success=ci/centos/k8s-e2e-external-storage/1.35
            • status-success=ci/centos/mini-e2e-helm/k8s-1.33
            • status-success=ci/centos/mini-e2e-helm/k8s-1.34
            • status-success=ci/centos/mini-e2e-helm/k8s-1.35
            • status-success=ci/centos/mini-e2e/k8s-1.33
            • status-success=ci/centos/mini-e2e/k8s-1.34
            • status-success=ci/centos/mini-e2e/k8s-1.35
            • status-success=ci/centos/upgrade-tests-cephfs
            • status-success=ci/centos/upgrade-tests-rbd
          • all of:
            • base=release-v3.16
            • status-success=ci/centos/k8s-e2e-external-storage/1.32
            • status-success=ci/centos/k8s-e2e-external-storage/1.33
            • status-success=ci/centos/k8s-e2e-external-storage/1.34
            • status-success=ci/centos/mini-e2e-helm/k8s-1.32
            • status-success=ci/centos/mini-e2e-helm/k8s-1.33
            • status-success=ci/centos/mini-e2e-helm/k8s-1.34
            • status-success=ci/centos/mini-e2e/k8s-1.32
            • status-success=ci/centos/mini-e2e/k8s-1.33
            • status-success=ci/centos/mini-e2e/k8s-1.34
            • status-success=ci/centos/upgrade-tests-cephfs
            • status-success=ci/centos/upgrade-tests-rbd
          • all of:
            • status-success=ci/centos/k8s-e2e-external-storage/1.34
            • status-success=ci/centos/k8s-e2e-external-storage/1.35
            • status-success=ci/centos/k8s-e2e-external-storage/1.36
            • status-success=ci/centos/mini-e2e-helm/k8s-1.34
            • status-success=ci/centos/mini-e2e-helm/k8s-1.35
            • status-success=ci/centos/mini-e2e-helm/k8s-1.36
            • status-success=ci/centos/mini-e2e/k8s-1.34
            • status-success=ci/centos/mini-e2e/k8s-1.35
            • status-success=ci/centos/mini-e2e/k8s-1.36
            • status-success=ci/centos/upgrade-tests-cephfs
            • status-success=ci/centos/upgrade-tests-rbd
            • base=devel
          • all of:
            • base=ci/centos
            • status-success=ci/centos/jjb-validate
            • status-success=ci/centos/job-validation
        • #approved-reviews-by>=2
        • approved-reviews-by=@ceph/ceph-csi-contributors
        • label!=DNM
        • status-success=DCO
        • any of:
          • all of:
            • base!=ci/centos
            • status-success=codespell
            • status-success=go-test
            • status-success=golangci-lint
            • status-success=lint-extras
            • status-success=mod-check
            • status-success=multi-arch-build
            • status-success=uncommitted-code-check
            • any of:
              • status-success=commitlint
              • author=dependabot[bot]
          • base=ci/centos
  • -closed [📌 queue requirement]
  • -conflict [📌 queue requirement]
  • -draft [📌 queue requirement]
  • any of [📌 queue -> configuration change requirements]:
    • -mergify-configuration-changed
    • check-success = Configuration changed
  • any of [📌 queue requirement]:
    • check-neutral = Mergify Merge Protections
    • check-skipped = Mergify Merge Protections
    • check-success = Mergify Merge Protections

@iPraveenParihar

Copy link
Copy Markdown
Contributor

@Mergifyio rebase

@mergify

mergify Bot commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

rebase

🛑 The pull request rule doesn't match anymore

Details

This action has been cancelled.

nixpanic added 2 commits July 28, 2026 13:35
When creating a volume from a source (either cloning from a volume or
restoring from a snapshot), acquire locks on the source to prevent
concurrent operations that could interfere with the clone/restore process.

This prevents race conditions where the source volume or snapshot could
be modified or deleted while being used as a source for creating a new
volume.

Assisted-by: AskBob <askbob@ibm.com>
Signed-off-by: Niels de Vos <ndevos@ibm.com>
When creating a volume from a source (either cloning from a volume or
restoring from a snapshot), acquire locks on the source to prevent
concurrent operations that could interfere with the clone/restore process.

This prevents race conditions where the source volume or snapshot could
be modified or deleted while being used as a source for creating a new
volume.

Assisted-by: AskBob <askbob@ibm.com>
Signed-off-by: Niels de Vos <ndevos@ibm.com>
@nixpanic

Copy link
Copy Markdown
Member Author

Note that #6277 is already running CI jobs.

@Rakshith-R Rakshith-R 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.

Doesn't this also block concurrent create ops on same parent ?
and therefore slow create op.

@Rakshith-R

Copy link
Copy Markdown
Contributor

Doesn't this also block concurrent create ops on same parent ? and therefore slow create op.

Option 2: Leave As-Is (rely on idempotency)

ext-provisioner and ext-snapshotter are responsible for adding finalizer on source pvc/snap.

PVC-A (vol-123) exists and is mounted
User creates PVC-B as clone of PVC-A
User deletes PVC-A while clone is in progress

Delete should get blocked since the pvc is still mounted.
I don't think this a valid scenario.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/cephfs Issues related to CephFS component/rbd Issues related to RBD

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Race Condition During Concurrent Clone and Delete (CephFS & RBD)

5 participants