Problems dealing with snapshot create requests timing out

[ShyamsundarR]

When a CSI plugin is passed a CreateSnapshot request and the caller (snapshotter sidecar) times out, the snapshotter sidecar marks this as an error and does not retry the snapshot. Further, as the call only timed out and did not fail, the storage provider may have actually created the said snapshot (although delayed).

When such an snapshot is deleted, there are no requests to the CSI plugin to delete the same, which cannot be issued by the sidecar as it does not have the SnapID.

The end result of this is that the snapshot is leaked on the storage provider.

The question/issue hence is as follows,

Should the snapshot be retried on timeouts from the CreateSnapshot call?

Based on the ready_to_use parameter in the CSI spec [1] and possibilities of application freeze as the snapshot is taken, I would assume this operation cannot be done indefinitely. But, also as per the spec timeout errors, the behavior should be a retry, as implemented for volume create and delete operations in the provisioner sidecar [2].

So to fix the potential snapshot leak by the storage provider, should the snapshotter sidecar retry till it gets an error from the plugin or a success with a SnapID, but mark the snapshot as bad/unusable as it was not completed in time (to honor the application freeze times and such)?

[1] CSI spec ready_to_use section: https://github.com/container-storage-interface/spec/blob/master/spec.md#the-ready_to_use-parameter

[2] timeout handling in provisioner sidecar: https://github.com/kubernetes-csi/external-provisioner#csi-error-and-timeout-handling

[ShyamsundarR]

The issue was faced when developing and testing the Ceph CSI implementation (issue details are captured here: ceph/ceph-csi#446 ).

snapshotter sidecar code reading also suggests that CreateSnapshot is a one time call, and the error returned is stashed in the snapshot object and hence the create never called again.

During the testing, introducing artificial delays was also performed to understand the behavior. If a reproducer is required please do let us know.

[Madhu-1]

cc @xing-yang

[msau42]

cc @jingxu97

[jingxu97]

@ShyamsundarR thank you very much for bringing this issue up. The following is the behavior I copied from create volume.

ControllerCreateVolume: The call might have timed out just before the driver provisioned a volume and was sending a response. From that reason, timeouts from ControllerCreateVolume is considered as "volume may be provisioned" or "volume is being provisioned in the background." The external-provisioner will retry calling ControllerCreateVolume after exponential backoff until it gets either successful response or final (non-timeout) error that the volume cannot be created.

As you mentioned, the reason CreateSnapshot does not retry is to avoid constantly trying to create new snapshot if the previous operations failed. In case of timeouts, as you pointed out, there is a potential issue that snapshot is being created. But the assumption here is that CreateSnapshot returns once snapshot is cut which should be very quick. The current timeout is set to 1 second and we assume that cutting the snapshot should not take this long, so that timeout means that an error occurred during snapshotting and there should no snapshots available. But if you have seen this does not cover some cases, we might need to change this behavior which probably adds quite some complexity.

For plugins that supports snapshot post processing such as uploading, CreateSnapshot SHOULD return 0 OK and ready_to_use SHOULD be set to false after the snapshot is cut but still being processed.

[msau42]

Problem is client side (sidecar) times out but server side (csi driver) operation continues. When the server side operation finally returns, the client side doesn't get the response because it gave up on the rpc.

Can we do something like repeatedly call DeleteSnapshot if CreateSnapshot failed until DeleteSnapshot returns success? Unsure how to handle snapshot handle that is returned by the driver

[xing-yang]

We can't call DeleteSnapshot before CreateSnapshot returns success because DeleteSnapshot needs the snapshot handle. If CreateSnapshot fails or times out, we won't get the handle to delete it.

[ShyamsundarR]

The problem is as @msau42 and @xing-yang put it, the client times out, and also the client does not have the Snapshot ID to call DeleteSnapshot

@jingxu97 Current timeouts for CreateSnapshot seem to be set to 60 seconds, and not 1 second. Considering timeouts as errors does not help, or expecting snapshots to always complete successfully in certain time may not be true for all systems all the time.

(following is a very theoretical approach for the solution, I have not looked at the feasibility)
I would suggest marking the VolumeSnapshotContent object as bad/error on a timeout, but retry the CreateSnapshot (like CreateVolume) till an error or the snapshot ID is returned by the plugin, and then garbage collect the same based on the stored state in the VolumeSnapshotContent object. This should satisfy that the snapshot was taken in the required freeze duration and was successful, or otherwise it was not and the content object is marked stale/bad but ensure that the storage backend snapshot will be garbage collected.

[msau42]

cc @jsafrane for any ideas

[jsafrane]

IMO, proper solution without changing of CSI spec is the same as in volume provisioning. The snapshotter should retry until it gets a final response and decide what to do - either the snapshot is too old or user deleted VolumeSnapshot objects in the meantime and delete it or create VolumeSnapshotContent.

As you can immediately spot, if the snapshotter is restarted while the snapshot is being cut and after user deleted related VolumeSnapshot object, newly started snapshotter does not know that it should resume creating the snapshot. Volume provisioning has the same issue. We hope that volume taints could help here and we could create empty PV / VolumeSnapshotContent before knowing the real volume / snapshot ID as memento that there is some operation in progress on the storage backend.

[xing-yang]

@ggriffiths will be helping out with this bug fix. Thanks.

[ggriffiths]

/assign ggriffiths

[fejta-bot]

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or fejta.
/lifecycle stale