KEP-6011: CSI ControllerGetNodeInfo - #6016
Conversation
…ller-side node info retrieval This KEP proposes two new CSI RPCs to enable node topology and capacity information retrieval from the controller side, eliminating the need for cloud API credentials on worker nodes: - NodeGetID: Returns only the node identifier without cloud API access - ControllerGetNodeInfo: Fetches topology and capacity from controller side Benefits: - Security: Node components no longer require cloud API credentials - Scalability: Controller can aggregate and cache API calls for large clusters - Accuracy: Controller-side VolumeAttachment info enables accurate non-CSI attachment detection Depends on CSI spec PR kubernetes#603: container-storage-interface/spec#603
Signed-off-by: Eddie Torres <torredil@amazon.com>
k8s-ci-robot
added
kind/kep
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: huww98 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
k8s-ci-robot
added
size/XL
|
|
||
| #### Race Condition Mitigation | ||
|
|
||
| A race exists between `ControllerGetNodeInfo` and concurrent attach/detach: if an attach completes between listing `VolumeAttachment` objects and the cloud API query, the newly attached volume appears in cloud results but not in `published_volume_ids`, causing the SP to misclassify it as non-CSI. |
There was a problem hiding this comment.
I'm considering an alternative approach. How about let SP return a list of published_volume_ids, and let CO (external-attacher) to calculate the final volume count available to CSI.
type nodeInfoProcessor struct {
pendingNodes sync.Map // nodeName -> Set[string]
}
func (p *nodeInfoProcessor) processNode(nodeName string) {
csiPublished := listVolumeAttachments(nodeName)
p.pendingNodes.Store(nodeName, csiPublished)
defer p.pendingNodes.Delete(nodeName)
info := ControllerGetNodeInfo(nodeID)
nonCsi := info.publishedVolumeIDs.Difference(csiPublished)
info.maxVolumesPerNode -= len(nonCsi)
updateCSINode(csiNode, info)
}
func (h *csiHandler) syncAttach(va) {
h.nodeInfoProcessor.recordPublish(va)
}We mitigate the race condition by recording all processed volumeIDs during the ControllerGetNodeInfo call, and also consider them as CSI-managed. We don't need to pause the attach this way.
There was a problem hiding this comment.
Yeah, I like this a lot better. Classification is naturally the CO's job since it has the VA context and having the SP just report what the cloud says is attached is cleaner.
There was a problem hiding this comment.
Updated the KEP and CSI spec PR
Add explanation for why NodeGetID is a separate RPC (protocol handshake, no safe empty values in NodeGetInfo, field ownership). Add three new alternatives documenting design decisions from SIG Storage discussion: - Alternative 5: Fold NodeGetID into NodeGetInfo (rejected) - Alternative 6: Reactive-Only Discovery (rejected as standalone) - Alternative 7: Static Node Context Object (rejected)
9a0911f to
a935a3c
Compare
Add a new "Hardcoded Instance-Type Tables" alternative and extend the node-label-patching alternative with GCP (gce-pd-node-labeler) and Alibaba Cloud (csi-metadata-labeler) as additional examples, showing three providers independently reinvented the controller-side label-patching sidecar. Note that Alibaba queries cloud APIs at runtime rather than hardcoding tables, demonstrating the controller-side-RPC approach is already viable in practice. Renumber subsequent alternatives and update TOC accordingly.
Replace the separate NodeGetID RPC with a controller_get_node_info request flag on the existing NodeGetInfo. This minimizes adoption cost for the 100+ existing CSI drivers (a capability plus one optional field rather than a new RPC), avoids two node-side RPCs with overlapping purpose, and keeps shared node-produced fields (node_id) on a single message. Rework the alternatives accordingly: the separate NodeGetID RPC becomes a rejected alternative, and a new alternative documents why node and controller values are not combined (no CO-agnostic merge rule; correct combination belongs in the SP). Unify the feature gate to CSIControllerGetNodeInfo across kubelet and external-attacher.
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: huww98 The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Updated according to CSI community meeting discussion. added |
3 participants
Uh oh!
There was an error while loading. Please reload this page.