Idempotency: repeated Node Unpublish operations cause failure about csi-driver-host-path HOT 6 CLOSED

We discussed this today in the Kubernetes-CSI standup and everyone agreed that a) this should be fixed in the hostpath driver and b) that having these idempotency tests in csi-test will be very useful.

from csi-driver-host-path.

what is necessary to fix this? just limiting the concurrency of operations in the driver?

from csi-driver-host-path.

xref: #72, this sounds like a superset of that

from csi-driver-host-path.

I created #139 to tolerate repeated NodeUnpublish requests.
My local testing shows that this allows hostpath driver to pass the added repeated csi-sanity testing currently coming in kubernetes-csi/csi-test#229, so right now there is no need to add mutex locking to satisfy those repeated tests, as I can't see failures.

from csi-driver-host-path.

so right now there is no need to add mutex locking to satisfy those repeated tests, as I can't see failures.

This is a dangerous argument: that you haven't been able to trigger a problem caused by missing locking does not mean that the problem doesn't exist or that we don't need to do anything about it.

In practice the problem should be exceedingly rare. It only happens when two gRPC calls reach the driver at the same time and get processed in parallel. I think the idempotency tests don't cover that, they just issue the same call multiple times sequentially.

So while we don't need mutex locking to make the NodeUnpublishVolume idempotent, we still need it to make the code more robust.

from csi-driver-host-path.

This is a dangerous argument: that you haven't been able to trigger a problem caused by missing locking does not mean that the problem doesn't exist or that we don't need to do anything about it.

I agree, I did not mean we don't need to deal with locking at all, just that currently to-be-added test cases do not expose problem that we can see fixed by added locking.
We should add locking, but the progress will be more assuring if we can have test case(s) improved by locking.

from csi-driver-host-path.