Skip to content

Commit

Permalink
fixup! internal/driver: Fix off-by-one error with max volume attachments
Browse files Browse the repository at this point in the history
  • Loading branch information
Nick Saika committed Jul 3, 2024
1 parent 4c7b258 commit abf376e
Showing 1 changed file with 16 additions and 3 deletions.
19 changes: 16 additions & 3 deletions internal/driver/controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,11 +265,17 @@ func (linodeCS *LinodeControllerServer) ControllerPublishVolume(ctx context.Cont
}

// Check to see if there is room to attach this volume to the instance.
if canAttach, err := linodeCS.canAttach(ctx, instance); err != nil {
if canAttach, err := linodeCS.canAttach(ctx, instance); errors.Is(err, errNilInstance) {
return &csi.ControllerPublishVolumeResponse{}, status.Error(codes.Internal, "cannot determine volume attachments for a nil instance")
} else if err != nil {
return &csi.ControllerPublishVolumeResponse{}, status.Error(codes.Internal, err.Error())
} else if !canAttach {
// If we can, try and add a little more information to the error message
// for the caller.
limit, err := linodeCS.maxVolumeAttachments(ctx, instance)
if err != nil {
if errors.Is(err, errNilInstance) {
return &csi.ControllerPublishVolumeResponse{}, status.Error(codes.Internal, "cannot calculate max volume attachments for a nil instance")
} else if err != nil {
return &csi.ControllerPublishVolumeResponse{}, status.Error(codes.ResourceExhausted, "max number of volumes already attached to instance")
}
return &csi.ControllerPublishVolumeResponse{}, status.Errorf(codes.ResourceExhausted, "max number of volumes (%d) already attached to instance", limit)
Expand Down Expand Up @@ -325,12 +331,19 @@ func (s *LinodeControllerServer) canAttach(ctx context.Context, instance *linode
return len(volumes) < limit, nil
}

var (
// errNilInstance is a general-purpose error used to indicate a nil
// [github.com/linode/linodego.Instance] was passed as an argument to a
// function.
errNilInstance = errors.New("nil instance")
)

// maxVolumeAttachments returns the maximum number of volumes that can be
// 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 0, errors.New("nil instance")
return 0, errNilInstance
}

disks, err := s.CloudProvider.ListInstanceDisks(ctx, instance.ID, nil)
Expand Down

0 comments on commit abf376e

Please sign in to comment.