Detach failure if node is removed from cluster about external-attacher HOT 7 CLOSED

@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.

from external-attacher.

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?

from external-attacher.

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.

from external-attacher.

@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?

from external-attacher.

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)

from external-attacher.

/assign

from external-attacher.

@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.

from external-attacher.