address review comments

Author: nearora-msft

pkg/azuredisk/azure_controller_common.go

@@ -20,6 +20,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"math"
 	"regexp"
 	"strings"
 	"sync"
@@ -196,13 +197,22 @@ func (c *controllerCommon) AttachDisk(ctx context.Context, diskName, diskURI str
 		time.Sleep(time.Duration(c.AttachDetachInitialDelayInMs) * time.Millisecond)
 	}
 
+	numDisksAllowed := math.MaxInt
 	_, instanceType, err := getNodeInfoFromLabels(ctx, string(nodeName), c.cloud.KubeClient)
 	if err != nil {
 		return -1, err
 	}
-	maxNumDisks := getMaxDataDiskCount(instanceType)
-	numDisksAttached := len(occupiedLuns)
-	numDisksAllowed := int(maxNumDisks) - numDisksAttached
+	if instanceType != "" {
+		maxNumDisks, instanceExists := getMaxDataDiskCount(instanceType)
+		if instanceExists {
+			attachedDisks, _, err := c.GetNodeDataDisks(ctx, nodeName, azcache.CacheReadTypeDefault)
+			if err != nil {
+				return -1, err
+			}
+			numDisksAttached := len(attachedDisks)
+			numDisksAllowed = int(maxNumDisks) - numDisksAttached
+		}
+	}
 
 	diskMap, err := c.cleanAttachDiskRequests(node)
 	if err != nil {
@@ -212,12 +222,13 @@ func (c *controllerCommon) AttachDisk(ctx context.Context, diskName, diskURI str
 	// Remove some disks from the batch if the number is more than the max number of disks allowed
 	removeDisks := len(diskMap) - numDisksAllowed
 	if removeDisks > 0 {
-		klog.V(2).Infof("azureDisk - too many disks to attach, remove %d disks from the request", removeDisks)
+		klog.V(2).Infof("too many disks to attach, remove %d disks from the request", removeDisks)
 		for diskURI, options := range diskMap {
 			if removeDisks == 0 {
 				break
 			}
 			if options != nil {
+				klog.V(2).Infof("remove disk(%s) from attach request", diskURI)
 				delete(diskMap, diskURI)
 			}
 			removeDisks--

pkg/azuredisk/nodeserver.go

@@ -412,7 +412,8 @@ func (d *Driver) NodeGetInfo(ctx context.Context, _ *csi.NodeGetInfoRequest) (*c
 		if instanceType == "" {
 			instanceType = instanceTypeFromLabels
 		}
-		maxDataDiskCount = getMaxDataDiskCount(instanceType) - d.ReservedDataDiskSlotNum
+		totalDiskDataCount, _ := getMaxDataDiskCount(instanceType)
+		maxDataDiskCount = totalDiskDataCount - d.ReservedDataDiskSlotNum
 	}
 
 	nodeID := d.NodeID
@@ -444,16 +445,16 @@ func (d *Driver) NodeGetInfo(ctx context.Context, _ *csi.NodeGetInfoRequest) (*c
 	}, nil
 }
 
-func getMaxDataDiskCount(instanceType string) int64 {
+func getMaxDataDiskCount(instanceType string) (int64, bool) {
 	vmsize := strings.ToUpper(instanceType)
 	maxDataDiskCount, exists := maxDataDiskCountMap[vmsize]
 	if exists {
 		klog.V(5).Infof("got a matching size in getMaxDataDiskCount, VM Size: %s, MaxDataDiskCount: %d", vmsize, maxDataDiskCount)
-		return maxDataDiskCount
+		return maxDataDiskCount, true
 	}
 
 	klog.V(5).Infof("not found a matching size in getMaxDataDiskCount, VM Size: %s, use default volume limit: %d", vmsize, defaultAzureVolumeLimit)
-	return defaultAzureVolumeLimit
+	return defaultAzureVolumeLimit, false
 }
 
 func (d *Driver) NodeGetVolumeStats(ctx context.Context, req *csi.NodeGetVolumeStatsRequest) (*csi.NodeGetVolumeStatsResponse, error) {

pkg/azuredisk/nodeserver_test.go

@@ -144,7 +144,7 @@ func TestGetMaxDataDiskCount(t *testing.T) {
 	}
 
 	for _, test := range tests {
-		result := getMaxDataDiskCount(test.instanceType)
+		result, _ := getMaxDataDiskCount(test.instanceType)
 		assert.Equal(t, test.expectResult, result)
 	}
 }
@@ -262,7 +262,8 @@ func TestNodeGetInfo(t *testing.T) {
 			},
 			validateFunc: func(t *testing.T, resp *csi.NodeGetInfoResponse) {
 				assert.Equal(t, testVMName, resp.NodeId)
-				assert.Equal(t, getMaxDataDiskCount(string(testVMSize)), resp.MaxVolumesPerNode)
+				maxDiskdataCount, _ := getMaxDataDiskCount(string(testVMSize))
+				assert.Equal(t, maxDiskdataCount, resp.MaxVolumesPerNode)
 				assert.Len(t, resp.AccessibleTopology.Segments, 2)
 			},
 		},