connectionTimeout is actually operationTimeout

[cduchesne]

I noticed that when I use a short connection timeout, for example 5 seconds, and then throttle between my csi plugin (csi-scaleio) and my storage api server, any operation that takes more than the timeout fails. This is a big problem because the external-provisioner has only connectionTimeout to complete any request. If a volume creation event took longer than 5 seconds, which I tested via throttling, a new request will kick off to the csi plugin, resulting in duplicate volumes with different names being created. This is because the first command was still sent to the csi plugin which it successfully connected to even though the operation is dropped from the provisioner side.

I find that context.WithTimeout is the culprit here as it is used frequently for every command that is sent to the grpc server. This results in the grpc request being severed once connectionTimeout is reached. This would possibly be fine if all operations were requested in an idempotent manner.

My suggestion is to change connectionTimeout to only occur at startup or whenever connection to the csi plugin is first established, and to maybe introduce a new option, operationTimeout.

There are some fixes required to make sure the same volume is requested each time to make the volume creation idempotent. Every time Provision (in pkg/controller) is called, a new random share name is generated, hence if the Provision command is called multiple times, it will result in duplicate volumes with different names. Only 1 volume will ever be tracked by Kubernetes and the others will be orphaned.

[saad-ali]

CC @jsafrane @childsb @sbezverk @lpabon @chakri-nelluri

Every time Provision (in pkg/controller) is called, a new random share name is generated, hence if the Provision command is called multiple times, it will result in duplicate volumes with different names. Only 1 volume will ever be tracked by Kubernetes and the others will be orphaned.

We should consider generating the volume name deterministically based on PVC namespace+name?

kubernetes/community#1790 is proposing this for the existing RBD Dynamic Provisioner.

My suggestion is to change connectionTimeout to only occur at startup or whenever connection to the csi plugin is first established, and to maybe introduce a new option, operationTimeout.

@cduchesne could you clarify what you mean by this?

[cduchesne]

@saad-ali - i mentioned this if you plan to use the variable name / configuration option connectionTimeout. It is being used as an operationTimeout and not connectionTimeout, so connectionTimeout should be used sparingly unless it can better be utilized with the actual connection to grpc endpoint and not the complete operation time.

[saad-ali]

Thanks! It looks like the timeout is being used for the create operation and it is too short (5 seconds). external-attacher has two sets of timeouts, one for short operations (1 second) and one for long running operations (1 minute), we should do something similar for external-provisioner.

[cduchesne]

Yes I used 5 seconds to validate the problem - I think default is 10s, which is definitely too short for some operations. Yes 2 separate timeouts would be better.

[cduchesne]

The bigger problem is the abandoned request without ability to re-use same volume name. I like the deterministic naming idea.

[chakri-nelluri]

@saad-ali Actually, isn't it a bug in the controller if it generates a new UUID/PV name for the same PVC every time it is called?

+1 for deterministic volume name.

[saad-ali]

@chakri-nelluri you mean bug in the Kubernetes controller? The k8s controller doesn't control the volume name, the external-provisioner does.

[saad-ali]

So, to summarize we need the following fixes for external-provisioner:

1. Fix timeouts: long timeout and short timeout.
2. Make names idempotent by generating deterministically.
3. external-provisioner (as well as other side car containers) should not give up after timeout, it should retry with exponential backoff as long as desired state indicates action is required (issue should never time out from connecting. #64).

[saad-ali]

Serguei has agreed to work on this!