Skip to content

Commit

Permalink
feat: report RESOURCE_EXHAUSTED when max volume
Browse files Browse the repository at this point in the history
Also fix already mount error code
  • Loading branch information
guilhem committed Nov 25, 2024
1 parent 5c734b9 commit 2bc84b3
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 11 deletions.
16 changes: 9 additions & 7 deletions internal/driver/controllerserver_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,6 @@ import (

"github.com/container-storage-interface/spec/lib/go/csi"
"github.com/linode/linodego"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

linodevolumes "github.com/linode/linode-blockstorage-csi-driver/pkg/linode-volumes"
"github.com/linode/linode-blockstorage-csi-driver/pkg/logger"
Expand Down Expand Up @@ -667,13 +665,17 @@ func (cs *ControllerServer) attachVolume(ctx context.Context, volumeID, linodeID
PersistAcrossBoots: &persist,
})
if err != nil {
code := codes.Internal // Default error code is Internal.
// Check if the error indicates that the volume is already attached.
// https://github.com/container-storage-interface/spec/blob/master/spec.md#controllerpublishvolume-errors
var apiErr *linodego.Error
if errors.As(err, &apiErr) && strings.Contains(apiErr.Message, "is already attached") {
code = codes.Unavailable // Allow a retry if the volume is already attached: race condition can occur here
if errors.As(err, &apiErr) {
switch {
case strings.Contains(apiErr.Message, "is already attached"):
return errAlreadyAttached
case strings.Contains(apiErr.Message, "Maximum number of block storage volumes are attached to this Linode"):
return errMaxAttachments
}
}
return status.Errorf(code, "attach volume: %v", err)
return errInternal("attach volume %d to Linode %d: %v", volumeID, linodeID, err)
}
return nil // Return nil if the volume is successfully attached.
}
8 changes: 4 additions & 4 deletions internal/driver/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ var (
// attachments allowed for the instance, call errMaxVolumeAttachments.
errMaxAttachments = status.Error(codes.ResourceExhausted, "max number of volumes already attached to instance")

// errAlreadyAttached is used to indicate that a volume is already attached
// to a Linode instance.
errAlreadyAttached = status.Error(codes.FailedPrecondition, "volume is already attached")

// errResizeDown indicates a request would result in a volume being resized
// to be smaller than it currently is.
//
Expand Down Expand Up @@ -61,10 +65,6 @@ func errRegionMismatch(gotRegion, wantRegion string) error {
return status.Errorf(codes.InvalidArgument, "source volume is in region %q, needs to be in region %q", gotRegion, wantRegion)
}

func errMaxVolumeAttachments(numAttachments int) error {
return status.Errorf(codes.ResourceExhausted, "max number of volumes (%d) already attached to instance", numAttachments)
}

func errInstanceNotFound(linodeID int) error {
return status.Errorf(codes.NotFound, "linode instance %d not found", linodeID)
}
Expand Down

0 comments on commit 2bc84b3

Please sign in to comment.