volume leak during deployment rollout

[pohly]

The controller is designed such that it collects information about volumes from nodes as the nodes register themselves. This implies that the controller cannot know about existing volumes for nodes that haven't registered (yet).

This leads to the following problem:

• DeleteVolume is called for an existing volume that the controller doesn't know about at the moment.
• The controller cannot distinguish between "volume already deleted" (idempotency!) and "need to wait for some node with that volume".
• It assumes that the volume is gone and returns success without doing anything, after a misleading log message about "Volume pvc-bd-adc62b1395a868c243a74ee138e313a19c72211c5fbc0d5f2706e486 not created by this controller".
• external-provisioner removes PV

=> volume leak

This problem was triggered by the new version skew tests which restart the driver while volumes exist, then does some operations (including removal) with them right after the driver deployment comes up again.

[pohly]

The whole registration process is also problematic because of #729. It may also be a scalability problem (single instance of the controller), although we don't have any evidence of that because we haven't done scale testing.

I'm currently leaning towards solving this problem by deploying the external-provisioner alongside each node driver instance and removing the central controller entirely. This was originally proposed in kubernetes-csi/external-provisioner#367 but wasn't finished.

What was missing in that PR was support for immediate binding. With immediate binding, all external-provisioner instances need to collaboratively figure out who's the one who should create the volume. I was proposing leadership election for that (kubernetes-csi/external-provisioner#367 (comment)) but that'll need further thought.

[avalluri]

I think the other alternate/quick solution is to persist the controller state(known volume details) in a config map.

[pohly]

I think the other alternate/quick solution is to persist the controller state(known volume details) in a config map.

The controller currently doesn't have access to a config map. Introducing that would introduce a dependency on Kubernetes into the CO-agnostic part of PMEM-CSI.

Even with a config map, getting this right in all cases will be tricky. Consider the naive approach:

• controller gets CreateVolume request
• sends CreateVolume to node
• records new volume in config map
• returns information to external-provisioner

If the controller dies at any point during this flow, the volume may leak. A lot of effort went into external-provisioner to prevent such leaks; we would have to duplicate all of that.

[pohly]

This is a valid alternative solution, but I wouldn't call it quick.

[pohly]

The upstream work on enabling de-centralized deployment of external-provisioner is tracked in kubernetes-csi/external-provisioner#487

[pohly]

If the controller dies at any point during this flow, the volume may leak. A lot of effort went into external-provisioner to prevent such leaks; we would have to duplicate all of that.

Not only when it dies - error handling also currently isn't sufficient to prevent volume leaks, see issue #823