Detach failure if node is removed from cluster

[mape90]

Problem

If node with volumes managed by CSI plugin is removed from Kubernetes cluster before volumes are detached then detaching will newer succeed.

function csiDetach (in pkg/controller/csi_handler.go +296 (master)) is failing always because it tries to get nodeID from kubernetes api and as node does not anymore exists it fails.

Environment

This behavior was produced in Openstack with Cinder-CSI plugin during scale down of worker nodes. As there Openstack cloud provider deleted the node from Kubernetes cluster when it found out that the virtual machine did not existed anymore.

Expected Behavior

Volume detach should not automatically fail if node where it was attached isn't anymore part of the kubernetes cluster. It should still try to detach the volume. The nodeID could be passed as empty string and then the actual detaching implementation could then decide if it possible to detach the volume or if it might already be detached.

In case of Openstack volumes are automatically detached when virtual machine is deleted.

[jsafrane]

Thanks for the report. It's hard to judge what's right in this case.

The nodeID could be passed as empty string

That's very implementation dependent. Different drivers will do different things, usage of empty string is not described in CSI spec. It SHALL be "ID returned by NodeGetInfo", but Kubernetes does not have this information, because the node was deleted.

One thing I could perhaps do here is to store the node ID in VolumeAttachment annotation.
@saad-ali @davidz627, any opinions?

[mape90]

Thanks for quick response. Yes it is best not to alter the spec, so yes that my proposal is not possible. I trust that you find some elegant way to solve this.

[davidz627]

@jsafrane I like the idea of storing the node ID in the VolumeAttachment, that information is very "tied" to the attach operation and so it seems like a natural place to put it. Is is possible to even create a field in the object?

[jsafrane]

Yesterday our Kubernetes CSI workgroup decided that we'll put Node ID into VolumeAttachment annotation during attach / ControllerPublish. Downside of this approach is that it's not updated when a driver gets updated and starts reporting different NodeID than before. The driver must accept old Node ID in this case.

/assing
(but patches are welcome)

[jsafrane]

/assign

[jsafrane]

@davidz627, sorry, I did not see your comment (github outage?)

I could add a field for it, still I have bad feeling about it. If a driver is updated while a volume is attached, we should use the new NodeID. Using an old Node ID from VolumeAttachment annotation or field feels like workaround.

[davidz627]

@davidz627, sorry, I did not see your comment (github outage?)

I could add a field for it, still I have bad feeling about it. If a driver is updated while a volume is attached, we should use the new NodeID. Using an old Node ID from VolumeAttachment annotation or field feels like workaround.

I see now from your PR that this is a backup only when the Node is gone. I did not realize that before, annotation seems like the right way here.