Implement Snapshot support

[leakingtapan]

Driver implementation should support following operations:

• CreateSnapshot
• DeleteSnapshot
• ListSnapshots Implement ListSnapshot #233
• CreateVolume from snapshot

References:

• https://kubernetes.io/docs/concepts/storage/volume-snapshots/
• https://kubernetes.io/docs/concepts/storage/volume-snapshot-classes/
• https://kubernetes-csi.github.io/docs/Implement-Snapshot-Feature.html

[bertinatto]

CC @tsmetana

[bertinatto]

/assign @tsmetana

[k8s-ci-robot]

@bertinatto: GitHub didn't allow me to assign the following users: tsmetana.

Note that only kubernetes-sigs members and repo collaborators can be assigned.
For more information please see the contributor guide

In response to this:

/assign @tsmetana

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[leakingtapan]

Punt the feature to beta release

[leakingtapan]

/kind feature

[tsmetana]

First version (does not implement snapshot listing yet): https://github.com/tsmetana/aws-ebs-csi-driver/tree/snapshots-updated

It's based on PR#122 (CSI-1.0) so I will wait with my PR.

[bertinatto]

@tsmetana: we just merged my PR into next branch. If you want to proceed with you PR you can point it against next as well.

[tsmetana]

Thanks. Created the promised PR.

[leakingtapan]

/assign @dkoshkin
Dimitri will help debugging issues related to snapshot

[tsmetana]

OK. I'm not sure which is the right place to discuss this... What is happening with the snapshots: in the waitForSnapshotCreate there is an truncated exponential backoff with a condition function that basically polls for the snapshot unitl it's "completed" or an error occurs. The polling loop uses ec2.DescribeSnapshotsWithContext(ctx, ...). After few iterations this call fails with GRPC error ("DeadlineExceeded" iirc).

Looks like the issue is that the CreateSnapshot CSI call should be blocking (by specification) but the GRPC connection times out prematurely -- it's unknown how long time a snapshot creation might take. The controllers (external snapshotter in our case) should be able to handle that -- for example external attacher distinguishes the fatal and transient errors like this: https://github.com/kubernetes-csi/external-attacher/blob/master/pkg/connection/connection.go#L256

I guess similar mechanism needs to be implemented also in the external snapshotter, otherwise this patch will not work...

Edit: In case of AWS it might be possible to avoid the error by simply not waiting for the snapshot to complete and return immediately with a snapshot in not ReadyToUse state. The snapshotter should then re-try until the snapshot is ready.

[tsmetana]

The issues I mentioned in the previous comment were worked around in the patch that's been recently merged.

[leakingtapan]

@tsmetana your snapshot change is merged into master branch

just to confirm that is ListSnapshot not used by k8s yet, so it is not implemented?

[tsmetana]

No, ListSnapshot is not implemented yet. I'll add it to my TODO.

[leakingtapan]

@tsmetana I created a follow up issue for the left task: #233

I am going to close this one since the major tasks are done.

/close

[k8s-ci-robot]

@leakingtapan: Closing this issue.

In response to this:

@tsmetana I created a follow up issue for the left task: #233

I am going to close this one since the major tasks are done.

/close

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.