internal/driver: Fix off-by-one error with max volume attachments

When calculating the maximum number of allowed volume attachments, the
code was previously taking the ideal maximum number of volumes that
could be attached to a node. The way the attachment was calculated, it
treated instance disks the same as volumes, which is not correct.

This commit fixes what is effectively an off-by-one error, by
subtracting the number of instance disks from the theoretical maximum
number of block devices that can be attached to the instance.

In other words, controller and node servers will now report the number
of block storage volumes that can be attached, not just block devices.

Fixes #182

internal/driver/controllerserver.go

@@ -307,41 +307,35 @@ func (linodeCS *LinodeControllerServer) ControllerPublishVolume(ctx context.Cont
 // Linode with the given ID.
 //
 // Whether or not another volume can be attached is based on how many instance
-// disks and block storage volumes are currently attached to the Linode
-// instance.
+// disks and block storage volumes are currently attached to the instance.
 func (s *LinodeControllerServer) canAttach(ctx context.Context, instance *linodego.Instance) (canAttach bool, err error) {
-	// Total number of attached drives.
-	var attached int
-
-	disks, err := s.CloudProvider.ListInstanceDisks(ctx, instance.ID, nil)
-	if err != nil {
-		return false, fmt.Errorf("list instance disks: %w", err)
-	}
-	attached += len(disks)
-
+	limit := s.maxVolumeAttachments(instance)
 	volumes, err := s.CloudProvider.ListInstanceVolumes(ctx, instance.ID, nil)
 	if err != nil {
 		return false, fmt.Errorf("list instance volumes: %w", err)
 	}
-	attached += len(volumes)
 
-	limit := s.maxVolumeAttachments(instance)
-	return attached < limit, nil
+	return len(volumes) < limit, nil
 }
 
 // maxVolumeAttachments returns the maximum number of volumes that can be
-// attached to a single Linode instance. If instance == nil, or instance.Specs
-// == nil, [maxPersistentAttachments] will be returned.
-func (s *LinodeControllerServer) maxVolumeAttachments(instance *linodego.Instance) int {
+// attached to a single Linode instance, minus any currently-attached instance
+// disks.
+func (s *LinodeControllerServer) maxVolumeAttachments(ctx context.Context, instance *linodego.Instance) (int, error) {
 	if instance == nil || instance.Specs == nil {
-		return maxPersistentAttachments
+		return 0, errors.New("nil instance")
+	}
+
+	disks, err := s.CloudProvider.ListInstanceDisks(ctx, instance.ID, nil)
+	if err != nil {
+		return 0, fmt.Errorf("list instance disks: %w", err)
 	}
 
 	// The reported amount of memory for an instance is in MB.
 	// Convert it to bytes.
 	memBytes := uint(instance.Specs.Memory) << 20
 
-	return maxVolumeAttachments(memBytes)
+	return maxVolumeAttachments(memBytes) - len(disks), nil
 }
 
 // ControllerUnpublishVolume deattaches the given volume from the node

internal/driver/nodeserver.go

@@ -391,20 +391,29 @@ func (ns *LinodeNodeServer) NodeGetCapabilities(ctx context.Context, req *csi.No
 }
 
 func (ns *LinodeNodeServer) NodeGetInfo(ctx context.Context, req *csi.NodeGetInfoRequest) (*csi.NodeGetInfoResponse, error) {
-	klog.V(4).Infof("NodeGetInfo called with req: %#v", req)
-
-	top := &csi.Topology{
-		Segments: map[string]string{
-			"topology.linode.com/region": ns.Metadata.Region,
-		},
+	// Get the number of currently attached instance disks, and subtract it
+	// from the limit of block devices that can be attached to the instance,
+	// which will effectively give us the number of block storage volumes
+	// that can be attached to this node/instance.
+	//
+	// This is what the spec wants us to report: the actual number of volumes
+	// that can be attached, and not the theoretical maximum number of
+	// devices that can be attached.
+	disks, err := s.CloudProvider.ListInstanceDisks(ctx, ns.Metadata.ID, nil)
+	if err != nil {
+		return &csi.NodeGetInfoResponse{}, status.Errorf(codes.Internal, "list instance disks: %v", err)
 	}
+	maxVolumes := maxVolumeAttachments(ns.Metadata.Memory) - len(disks)
 
-	resp := &csi.NodeGetInfoResponse{
-		NodeId:             strconv.Itoa(ns.Metadata.ID),
-		MaxVolumesPerNode:  int64(maxVolumeAttachments(ns.Metadata.Memory)),
-		AccessibleTopology: top,
-	}
-	return resp, nil
+	return &csi.NodeGetInfoResponse{
+		NodeId:            strconv.Itoa(ns.Metadata.ID),
+		MaxVolumesPerNode: int64(maxVolumes),
+		AccessibleTopology: &csi.Topology{
+			Segments: map[string]string{
+				"topology.linode.com/region": ns.Metadata.Region,
+			},
+		},
+	}, nil
 }
 
 func (ns *LinodeNodeServer) NodeGetVolumeStats(ctx context.Context, req *csi.NodeGetVolumeStatsRequest) (*csi.NodeGetVolumeStatsResponse, error) {