Implement wait for volume create/delete

[leakingtapan]

Similar to #38

[bertinatto]

I'm afraid that in large clusters this will increase the API usage considerably, possibly getting the rate-limited.

Kubernetes waits on volume creation only when it's using encryption keys [1]. Apparently it only waits on tag creation, so this ends up as a side effect. As for deletion, it doesn't seem to wait at all.

[1] https://github.com/kubernetes/kubernetes/blob/95c99eb052baba498dfb366bb1e43d2af6c51270/pkg/cloudprovider/providers/aws/aws.go#L2228

[leakingtapan]

In this case, we could follow in-tree implementation then. And implement wait for creation only for encrypted volume #16. And I would like to punt this to 0.3-beta milestone and we will decide later if this is needed. How do you think?

Apparently it only waits on tag creation, so this ends up as a side effect.

Could you elaborate on this?

[bertinatto]

Could you elaborate on this?

The in-tree implementation works like this:

• K8s sends the CreateVolume request
• K8s sends another request to create some tags on that volume
  • If this request fails, k8s tries again until a request succeeds or the timeout limit is reached
  • In other words, k8s doesn't wait for the volume to be available, but I believe that until the volume is not available, the tag creation won't succeed.
• If there are KMS keys, k8s watches the volume until it becomes available.

The CSI driver works a bit better: it creates the volume and sets its tags in a single request.

Waiting for the volume to be available would mean performing more requests to AWS, increasing the chances of exceeding the quota of requests and getting rate-limited. This is something I was trying to avoid, as this seems to be a recurrent problem.

Ideally this would be taken care on AWS side, i.g., CreateVolume would block until the volume is ready or we would get an event (without the need to perform more requests). If I remember correctly from our initial meetings, you guys were considering a solution for this problem. Do you know if there's any progress on this front? Or do you think we go ahead with a client-side workaround?

[leakingtapan]

I researched the CreateVolume issue a bit. It is caused by the nature that the API is an async API so that any calls to it is executed under the hood of EBS as an asynchronous workflow. Given that, if an invalid KMS key is passed in CreateVolume the validation error is not known (at least now) until the async workflow is executed. I have contacted EBS service team for their plan on solving this.

So making CreateVolume blocking will not be an option. As of now, we could only keep the client side work around. I will keep an eye on this for any updates from EBS.

[bertinatto]

/assign

[bertinatto]

K8s sends another request to create some tags on that volume

The behaviour has changed since kubernetes/kubernetes#67812 (merged about a month ago).

Edit:

With the change above, the in-tree implementation will try to attach the volume while it's state is "creating". Initially it'll fail, but after a few tries it'll succeed. In other words, the in-tree implementation matches the driver now:

  Warning  FailedAttachVolume      15s (x4 over 19s)  attachdetach-controller               AttachVolume.Attach failed for volume "pvc-320dced4-eefa-11e8-a19d-0e7ca462e948" : "Error attaching EBS volume \"vol-01cce92492ab6d0b0\"" to instance "i-0fcdf055a3d119b1b" since volume is in "creating" state
  Normal   SuccessfulAttachVolume  7s                 attachdetach-controller               AttachVolume.Attach succeeded for volume "pvc-320dced4-eefa-11e8-a19d-0e7ca462e948"

[bertinatto]

@leakingtapan, I have been thinking about this and I'd like to propose this:

Wait for the right volume state in CreateDisk, but poll the AWS API every 5 seconds during 1 minute. Assuming the volume takes in average 4 seconds to be available, this should result in only 1 extra request in most cases. Thoughts?

[leakingtapan]

Wait for the right volume state in CreateDisk, but poll the AWS API every 5 seconds during 1 minute

To clarify, we are proposing this for only CreateDisk? And this applies to both encrypted disk and regular disk, right?

[bertinatto]

That's correct.

I have a PoC working, but I might have to play with the interval times, as the context coming from k8s has a timeout deadline that occasionally is run over.

[bertinatto]

As for the delete operation, I propose to simply call DeleteVolume without waiting for the state.

This saves one or more requests, plus we'll get an error if something is wrong.