Under certain conditions, a DeleteSnapshot will never get issued for a deleted volumesnapshot

[akalenyu]

What happened:
Some race that I have yet to figure out will cause the sidecar controller to opt out of sending a
DeleteSnapshot and thus the volumesnapshot/content to stay pending in deletion indefinitely
(including waiting out the resync period)

What you expected to happen:
volumesnapshot/content requeued on some condition so eventually deleted properly

How to reproduce it:
seems tough, but something along the lines of quickly deleting a recently created snapshot

apiVersion: snapshot.storage.k8s.io/v1
kind: VolumeSnapshot
metadata:
  annotations: ...
  creationTimestamp: 2024-12-23T08:01:21Z
  deletionGracePeriodSeconds: 0
  deletionTimestamp: 2024-12-23T08:01:34Z
  finalizers:
    - snapshot.storage.kubernetes.io/volumesnapshot-bound-protection
  generation: 2
  labels: ...
spec:
  source:
    persistentVolumeClaimName: ...
  volumeSnapshotClassName: ocs-...
status:
  boundVolumeSnapshotContentName: snapcontent-...
  readyToUse: false

Anything else we need to know?:

Environment:

• Driver version:
• Kubernetes version (use kubectl version):
• OS (e.g. from /etc/os-release):
• Kernel (e.g. uname -a):
• Install tools:
• Others:

[xing-yang]

Can you show the details of the VolumeSnapshotContent in this case?

[akalenyu]

I actually had it backed up:

apiVersion: snapshot.storage.k8s.io/v1
  kind: VolumeSnapshotContent
  metadata:
    annotations:
      snapshot.storage.kubernetes.io/deletion-secret-name: ...
      snapshot.storage.kubernetes.io/deletion-secret-namespace: ...
      snapshot.storage.kubernetes.io/volumesnapshot-being-deleted: "yes"
    creationTimestamp: "2024-12-23T08:01:21Z"
    deletionGracePeriodSeconds: 0
    deletionTimestamp: "2024-12-23T08:01:34Z"
    finalizers:
    - snapshot.storage.kubernetes.io/volumesnapshotcontent-bound-protection
    generation: 2
    name: snapcontent-...
...
  spec:
    deletionPolicy: Delete
    driver: openshift-storage.rbd.csi.ceph.com
    source:
      volumeHandle: ...
    sourceVolumeMode: Block
    volumeSnapshotClassName: ocs-...
    volumeSnapshotRef:
      apiVersion: snapshot.storage.k8s.io/v1
      kind: VolumeSnapshot
      name: ...
      namespace: ...
...
  status:
    creationTime: 1734940895115234196
    readyToUse: true
    restoreSize: 32212254720
    snapshotHandle: ...

Actually, now that I think about it some more, it's possible #1259 fixes this even without waiting out the entire resync period
(Since some things like status changes don't result in a requeue)

[xing-yang]

Please provide logs if deletion does not happen.

[akalenyu]

Please provide logs if deletion does not happen.

I'll try to reproduce and increase verbosity. Which value should I set it to?

Actually, now that I think about it some more, it's possible #1259 fixes this even without waiting out the entire resync period
(Since some things like status changes don't result in a requeue)

Does this theory make sense?

[k8s-triage-robot]

The Kubernetes project currently lacks enough contributors to adequately respond to all issues.

This bot triages un-triaged issues according to the following rules:

• After 90d of inactivity, lifecycle/stale is applied
• After 30d of inactivity since lifecycle/stale was applied, lifecycle/rotten is applied
• After 30d of inactivity since lifecycle/rotten was applied, the issue is closed

You can:

• Mark this issue as fresh with /remove-lifecycle stale
• Close this issue with /close
• Offer to help out with Issue Triage

Please send feedback to sig-contributor-experience at kubernetes/community.

/lifecycle stale

[akalenyu]

/remove-lifecycle stale

[akalenyu]

Please provide logs if deletion does not happen.

I'll try to reproduce and increase verbosity. Which value should I set it to?

Actually, now that I think about it some more, it's possible #1259 fixes this even without waiting out the entire resync period
(Since some things like status changes don't result in a requeue)

Does this theory make sense?

@xing-yang I think the theory is correct, the PR should be ready for review

[ConnorJC3]

I've been helping try to chase down what I suspect is either this bug or some variation on it.

As I see it, there's 2 parts to this:

1) For some reason, the csi-snapshotter sidecar fails to perform an initial delete call

I think the flow looks something like this:

1. A create is ongoing - the content has the VolumeSnapshotBeingCreated annotation
2. The VolumeSnapshotContent is deleted - this delete is (correctly) rejected as the snapshot is still being created in shouldDelete
3. In a CreateSnapshot call the CSI Driver returns a ReadyToUse state of true
4. In checkandUpdateContentStatus, storeContentUpdate returns an error for whatever reason (most likely is that it happened to race with another update to the content)
5. checkandUpdateContentStatus returns !contentIsReady(contentObj) for requeue - because the content just became ready to use, this means it will return false in this case
6. The content is therefore not requeued
7. The update to the content's status is (correctly) discarded at the informer level as it is an update the sidecar itself initiated
8. We're now in a state where the snapshot needs to be deleted, but wasn't requeued

My theory is that changing the requeue return to something like !contentIsReady(contentObj) || contentObj.DeletionTimestamp != nil would solve this problem by requeuing snapshots that are marked for deletion and just finished creating. However, I'm still trying to get a working repro case and confirm my above theory of what's going on.

2) Resync is broken in the sidecar for VolumeSnapshotContent objects

Normally, even with the above bug from 1) - the VolumeSnapshotContent should get requeued after the resync period (default 15 minutes), and at that point the sidecar will realize it needs to make a DeleteSnapshot call. However, due to a mistake that was missed in #1009 no VolumeSnapshotContent resyncs ever occur.

I've submitted a PR (#1302) to fix this that uses a similar mechanism to how the external-attacher handles this case.