From 85f94965367420808b6a57bc9e8d9b5adbe1426e Mon Sep 17 00:00:00 2001
From: andyzhangx <xiazhang@microsoft.com>
Date: Wed, 8 Jan 2025 13:59:12 +0000
Subject: [PATCH] fix: wrong node matching when detaching dangling node

---
 pkg/azuredisk/azure_controller_common.go |  2 +-
 pkg/azuredisk/azuredisk.go               | 35 ++++++++++++++++
 pkg/azuredisk/azuredisk_test.go          | 52 ++++++++++++++++++++++++
 3 files changed, 88 insertions(+), 1 deletion(-)

diff --git a/pkg/azuredisk/azure_controller_common.go b/pkg/azuredisk/azure_controller_common.go
index 26fd6f0d5f..e2240ea832 100644
--- a/pkg/azuredisk/azure_controller_common.go
+++ b/pkg/azuredisk/azure_controller_common.go
@@ -132,7 +132,7 @@ func (c *controllerCommon) AttachDisk(ctx context.Context, diskName, diskURI str
 			if err != nil {
 				return -1, err
 			}
-			if strings.EqualFold(string(nodeName), string(attachedNode)) {
+			if isSameNode(string(nodeName), string(attachedNode)) {
 				klog.Warningf("volume %s is actually attached to current node %s, invalidate vm cache and return error", diskURI, nodeName)
 				// update VM(invalidate vm cache)
 				if errUpdate := c.UpdateVM(ctx, nodeName); errUpdate != nil {
diff --git a/pkg/azuredisk/azuredisk.go b/pkg/azuredisk/azuredisk.go
index 5cf65726d7..0fb06ddaf4 100644
--- a/pkg/azuredisk/azuredisk.go
+++ b/pkg/azuredisk/azuredisk.go
@@ -786,3 +786,38 @@ func checkAllocatable(ctx context.Context, clientset kubernetes.Interface, nodeN
 
 	return fmt.Errorf("isAllocatableSet: driver not found on node %s", nodeName)
 }
+
+// isSameNode checks whether node1 and node2 are the same node
+func isSameNode(node1, node2 string) bool {
+	if strings.EqualFold(node1, node2) {
+		return true
+	}
+	// check whether node1 and node2 are VMSS instance name
+	if len(node1) < 6 || len(node2) < 6 {
+		return false
+	}
+
+	// machineName is composed of computerNamePrefix and 36-based instanceID.
+	// And instanceID part if in fixed length of 6 characters.
+	// Refer https://msftstack.wordpress.com/2017/05/10/figuring-out-azure-vm-scale-set-machine-names/.
+	if strings.EqualFold(node1[:len(node1)-6], node2[:len(node2)-6]) {
+		// check whether node1 is 36-based instanceID and node2 is 10-based instanceID
+		id1, err := strconv.ParseUint(node1[len(node1)-6:], 36, 64)
+		if err == nil {
+			id2, err := strconv.ParseUint(node2[len(node2)-6:], 10, 64)
+			if err == nil && id1 == id2 {
+				return true
+			}
+		}
+
+		// check whether node1 is 10-based instanceID and node2 is 36-based instanceID
+		id1, err = strconv.ParseUint(node1[len(node1)-6:], 10, 64)
+		if err == nil {
+			id2, err := strconv.ParseUint(node2[len(node2)-6:], 36, 64)
+			if err == nil && id1 == id2 {
+				return true
+			}
+		}
+	}
+	return false
+}
diff --git a/pkg/azuredisk/azuredisk_test.go b/pkg/azuredisk/azuredisk_test.go
index c1ba86e950..fc12839e19 100644
--- a/pkg/azuredisk/azuredisk_test.go
+++ b/pkg/azuredisk/azuredisk_test.go
@@ -589,3 +589,55 @@ func TestGetUsedLunsFromNode(t *testing.T) {
 		}
 	}
 }
+
+func TestIsSameNode(t *testing.T) {
+	tests := []struct {
+		name          string
+		nodeName      string
+		nodeName2     string
+		expectedValue bool
+	}{
+		{
+			name:          "same node",
+			nodeName:      "test-node",
+			nodeName2:     "test-node",
+			expectedValue: true,
+		},
+		{
+			name:          "different node with shorter name",
+			nodeName:      "node1",
+			nodeName2:     "node2",
+			expectedValue: false,
+		},
+		{
+			name:          "different node",
+			nodeName:      "test-node",
+			nodeName2:     "test-node2",
+			expectedValue: false,
+		},
+		{
+			name:          "empty node",
+			nodeName:      "",
+			nodeName2:     "test-node",
+			expectedValue: false,
+		},
+		{
+			name:          "node1 is 10 based instance name, node2 is 36 based instance name",
+			nodeName:      "aks-agentpool-20657377-vmss000012",
+			nodeName2:     "aks-agentpool-20657377-vmss00000c",
+			expectedValue: true,
+		},
+		{
+			name:          "node1 is 36 based instance name, node2 is 10 based instance name",
+			nodeName:      "aks-agentpool-20657377-vmss00000c",
+			nodeName2:     "aks-agentpool-20657377-vmss000012",
+			expectedValue: true,
+		},
+	}
+	for _, test := range tests {
+		result := isSameNode(test.nodeName, test.nodeName2)
+		if result != test.expectedValue {
+			t.Errorf("test(%s): result(%v) != expected result(%v)", test.name, result, test.expectedValue)
+		}
+	}
+}