Skip to content

Commit

Permalink
Merge branch 'main' into golangcli-lint
Browse files Browse the repository at this point in the history
  • Loading branch information
amold1 authored Sep 23, 2024
2 parents b355ab3 + d290cb7 commit 7e4ae2a
Show file tree
Hide file tree
Showing 5 changed files with 702 additions and 19 deletions.
6 changes: 3 additions & 3 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ require (
go.uber.org/mock v0.4.0
golang.org/x/net v0.29.0
golang.org/x/sys v0.25.0
google.golang.org/grpc v1.66.2
google.golang.org/grpc v1.67.0
google.golang.org/protobuf v1.34.2
k8s.io/apimachinery v0.31.1
k8s.io/klog/v2 v2.130.1
Expand All @@ -27,10 +27,10 @@ require (
github.com/go-resty/resty/v2 v2.13.1 // indirect
github.com/godbus/dbus/v5 v5.1.0 // indirect
github.com/moby/sys/mountinfo v0.7.1 // indirect
github.com/opencontainers/runc v1.1.13 // indirect
github.com/opencontainers/runc v1.1.14 // indirect
github.com/opencontainers/runtime-spec v1.0.3-0.20220909204839-494a5a6aca78 // indirect
github.com/sirupsen/logrus v1.9.3 // indirect
golang.org/x/text v0.18.0 // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20240711142825-46eb208f015d // indirect
google.golang.org/genproto/googleapis/rpc v0.0.0-20240814211410-ddb44dafa142 // indirect
gopkg.in/ini.v1 v1.66.6 // indirect
)
12 changes: 6 additions & 6 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ github.com/martinjungblut/go-cryptsetup v0.0.0-20220520180014-fd0874fd07a6 h1:YD
github.com/martinjungblut/go-cryptsetup v0.0.0-20220520180014-fd0874fd07a6/go.mod h1:gZoZ0+POlM1ge/VUxWpMmZVNPzzMJ7l436CgkQ5+qzU=
github.com/moby/sys/mountinfo v0.7.1 h1:/tTvQaSJRr2FshkhXiIpux6fQ2Zvc4j7tAhMTStAG2g=
github.com/moby/sys/mountinfo v0.7.1/go.mod h1:IJb6JQeOklcdMU9F5xQ8ZALD+CUr5VlGpwtX+VE0rpI=
github.com/opencontainers/runc v1.1.13 h1:98S2srgG9vw0zWcDpFMn5TRrh8kLxa/5OFUstuUhmRs=
github.com/opencontainers/runc v1.1.13/go.mod h1:R016aXacfp/gwQBYw2FDGa9m+n6atbLWrYY8hNMT/sA=
github.com/opencontainers/runc v1.1.14 h1:rgSuzbmgz5DUJjeSnw337TxDbRuqjs6iqQck/2weR6w=
github.com/opencontainers/runc v1.1.14/go.mod h1:E4C2z+7BxR7GHXp0hAY53mek+x49X1LjPNeMTfRGvOA=
github.com/opencontainers/runtime-spec v1.0.3-0.20220909204839-494a5a6aca78 h1:R5M2qXZiK/mWPMT4VldCOiSL9HIAMuxQZWdG0CSM5+4=
github.com/opencontainers/runtime-spec v1.0.3-0.20220909204839-494a5a6aca78/go.mod h1:jwyrGlmzljRJv/Fgzds9SsS/C5hL+LL3ko9hs6T5lQ0=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
Expand Down Expand Up @@ -105,10 +105,10 @@ golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtn
golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
google.golang.org/genproto/googleapis/rpc v0.0.0-20240711142825-46eb208f015d h1:JU0iKnSg02Gmb5ZdV8nYsKEKsP6o/FGVWTrw4i1DA9A=
google.golang.org/genproto/googleapis/rpc v0.0.0-20240711142825-46eb208f015d/go.mod h1:Ue6ibwXGpU+dqIcODieyLOcgj7z8+IcskoNIgZxtrFY=
google.golang.org/grpc v1.66.2 h1:3QdXkuq3Bkh7w+ywLdLvM56cmGvQHUMZpiCzt6Rqaoo=
google.golang.org/grpc v1.66.2/go.mod h1:s3/l6xSSCURdVfAnL+TqCNMyTDAGN6+lZeVxnZR128Y=
google.golang.org/genproto/googleapis/rpc v0.0.0-20240814211410-ddb44dafa142 h1:e7S5W7MGGLaSu8j3YjdezkZ+m1/Nm0uRVRMEMGk26Xs=
google.golang.org/genproto/googleapis/rpc v0.0.0-20240814211410-ddb44dafa142/go.mod h1:UqMtugtsSgubUsoxbuAoiCXvqvErP7Gf0so0mK9tHxU=
google.golang.org/grpc v1.67.0 h1:IdH9y6PF5MPSdAntIcpjQ+tXO41pcQsfZV2RxtQgVcw=
google.golang.org/grpc v1.67.0/go.mod h1:1gLDyUQU7CTLJI90u3nXZ9ekeghjeM7pTDZlqFNg2AA=
google.golang.org/protobuf v1.34.2 h1:6xV6lTsCfpGD21XK49h7MhtcApnLqkfYgPcdHftf6hg=
google.golang.org/protobuf v1.34.2/go.mod h1:qYOHts0dSfpeUzUFpOMr/WGzszTmLH+DiWniOlNbLDw=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
Expand Down
4 changes: 2 additions & 2 deletions internal/driver/controllerserver.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"context"
"errors"
"strconv"
"strings"

"github.com/container-storage-interface/spec/lib/go/csi"
"github.com/linode/linodego"
Expand Down Expand Up @@ -316,7 +315,8 @@ func (cs *ControllerServer) ControllerPublishVolume(ctx context.Context, req *cs
}

log.V(4).Info("Waiting for volume to attach", "volume_id", volumeID)
volume, err = cs.client.WaitForVolumeLinodeID(ctx, volumeID, &linodeID, waitTimeout())
// Wait for the volume to be successfully attached to the instance
volume, err := cs.client.WaitForVolumeLinodeID(ctx, volumeID, &linodeID, waitTimeout())

Check failure on line 319 in internal/driver/controllerserver.go

View workflow job for this annotation

GitHub Actions / ci

no new variables on left side of :=
if err != nil {
return resp, errInternal("wait for volume to attach: %v", err)
}
Expand Down
148 changes: 143 additions & 5 deletions internal/driver/controllerserver_helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,18 +85,18 @@ const (
//
// Whether or not another volume can be attached is based on how many instance
// disks and block storage volumes are currently attached to the instance.
func (s *ControllerServer) canAttach(ctx context.Context, instance *linodego.Instance) (canAttach bool, err error) {
func (cs *ControllerServer) canAttach(ctx context.Context, instance *linodego.Instance) (canAttach bool, err error) {
log := logger.GetLogger(ctx)
log.V(4).Info("Checking if volume can be attached", "instance_id", instance.ID)

// Get the maximum number of volume attachments allowed for the instance
limit, err := s.maxAllowedVolumeAttachments(ctx, instance)
limit, err := cs.maxAllowedVolumeAttachments(ctx, instance)
if err != nil {
return false, err
}

// List the volumes currently attached to the instance
volumes, err := s.client.ListInstanceVolumes(ctx, instance.ID, nil)
volumes, err := cs.client.ListInstanceVolumes(ctx, instance.ID, nil)
if err != nil {
return false, errInternal("list instance volumes: %v", err)
}
Expand All @@ -107,7 +107,7 @@ func (s *ControllerServer) canAttach(ctx context.Context, instance *linodego.Ins

// maxAllowedVolumeAttachments calculates the maximum number of volumes that can be attached to a Linode instance,
// taking into account the instance's memory and currently attached disks.
func (s *ControllerServer) maxAllowedVolumeAttachments(ctx context.Context, instance *linodego.Instance) (int, error) {
func (cs *ControllerServer) maxAllowedVolumeAttachments(ctx context.Context, instance *linodego.Instance) (int, error) {
log := logger.GetLogger(ctx)
log.V(4).Info("Calculating max volume attachments")

Expand All @@ -117,7 +117,7 @@ func (s *ControllerServer) maxAllowedVolumeAttachments(ctx context.Context, inst
}

// Retrieve the list of disks currently attached to the instance
disks, err := s.client.ListInstanceDisks(ctx, instance.ID, nil)
disks, err := cs.client.ListInstanceDisks(ctx, instance.ID, nil)
if err != nil {
return 0, errInternal("list instance disks: %v", err)
}
Expand Down Expand Up @@ -480,3 +480,141 @@ func (cs *ControllerServer) prepareCreateVolumeResponse(ctx context.Context, vol

return resp
}

// validateControllerPublishVolumeRequest validates the incoming ControllerPublishVolumeRequest.
// It extracts the Linode ID and Volume ID from the request and checks if the
// volume capability is provided and valid. If any validation fails, it returns
// an appropriate error.
func (cs *ControllerServer) validateControllerPublishVolumeRequest(ctx context.Context, req *csi.ControllerPublishVolumeRequest) (int, int, error) {
log := logger.GetLogger(ctx)
log.V(4).Info("Entering validateControllerPublishVolumeRequest()", "req", req)
defer log.V(4).Info("Exiting validateControllerPublishVolumeRequest()")

// extract the linode ID from the request
linodeID, statusErr := linodevolumes.NodeIdAsInt("ControllerPublishVolume", req)
if statusErr != nil {
return 0, 0, statusErr
}

// extract the volume ID from the request
volumeID, statusErr := linodevolumes.VolumeIdAsInt("ControllerPublishVolume", req)
if statusErr != nil {
return 0, 0, statusErr
}

// retrieve the volume capability from the request
cap := req.GetVolumeCapability()
// return an error if no volume capability is provided
if cap == nil {
return 0, 0, errNoVolumeCapability
}
// return an error if the volume capability is invalid
if !validVolumeCapabilities([]*csi.VolumeCapability{cap}) {
return 0, 0, errInvalidVolumeCapability([]*csi.VolumeCapability{cap})
}

log.V(4).Info("Validation passed", "linodeID", linodeID, "volumeID", volumeID)
return linodeID, volumeID, nil
}

// getAndValidateVolume retrieves the volume by its ID and checks if it is
// attached to the specified Linode instance. If the volume is found and
// already attached to the instance, it returns its device path.
// If the volume is not found or attached to a different instance, it
// returns an appropriate error.
func (cs *ControllerServer) getAndValidateVolume(ctx context.Context, volumeID, linodeID int) (string, error) {
log := logger.GetLogger(ctx)
log.V(4).Info("Entering getAndValidateVolume()", "volumeID", volumeID, "linodeID", linodeID)
defer log.V(4).Info("Exiting getAndValidateVolume()")

volume, err := cs.client.GetVolume(ctx, volumeID)
if linodego.IsNotFound(err) {
return "", errVolumeNotFound(volumeID)
} else if err != nil {
return "", errInternal("get volume %d: %v", volumeID, err)
}

if volume.LinodeID != nil {
if *volume.LinodeID == linodeID {
log.V(4).Info("Volume already attached to instance", "volume_id", volume.ID, "node_id", *volume.LinodeID, "device_path", volume.FilesystemPath)
return volume.FilesystemPath, nil
}
return "", errVolumeAttached(volumeID, linodeID)
}

log.V(4).Info("Volume validated and is not attached to instance", "volume_id", volume.ID, "node_id", linodeID)
return "", nil
}

// getInstance retrieves the Linode instance by its ID. If the
// instance is not found, it returns an error indicating that the instance
// does not exist. If any other error occurs during retrieval, it returns
// an internal error.
func (cs *ControllerServer) getInstance(ctx context.Context, linodeID int) (*linodego.Instance, error) {
log := logger.GetLogger(ctx)
log.V(4).Info("Entering getInstance()", "linodeID", linodeID)
defer log.V(4).Info("Exiting getInstance()")

instance, err := cs.client.GetInstance(ctx, linodeID)
if linodego.IsNotFound(err) {
return nil, errInstanceNotFound(linodeID)
} else if err != nil {
// If any other error occurs, return an internal error.
return nil, errInternal("get linode instance %d: %v", linodeID, err)
}

log.V(4).Info("Instance retrieved", "instance", instance)
return instance, nil
}

// checkAttachmentCapacity checks if the specified instance can accommodate
// additional volume attachments. It retrieves the maximum number of allowed
// attachments and compares it with the currently attached volumes. If the
// limit is exceeded, it returns an error indicating the maximum volume
// attachments allowed.
func (cs *ControllerServer) checkAttachmentCapacity(ctx context.Context, instance *linodego.Instance) error {
log := logger.GetLogger(ctx)
log.V(4).Info("Entering checkAttachmentCapacity()", "linodeID", instance.ID)
defer log.V(4).Info("Exiting checkAttachmentCapacity()")

canAttach, err := cs.canAttach(ctx, instance)
if err != nil {
return err
}
if !canAttach {
// If the instance cannot accommodate more attachments, retrieve the maximum allowed attachments.
limit, err := cs.maxAllowedVolumeAttachments(ctx, instance)
if errors.Is(err, errNilInstance) {
return errInternal("cannot calculate max volume attachments for a nil instance")
} else if err != nil {
return errMaxAttachments // Return an error indicating the maximum attachments limit has been reached.
}
return errMaxVolumeAttachments(limit) // Return an error indicating the maximum volume attachments allowed.
}
return nil // Return nil if the instance can accommodate more attachments.
}

// attachVolume attaches the specified volume to the given Linode instance.
// It logs the action and handles any errors that may occur during the
// attachment process. If the volume is already attached, it allows for a
// retry by returning an Unavailable error.
func (cs *ControllerServer) attachVolume(ctx context.Context, volumeID, linodeID int) error {
log := logger.GetLogger(ctx)
log.V(4).Info("Entering attachVolume()", "volume_id", volumeID, "node_id", linodeID)
defer log.V(4).Info("Exiting attachVolume()")

persist := false
_, err := cs.client.AttachVolume(ctx, volumeID, &linodego.VolumeAttachOptions{
LinodeID: linodeID,
PersistAcrossBoots: &persist,
})
if err != nil {
code := codes.Internal // Default error code is Internal.

Check failure on line 612 in internal/driver/controllerserver_helper.go

View workflow job for this annotation

GitHub Actions / ci

undefined: codes
// Check if the error indicates that the volume is already attached.
if apiErr, ok := err.(*linodego.Error); ok && strings.Contains(apiErr.Message, "is already attached") {
code = codes.Unavailable // Allow a retry if the volume is already attached: race condition can occur here

Check failure on line 615 in internal/driver/controllerserver_helper.go

View workflow job for this annotation

GitHub Actions / ci

undefined: codes
}
return status.Errorf(code, "attach volume: %v", err)

Check failure on line 617 in internal/driver/controllerserver_helper.go

View workflow job for this annotation

GitHub Actions / ci

undefined: status
}
return nil // Return nil if the volume is successfully attached.
}
Loading

0 comments on commit 7e4ae2a

Please sign in to comment.