From a375b0aa3679d165d64129596e5e098cc12710d9 Mon Sep 17 00:00:00 2001 From: Baptiste Girard-Carrabin Date: Tue, 4 Mar 2025 15:29:42 +0100 Subject: [PATCH] Ensure CSINode belongs to current node on init When a CSI plugin attempts to register on a node, the node checks if the CSINode object exists. If it does but the ownerReference of the CSINode does not match the UID of the current node, possibly because the node object was recreated with the same name, we end up in a race condition where the CSINode object will be updated but subsequently deleted by the GC controller. In this situation, the CSINode object will be gone and won't be recreated unless the CSI plugin or the kubelet are restarted. This commit fixes this race by checking that the CSINode object belong to the current node during initialization. If it doesn't, it means that the CSINode object is left over from a previous node and it must be removed first. Once removed, registration can progress as usual. --- .../csi/nodeinfomanager/nodeinfomanager.go | 45 ++++++++++++++++--- 1 file changed, 38 insertions(+), 7 deletions(-) diff --git a/pkg/volume/csi/nodeinfomanager/nodeinfomanager.go b/pkg/volume/csi/nodeinfomanager/nodeinfomanager.go index 1bff173c5fe..e03af9d08e6 100644 --- a/pkg/volume/csi/nodeinfomanager/nodeinfomanager.go +++ b/pkg/volume/csi/nodeinfomanager/nodeinfomanager.go @@ -63,6 +63,7 @@ var ( // the Node and CSINode objects. type nodeInfoManager struct { nodeName types.NodeName + nodeID types.UID volumeHost volume.VolumeHost migratedPlugins map[string](func() bool) // lock protects changes to node. @@ -430,6 +431,12 @@ func (nim *nodeInfoManager) tryInitializeCSINodeWithAnnotation(csiKubeClient cli nim.lock.Lock() defer nim.lock.Unlock() + node, err := csiKubeClient.CoreV1().Nodes().Get(context.TODO(), string(nim.nodeName), metav1.GetOptions{}) + if err != nil { + return err + } + nim.nodeID = node.UID + nodeInfo, err := csiKubeClient.StorageV1().CSINodes().Get(context.TODO(), string(nim.nodeName), metav1.GetOptions{}) if nodeInfo == nil || errors.IsNotFound(err) { // CreateCSINode will set the annotation @@ -439,6 +446,16 @@ func (nim *nodeInfoManager) tryInitializeCSINodeWithAnnotation(csiKubeClient cli return err } + if !nim.nodeOwnsCSINode(nodeInfo) { + klog.V(2).Infof("existing CSINode %q is owned by different node, cleaning up...", nodeInfo.Name) + err = nim.DeleteCSINode() + if err != nil { + return fmt.Errorf("error deleting existing CSINode %q: %w", nodeInfo.Name, err) + } + // Returning now so that the next attempt can create a new CSINode object + return fmt.Errorf("CSINode %q was owned by different node, deleted it", nodeInfo.Name) + } + annotationModified := setMigrationAnnotation(nim.migratedPlugins, nodeInfo) if annotationModified { @@ -449,6 +466,15 @@ func (nim *nodeInfoManager) tryInitializeCSINodeWithAnnotation(csiKubeClient cli } +func (nim *nodeInfoManager) nodeOwnsCSINode(nodeInfo *storagev1.CSINode) bool { + for _, ownerRef := range nodeInfo.OwnerReferences { + if ownerRef.Kind == nodeKind.Kind && ownerRef.Name == string(nim.nodeName) && ownerRef.UID == nim.nodeID { + return true + } + } + return false +} + func (nim *nodeInfoManager) CreateCSINode() (*storagev1.CSINode, error) { csiKubeClient := nim.volumeHost.GetKubeClient() @@ -456,11 +482,6 @@ func (nim *nodeInfoManager) CreateCSINode() (*storagev1.CSINode, error) { return nil, fmt.Errorf("error getting CSI client") } - node, err := csiKubeClient.CoreV1().Nodes().Get(context.TODO(), string(nim.nodeName), metav1.GetOptions{}) - if err != nil { - return nil, err - } - nodeInfo := &storagev1.CSINode{ ObjectMeta: metav1.ObjectMeta{ Name: string(nim.nodeName), @@ -468,8 +489,8 @@ func (nim *nodeInfoManager) CreateCSINode() (*storagev1.CSINode, error) { { APIVersion: nodeKind.Version, Kind: nodeKind.Kind, - Name: node.Name, - UID: node.UID, + Name: string(nim.nodeName), + UID: nim.nodeID, }, }, }, @@ -483,6 +504,16 @@ func (nim *nodeInfoManager) CreateCSINode() (*storagev1.CSINode, error) { return csiKubeClient.StorageV1().CSINodes().Create(context.TODO(), nodeInfo, metav1.CreateOptions{}) } +func (nim *nodeInfoManager) DeleteCSINode() error { + + csiKubeClient := nim.volumeHost.GetKubeClient() + if csiKubeClient == nil { + return fmt.Errorf("error getting CSI client") + } + + return csiKubeClient.StorageV1().CSINodes().Delete(context.TODO(), string(nim.nodeName), metav1.DeleteOptions{}) +} + func setMigrationAnnotation(migratedPlugins map[string](func() bool), nodeInfo *storagev1.CSINode) (modified bool) { if migratedPlugins == nil { return false